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

Core: Catch and do nothing to avoid triggering unhandled exception problems #20177

Merged
merged 1 commit into from
Dec 9, 2022

Conversation

tmeasday
Copy link
Member

@tmeasday tmeasday commented Dec 9, 2022

Issue: #20161

What I did

Put an extra empty .catch() in to avoid the issue.

Explanation

Try running this code:

async function errorFunction() {
  throw new Error('error')
}

async function go() {
  try {
    const r = errorFunction()
    //  r.then(() => { });
    await r;
  } catch (err) {
    console.log('caught error');
    await new Promise((r) => setTimeout(r, 10));
    console.log('done');
  }
}

go();

As written it will exit 0 and log:

caught error
done

If you comment the .then() block, it will exit 1 and log:

caught error
/Users/tom/GitHub/storybookjs/storybook/sandbox/nextjs-default-js/test.js:2
  throw new Error('error')
        ^

Error: error
    at errorFunction (/Users/tom/GitHub/storybookjs/storybook/sandbox/nextjs-default-js/test.js:2:9)
    at go (/Users/tom/GitHub/storybookjs/storybook/sandbox/nextjs-default-js/test.js:7:15)
    at Object.<anonymous> (/Users/tom/GitHub/storybookjs/storybook/sandbox/nextjs-default-js/test.js:17:1)
    at Module._compile (node:internal/modules/cjs/loader:1101:14)
    at Object.Module._extensions..js (node:internal/modules/cjs/loader:1153:10)
    at Module.load (node:internal/modules/cjs/loader:981:32)
    at Function.Module._load (node:internal/modules/cjs/loader:822:12)
    at Function.executeUserEntryPoint [as runMain] (node:internal/modules/run_main:81:12)
    at node:internal/main/run_main_module:17:47

If you put a .catch() in before the .then() it will do the first behaviour.

I suppose node has a heuristic for whether exceptions are being handled that is wrong here. We could also (and maybe should) add a process.on('unhandledException') to deal with this.

How to test

Run a sandbox (not with --ci) and put a incorrect import in a file.

Copy link
Member

@shilman shilman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Crazy stuff 🤯

Thanks so much for getting to the bottom of this @tmeasday -- you're a hero!

@shilman shilman merged commit 50ea090 into next Dec 9, 2022
@shilman shilman deleted the fix-unhandled-exception branch December 9, 2022 04:31
@tmeasday tmeasday restored the fix-unhandled-exception branch December 10, 2022 01:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants