Figure out the best way to address unhandled promise rejections

[davewasmer]

Should we add our own unhandledRejection hook that prints out more useful debug info? How do we handle the change in Node 8 (I think) that throws on unhandledRejections

[seawatts]

What are you thinking for this? I have run into this problem a lot already with my app.

[davewasmer]

Not sure yet, exactly.

Here's my gut reaction: unhandled rejections should throw and crash the server process, regardless of whether Node will do this for us[1]. Especially with the prevalence of async/await throughout Denali, which means lots of normally synchronous throws will end up wrapped as promise rejections, and the best thing to do with an uncaught exception is to crash the process and restart. Unhandled rejections are semantically equivalent[2] to a error thrown without a try/catch block to handle it, and those crash the process, so rejections should as well.

Now, what we actually do with the unhandled rejection before crashing - not entirely sure. Based on the linked thread above, it seems like basically all I/O operations might be unsafe once an unhandled exception occurs. But obviously crashing silently is useless. So my thought is we try to invoke the app's Logger.error method with the error, then crash?

---

• [1] Seems like Node 8 does not implement this, it just continues to warn that future versions will.
• [2] Most of the time. You could be using Promises for async control flow, with a rejection indicating something like break;, and it may be impossible for the wrapped code to actually throw an exception. In that scenario, you'd have to add a .catch(() => {}) to make Node happy, but that seems like a small price to pay vs. allowing applications to continue in an undefined state.

[seawatts]

Yeah those are good points. I'm not sure about the automatic restart. I'm on heroku and it will do the automatic restart for you. I'm curious of what it would do if you have both restarts in there?

…

On Mon, Oct 2, 2017 at 1:16 PM Dave Wasmer ***@***.***> wrote: Not sure yet, exactly. Here's my gut reaction: unhandled rejections should throw *and crash the server process*, regardless of whether Node will do this for us[1]. Especially with the prevalence of async/await throughout Denali, which means lots of normally synchronous throws will end up wrapped as promise rejections, and the best thing to do with an uncaught exception is to crash the process and restart <nodejs/node-v0.x-archive#2582 (comment)>. Unhandled rejections are semantically equivalent[2] to a error thrown without a try/catch block to handle it, and those crash the process, so rejections should as well. Now, what we actually do with the unhandled rejection before crashing - not entirely sure. Based on the linked thread above, it seems like basically all I/O operations might be unsafe once an unhandled exception occurs. But obviously crashing silently is useless. So my thought is we try to invoke the app's Logger.error method with the error, then crash? ------------------------------ - [1] Seems like Node 8 does *not* implement this, it just continues to warn that future versions will. - [2] Most of the time. You could be using Promises for async control flow, with a rejection indicating something like break;, and it may be impossible for the wrapped code to actually throw an exception. In that scenario, you'd have to add a .catch(() => {}) to make Node happy, but that seems like a small price to pay vs. allowing applications to continue in an undefined state. — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#390 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/ACDIwnEnZFSqYxbBxRJWjj3LXq8t1DMBks5soUSEgaJpZM4PSEB5> .

[davewasmer]

Ah, sorry, should have clarified - the Denali runtime framework won't do anything to restart itself, it will just crash.

We haven't fleshed out the deployment story very well yet, so perhaps there will be a denali server --production that would include automatic restarts, but that would be the CLI running as an daemon and restarting child server processes.