Improve node OnUncaughtException integration

[mydea]

Problem Statement

The current integration configuration for the node OnUncaughtExceptionOptions exception can be a bit confusing, especially with recently necessary changes regarding the default behavior.

Current behavior:

• Following the node.js default, we exit the process on uncaught exceptions
• For e.g. next.js, we need the capability to opt out of this behavior - however, we only want to opt out of it if any other handler has been registered.
• Then there is also the option of providing an onFatalError handler to conditionally define if the process should exit or not

Solution Brainstorm

We could add a new option to (eventually) replace all existing options, e.g. mode.

A possible API could be:

type OnUncaughtExceptionMode = 'exit' | 'continue' | (firstError: Error, secondError?: Error) => boolean;

// default, matches node.js
new OnUncaughtException({
  onException: 'exit'
});

new OnUncaughtException({
  onException: 'continue'
});

new OnUncaughtException({
  onException: (error) => {
    // do something
    // return `true` to exit the process, any other return value will keep it running
    return true;
  }
});

With the behaviour:

• exit: Always exit the process on uncaught exceptions. The default.
• continue: Do not exit the process. Could be set by the next.js integration.
• a function: Leave this to the user. Can eventually replace onFatalError

[mydea]

So I think the following exception handler would kind of cover all relevant cases for now:

const onExceptionHandler = (error: Error): void => {
  const { onException } = this._options;
  const hub = getCurrentHub();

  if (hub.getIntegration(OnUncaughtException)) {
    hub.withScope((scope: Scope) => {
      scope.setLevel('fatal');
      hub.captureException(error, {
        originalException: error,
        data: { mechanism: { handled: false, type: 'onuncaughtexception' } },
      });
    });
  }

  if (onException === 'exit') {
    logAndExitProcess(error);
    return;
  }

  if (onException === 'warn') {
   logger.warn(`onUncaughtException integration encountered an exception: ${error}`);
    return;
  }

  if (typeof onException === 'function') {
    const result = onException(error);

    if (result !== undefined) {
      logAndExitProcess(result);
    }
    return;
  }

  // else: 'continue'
};

My idea would be to, for now, allow to opt-in to that behavior by setting onException on the integration. If that's set, use the new logic, else, the "old" one.

Then we can think about deprecating the "old" functionality, and flipping defaults for v8. The trickiest migration/thing is the onFatalError stuff, not sure if we want to/need to keep support for that (without any changes) in the future.

Thoughts, @lobsterkatie ?

We could then remove (or deprecate) the exitEvenIfOtherHandlersAreRegistered and instead set onException: 'continue' in the nextjs integration.

[Nikoms]

I agree that, by default, it shouldn't call exit. We just updated the library and took a day to understand why our servers were killed 😅.

[AbhiPrasad]

Can this be closed now @lforst?

[lforst]

Yes! Thanks for reminding me.