express process termination with async exceptions

[ljearyscg]

Is there an existing issue for this?

• I have checked for existing issues https://github.com/getsentry/sentry-javascript/issues
• I have reviewed the documentation https://docs.sentry.io/
• I am using the latest SDK release https://github.com/getsentry/sentry-javascript/releases

How do you use Sentry?

Sentry Saas (sentry.io)

Which package are you using?

@sentry/node

SDK Version

7.14.2

Framework Version

No response

Link to Sentry event

No response

Steps to Reproduce

1. create new express node project from https://github.com/sentry-demos/express
2. upgraded sentry to 7.14.2
3. added an async request with a reject

app.get('/unhandledasync', async function () {
    Promise.reject('failure to handle async')
});

4. add basic error handler that will respond with 501 and possibly send an error object back

app.use((error,_req,res,next) => {
    console.log('500')
    res
      .status(501)
      .send(error)
    next(error)
  })

5. call the async reject /unhandledasync using curl

this is a change in version 7.12
version 7.11 worked as expected

Expected Result

for sentry to call next when it received an error that is asynchronous and allow the following handler 'custom error handler) process the response

Actual Result

the exception is unhandled and the process is terminated.

when /unhandled is called the process works as expected

[AbhiPrasad]

Hey, thanks for writing in! We made the change in #5627 because routing the error to next is incorrect behaviour as per #1922 (comment) - Sentry should not be doing this.

Does the behaviour you want occur when you remove the Sentry library?

[ljearyscg]

@AbhiPrasad Hi yep looks like we were wrong in our implementation and it only worked as sentry was listening to the event.
We should have been catching the exception on the promise and calling next ourselves.

as per here https://reflectoring.io/express-error-handling/

app.get('/unhandledasync', async function (_,_,next) {
    Promise.reject('failure to handle async').catch(next)
});

thanks for replying.