Should we handle uncaughtExceptions?

[afharo]

I noticed that running the piece of code below logs Uncaught Exception error: Error: Something went terribly wrong.

const RxJs = require('rxjs');

RxJs.timer(0, 10_000)
  .pipe(
    RxJs.exhaustMap(async () => {
      return Promise.reject(new Error('Something went terribly wrong.'))
    })
  )
  .subscribe()


process.on('unhandledRejection', (reason) => {
  console.log('Unhandled Rejection ', 'reason:', reason);
});

process.on('uncaughtException', (error) => {
  console.log('Uncaught Exception ', 'error:', error);
});

// Keep the app running (so that I can detect crashes)
setInterval(() => {}, 1000)

We have a lot of patterns where we run promises inside exhaustMap/switchMap/concatMap in RxJS. We are already handling unhandled rejections, but we don't do so for exceptions. Should we?

How would we handle intentional crashing errors, though?

[elasticmachine]

Pinging @elastic/kibana-core (Team:Core)

[elasticmachine]

Pinging @elastic/kibana-operations (Team:Operations)

[lukeelmers]

This feels like such an easy thing to miss, especially when dealing with RxJS which can already be confusing enough.

I'm not sure how we could differentiate between exceptions we want to crash, and ones we want to prevent crashing... but if we could come up with a strategy for it, I think it'd be worth considering.

Or alternatively maybe we catch exceptions and continue to crash, but try to log the error in a more uniform way that's easier to monitor? 🤷

[TinaHeiligers]

Or alternatively maybe we catch exceptions and continue to crash, but try to log the error in a more uniform way that's easier to monitor

We should fix unhandled exceptions rather than blindly catching them all. We can't know for certain how Kibana is going to react and will probably lead to very unexpected behavior (if working at all that is!)

[TinaHeiligers]

We can always consider a more global option to catch exceptions, as long as it's opt-in only.

[pgayvallet]

How would we handle intentional crashing errors, though?

So, I assume such errors would / should only be thrown from Core code, correct?

If so, we might be able to re-use the concept of "critical errors" and manually "flag" errors that should crash the process.

kibana/packages/core/root/core-root-server-internal/src/bootstrap.ts

Line 141 in dcba4d0

process.exit(reason instanceof CriticalError ? reason.processExitCode : 1);

OTOH, from the nodeJS documentation itself:

'uncaughtException' is a crude mechanism for exception handling intended to be used only as a last resort. The event should not be used as an equivalent to On Error Resume Next. Unhandled exceptions inherently mean that an application is in an undefined state. Attempting to resume application code without properly recovering from the exception can cause additional unforeseen and unpredictable issues.

So I agree with the previous statements from @lukeelmers and @TinaHeiligers that still crashing while properly logging the error cause / stack may be the best approach.

OTOOH, usually rxjs stack traces are not helpful at all, and don't have any context information of the original code / caller, so I wonder if logging the stack trace in such scenario would really help us in any way (aside from knowing for sure that an uncaught error was the cause of the process termination)

[afharo]

++ I agree with you all: at the moment, the best handling we can do is logging before we crash.