The Wayback Machine - http://web.archive.org/web/20201023063354/https://github.com/andywer/threads.js/pull/289
Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: catch worker errors #289

Draft
wants to merge 1 commit into
base: master
from
Draft

feat: catch worker errors #289

wants to merge 1 commit into from

Conversation

@Ugzuzg
Copy link

@Ugzuzg Ugzuzg commented Aug 6, 2020

Depends on avoidwork/tiny-worker#43
Reject the promise if the worker was killed by the system or unexpectedly exited.

A nice feature of threads@0.x was that you could listen for exit event of the spawned worker and reject the promise based on it.

await new Promise((resolve, reject) => thread
  .run('./src/ChildProcessHelper')
  .send({ data })
  .on('exit', (code, signal) => {
    if (code !== 0) {
      reject(new Error(`child process exited with ${code} ${signal}`));
    } else {
      resolve([]);
    }
  })
  .promise()
  .then(resolve, reject));

With worker_threads this isn't possible at all, as they are running in the same process, so if something bad happens there, the main process is affected (e.g. segfault).

That's one more reason for enabling tiny-worker on latest node versions. #229

The code just illustrates the idea.

@andywer
Copy link
Owner

@andywer andywer commented Aug 16, 2020

Thanks for sharing that PR, @Ugzuzg.

Can you check why the new test case is failing?

test/spawn.test.ts Outdated Show resolved Hide resolved
Depends on avoidwork/tiny-worker#43
Reject the promise if the worker was killed by the system
or unexpectedly exited.
@Ugzuzg Ugzuzg force-pushed the Ugzuzg:master branch from f23c4d5 to 5470468 Aug 16, 2020
@Ugzuzg
Copy link
Author

@Ugzuzg Ugzuzg commented Aug 28, 2020

@andywer, it fails because there is a dependency on a tiny-worker avoidwork/tiny-worker#43 PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants
You can’t perform that action at this time.