sentry-javascript: Can't prevent Sentry SDK from exiting the process even with onFatalError

@sentry/node version: 4.1.1 Node.js version: 10.9.0

Sentry exits my process after an uncaught exception even I provide my own implementation of onFatalError. How can I prevent that?

Here is a minimal working example. Without sentry.init I get to the 2nd throw. With it the process finisches after the 1st one.

const sentry = require('@sentry/node');

sentry.init(
  { dsn: 'https://key@mysentry.mycompany/2', 
    integrations: [new sentry.Integrations.OnUncaughtException({
      onFatalError: error => {
        console.log('an error occured', error);
      }
    })]
  }
);

process.on('uncaughtException', err => {
  console.log(err, 'Uncaught Exception thrown');
  // process.exit(1);
});

console.log("before throwing the first time")
setTimeout(() => { throw 42 }, 2000)
setTimeout(() => { console.log('still running') }, 10000)
throw 42;

About this issue

Original URL
State: closed
Created 6 years ago
Reactions: 4
Comments: 22 (12 by maintainers)

Commits related to this issue

Sentry Node: do not exit process if the app added its own 'uncaughtException' or 'unhandledRejection' listener - Fixes #1661 — committed to ibc/sentry-javascript by ibc 2 years ago
Sentry Node: do not exit process if the app added its own 'uncaughtException' or 'unhandledRejection' listener - Fixes #1661 — committed to ibc/sentry-javascript by ibc 2 years ago

Most upvoted comments

onFatalError is only meant to perform a cleanup before we exit the process, not fully prevent it. This is how Node.js is behaving by default (always crash the process and let it restart).

If you want to have a complete control over it, you have to turn off this integration and add handler yourself.

Sentry.init({
  dsn: 'https://key@mysentry.mycompany/2',
  integrations(integrations) {
    return integrations.filter(integration => integration.id !== 'OnUncaughtException')
  }
});

global.process.on('uncaughtException', (error) => {
  const hub = Sentry.getCurrentHub();
  hub.withScope(async (scope) => {
    scope.setLevel('fatal');
    hub.captureException(error, { originalException: error });
  });
});

kamilogorek on Oct 17, 2018

onFatalError is only meant to perform a cleanup before we exit the process, not fully prevent it. This is how Node.js is behaving by default (always crash the process and let it restart).

I just ran into this. It would be wonderful to document the intent spelled out by @kamilogorek somewhere.

Maybe here?: https://docs.sentry.io/platforms/node/configuration/integrations/default-integrations/#onuncaughtexception

JamesHagerman on Oct 28, 2020

@dj-hedgehog it should be integration.name !== 'OnUncaughtException' not integration.id !== 'OnUncaughtException'. Second format is still unreleased, sorry about that 😅

kamilogorek on Oct 18, 2018

As per the Node docs if an unhandled rejection is triggered the process should be exited: https://nodejs.org/api/process.html#warning-using-uncaughtexception-correctly.

The correct use of ‘uncaughtException’ is to perform synchronous cleanup of allocated resources (e.g. file descriptors, handles, etc) before shutting down the process. It is not safe to resume normal operation after ‘uncaughtException’.

To restart a crashed application in a more reliable way, whether ‘uncaughtException’ is emitted or not, an external monitor should be employed in a separate process to detect application failures and recover or restart as needed.

If you want to override Node’s recommendation, you can override this via the exitEvenWhenOtherOnUncaughtExceptionHandlersAreRegistered option.

Sentry.init({
  dsn: 'https://public@dsn.ingest.sentry.io/1337',
  integrations: integrations => {
    return integrations.map(integration => {
      if (integration.name === 'OnUncaughtException') {
        // override `OnUncaughtException` integration to not exit.
        return new Sentry.Integrations.OnUncaughtException({
          // do not exit if other uncaught exception handlers are registered.
          exitEvenIfOtherHandlersAreRegistered: false,
        });
      } else {
        return integration;
      }
    });
  },
});

In a future major version we will make this API better. Thanks!

AbhiPrasad on Apr 5, 2023

PR here https://github.com/getsentry/sentry-javascript/pull/5176

ibc on May 30, 2022

We do that for browser SDK, not sure why we decide to skip it here tbh. Might be just something that we missed - https://github.com/getsentry/sentry-javascript/blob/cc4495790e49b654e2f1d1e65c64e95de9f5a1ef/packages/utils/src/instrument.ts#L583-L621

TIL about uncaughtExceptionMonitor, thanks! We still do need to support older versions, so we’d need to do something similar to browser implementation instead. I wonder if it can handle async code though, which we need to flush remaining events from the buffer - https://github.com/getsentry/sentry-javascript/blob/cc4495790e49b654e2f1d1e65c64e95de9f5a1ef/packages/node/src/integrations/utils/errorhandling.ts#L32 There’s a possibility that monitor will trigger events, and let it fall through to uncaughtException handler, which will in turns kill the process before letting requests out.

I’ll notify the team to backlog this issue 😃

kamilogorek on Mar 16, 2022

What was exactly your case? We are following all the defaults from node’s implementation written down in the docs - https://nodejs.org/api/process.html#event-uncaughtexception We also mention what we do there in our own docs - https://docs.sentry.io/platforms/node/configuration/integrations/default-integrations/#onuncaughtexception

We also had no reports of this being not an intended behavior in almost 4 years, thus my question here.

cc @getsentry/team-web-backend

kamilogorek on Mar 15, 2022

@dj-hedgehog Does this help? https://docs.sentry.io/learn/draining/?platform=node

HazAT on Oct 17, 2018