fix(sandbox): strip node --watch flags from child execArgv - #4104
Open
mohanrajvenkatesan23-04 wants to merge 3 commits into
Open
fix(sandbox): strip node --watch flags from child execArgv#4104mohanrajvenkatesan23-04 wants to merge 3 commits into
mohanrajvenkatesan23-04 wants to merge 3 commits into
Conversation
…sh#1833) When a BullMQ worker is launched with `node --watch`, the `--watch` family of flags is inherited by sandboxed child processes and worker threads via `process.execArgv`. Having the sandboxed runtime also enter watch mode interferes with the IPC channel between the parent worker and the processor, which on older Node.js versions (<24.15) left jobs stuck in the `active` state and on newer versions surfaced as `ERR_WORKER_INVALID_EXEC_ARGV` crashes. Extend `convertExecArgv` to drop any `--watch`, `--watch=*`, or `--watch-*` entry before forwarding the remaining flags to the child. Unrelated flags (including `--inspect` with its port-reassignment logic) are untouched. Adds a regression test in the child-pool suite that asserts `--watch`, `--watch-path=...`, `--watch-preserve-output`, and `--watch-kill-signal=...` are removed from the forked child's spawn arguments, and documents the behaviour in the sandboxed-processors docs.
Contributor
There was a problem hiding this comment.
Pull request overview
This PR prevents Node.js --watch* flags from leaking into BullMQ sandboxed child processes / worker threads via process.execArgv, addressing jobs getting stuck in active and newer Node versions crashing with invalid worker execArgv.
Changes:
- Extend
convertExecArgvto drop the--watchfamily of flags before existing--inspect*handling. - Add a regression test ensuring
--watch*flags are not present in spawned child process arguments. - Document behavior and guidance when overriding
execArgvviaworkerForkOptions/workerThreadsOptions.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
src/classes/child.ts |
Adds isWatchFlag and filters --watch* flags out of execArgv passed to children. |
tests/child-pool.test.ts |
Adds regression coverage to ensure --watch* flags aren’t forwarded to forked children. |
docs/gitbook/guide/workers/sandboxed-processors.md |
Documents running sandboxed processors under node --watch and warns about overrides. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Address review feedback on taskforcesh#4104: when the test runner itself is started with `node --watch`, the existing indexOf() cleanup would strip the runner's flag. Since each watchFlag is appended via push(), lastIndexOf() reliably targets our own entry without disturbing pre-existing values.
Contributor
Author
|
Thanks for the review! Pushed a follow-up addressing the cleanup concern:
A short comment was added so the rationale isn't lost. Happy to refactor to a snapshot/restore pattern instead if you'd prefer. |
Contributor
|
@copilot resolve the merge conflicts in this pull request |
2 similar comments
Contributor
|
@copilot resolve the merge conflicts in this pull request |
Contributor
|
@copilot resolve the merge conflicts in this pull request |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #1833
Why
When a BullMQ worker is launched with
node --watch, the--watchfamily of flags is inherited by sandboxed child processes (and worker threads) viaprocess.execArgv. Having the sandboxed runtime also enter watch mode interferes with the IPC channel between the parent worker and the processor:activestate (this issue), andERR_WORKER_INVALID_EXEC_ARGVcrashes (now-closed issue [Bug]: Error "--inspect-publish-uid destination can be stderr or http" when run withnode --watch#3699 after upstream fix in Node v24.15.0).The existing
convertExecArgvfilter only stripped--inspect*, so--watchand friends leaked through.How
convertExecArgvinsrc/classes/child.tswith anisWatchFlagpredicate that matches--watch,--watch=*, and--watch-*and drops those entries before the existing--inspecthandling.--inspectwith its port-reassignment logic) are untouched.convertExecArgvis unchanged; no new exports.Additional Notes (Optional)
tests/child-pool.test.tspushes the four--watch*variants (--watch,--watch-path=...,--watch-preserve-output,--watch-kill-signal=...) ontoprocess.execArgv, asserts none end up in the forked child'sspawnargs, and cleans up only the flags it added so neighbouringexecArgvtests are unaffected.node --watch" section added todocs/gitbook/guide/workers/sandboxed-processors.mdwarning users about customworkerForkOptions.execArgv/workerThreadsOptions.execArgvoverriding the filter.