Skip to content

fix(process-runner): settle a JS process run when its worker dies - #342

Open
kevin9327 wants to merge 1 commit into
lightningpixel:devfrom
kevin9327:fix/process-runner-worker-exit
Open

kevin9327 wants to merge 1 commit into
lightningpixel:devfrom
kevin9327:fix/process-runner-worker-exit

Conversation

@kevin9327

Copy link
Copy Markdown
Contributor

What

If the worker thread behind a JS process node dies in the middle of a run, the workflow waits on that node forever. Every later run of the same node hangs the same way until the app is restarted. Ways the worker can die include:

  • running out of memory on a large mesh;
  • an uncaught error thrown outside the awaited processor call, such as a stream error event or a timer callback;
  • process.exit().

No error is shown and the run never finishes.

Why it triggers

ProcessRunner keeps one warm worker per extension. run() only listens for the worker's 'message' events and waits for a done or error message:

worker.on('message', handler)
worker.postMessage({ action: 'run', input, params })

A worker that dies never posts either message, so the promise never settles. extensions:runProcess has no timeout, so the workflow's await window.electron.extensions.runProcess(...) never returns. The dead worker also stays cached with ready = true, so the next run skips ensureReady() and posts into a terminated worker. That run hangs too.

Fix

For the duration of a run, also listen for the worker's 'error' and 'exit' events. When either fires, reject the run with the cause (the error itself, or Process extension worker exited with code N) and drop the dead worker, so the next run starts a fresh one. All three listeners are removed once the run settles.

Errors thrown by the processor itself are unchanged. The worker still catches them and posts an error message, and the warm worker is kept.

This touches a different part of process-runner.ts than #338, which changes the runner registry. git merge-tree merges the two branches cleanly, and both new test files pass on the merged result (6/6).

Verification

New electron/main/process-runner-worker-exit.test.mjs, esbuild-bundled like the other electron/main tests. It drives ProcessRunner with a small processor whose behaviour depends on params.mode.

  • a run whose worker crashes rejects instead of hanging: an uncaught error thrown from a timer. Fails before (times out), passes after.
  • after its worker exits, the runner starts a fresh one for the next run: process.exit(3), then a normal run gets a fresh worker (run counter 1). Fails before (times out), passes after.
  • an error thrown by the processor still rejects with its message and keeps the warm worker: passes before and after. The run rejects with Error: bad input, and the next run lands on the same worker (run counter 2). That proves the change doesn't widen into tearing the worker down on ordinary processor errors.

Fail-before output, with the source reverted and the test kept. It is verbatim:

✖ a run whose worker crashes rejects instead of hanging (5005.9191ms)
✖ after its worker exits, the runner starts a fresh one for the next run (5015.345ms)
✔ an error thrown by the processor still rejects with its message and keeps the warm worker (40.3875ms)
ℹ tests 3
ℹ suites 0
ℹ pass 1
ℹ fail 0
ℹ cancelled 2
ℹ skipped 0
ℹ todo 0
ℹ duration_ms 10199.8863

✖ failing tests:

test at electron\main\process-runner-worker-exit.test.mjs:55:1
✖ a run whose worker crashes rejects instead of hanging (5005.9191ms)
  'test timed out after 5000ms'

test at electron\main\process-runner-worker-exit.test.mjs:64:1
✖ after its worker exits, the runner starts a fresh one for the next run (5015.345ms)
  'test timed out after 5000ms'

After the fix:

✔ a run whose worker crashes rejects instead of hanging (95.675ms)
✔ after its worker exits, the runner starts a fresh one for the next run (58.3336ms)
✔ an error thrown by the processor still rejects with its message and keeps the warm worker (38.5979ms)
ℹ tests 3
ℹ pass 3
ℹ fail 0
ℹ cancelled 0

Whole suite (npm run test:node), unmodified dev compared with this branch:

dev this branch
TS tests 37 pass, 0 fail 37 pass, 0 fail
*.test.mjs 135 tests: 130 pass, 5 skipped, 0 fail 138 tests: 133 pass, 5 skipped, 0 fail
npx eslint . 0 problems 0 problems
npx tsc --noEmit -p tsconfig.node.json 9 errors 9 errors

The 5 skips were already there on dev. The 9 tsc errors are also pre-existing, and none of them are in process-runner.ts. No Python touched.

🤖 Generated with Claude Code

A JS process extension runs in a worker thread kept warm between runs,
and run() only listened for the worker's 'done'/'error' messages. If the
worker died mid-run (an uncaught error outside the awaited processor
call, running out of memory on a large mesh, process.exit) neither
message ever arrived: the workflow waited on that node forever, and
because the dead worker stayed cached, every later run of the node hung
the same way until the app was restarted.

Listen for the worker's 'error' and 'exit' events for the duration of a
run, reject with the cause, and drop the dead worker so the next run
starts a fresh one. Errors thrown by the processor itself are still
reported through its 'error' message and keep the warm worker.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant