sentry-javascript
sentry-javascript copied to clipboard
The `onUnhandledRejectionIntegration` does not exit gracefully in strict mode
Is there an existing issue for this?
- [X] I have checked for existing issues https://github.com/getsentry/sentry-javascript/issues
- [X] I have reviewed the documentation https://docs.sentry.io/
- [X] I am using the latest SDK release https://github.com/getsentry/sentry-javascript/releases
How do you use Sentry?
Sentry Saas (sentry.io)
Which SDK are you using?
@sentry/node
SDK Version
8.5.0
Framework Version
No response
Link to Sentry event
No response
SDK Setup
const sentryConfig: Sentry.NodeOptions = {
dsn: process.env.SENTRY_DSN,
environment: process.env.ENVIRONMENT,
integrations: [
Sentry.onUnhandledRejectionIntegration({
mode: 'strict',
}),
],
};
Sentry.init(sentryConfig);
Steps to Reproduce
Trigger an unhandled promise rejection while using the onUnhandledRejectionIntegration in mode: 'strict'.
Expected Result
For the app to exit gracefully.
Actual Result
When Sentry's onUnhandledRejectionIntegration catches an unhandled promise rejection in strict mode, it is currently designed to exit immediately:
https://github.com/getsentry/sentry-javascript/blob/005f40df3e9e2715b9f634d4490d1e42760be61d/packages/node/src/utils/errorhandling.ts#L34
This does not allow the app to shut things down and exit gracefully, because it bypasses any SIGTERM listeners. The code should really be designed like so:
if (process.listenerCount('SIGTERM')) {
global.process.kill(process.pid, 'SIGTERM');
} else {
global.process.exit(1);
}
This will detect if there are any SIGTERM listeners, and if so, rely on those to handle shut down and exit.
Note, a temporary fix for anyone else encountering this would be to not use mode: 'strict' and instead add your own unhandledRejection listener after calling Sentry.init():
process.on('unhandledRejection', () => {
if (process.listenerCount('SIGTERM')) {
process.kill(process.pid, 'SIGTERM');
} else {
process.exit(1);
}
});
Hi, thanks for writing in. Would you mind walking me through your thought process?
This is how I currently look at this issue: You manually set mode: 'strict' meaning you told the SDK to exit the process within its unhandledRejection handler. You also mentioned SIGTERM listeners not being called. As far as I understand, SIGTERM listeners are not invoked by Node when it crashes through unhandled promise rejections. I verified with the following snippet:
process.once('SIGTERM', () => {
console.log('sigterm received'); // not called
});
setTimeout(() => {
console.log('healthy'); // not called
}, 4000);
Promise.reject();
As far as I can tell, you simply shouldn't set the mode to 'strict' if you don't want the SDK to exit, and instead use 'warn' (which is the default) or 'none'.
I can see your reasoning, and while I'd normally say adhering to the behavior of a platform like Node.js is a good idea, when it comes to exiting gracefully and signal handling, Node.js isn't the best example; there are a number of flaws with how Node.js handles signal listeners and graceful shutdown.
Essentially, my reasoning is that if a program registers SIGTERM listeners, then it wants to clean things up when it's time to exit. So using process.kill(process.pid, 'SIGTERM') to shut down is the way to trigger a graceful exit. While it deviates from Node.js's behavior, I'd argue that this is a better exiting strategy because it allows for cleanup tasks to be executed.
The workaround I suggested avoids using strict, but it introduces slightly extra complexity that wouldn't be necessary if Sentry's SDK terminated more gracefully.
Thanks for elaborating. I think we will not change this behaviour. I don't intend to dismiss your point but there is some history 😄 From my last research on whether we can be more accurate here, I don't think there is a way to exactly match Node.js's exit behaviour when intending run IO (in our case to flush data). We certainly do not want to "improve" Node.js' behaviours - that is up to our users.
I'll close this to keep our issue stream clean but feel free to reach out if you have any questions or concerns!