getsentry / sentry-javascript

Official Sentry SDKs for JavaScript
https://sentry.io
MIT License
7.97k stars 1.57k forks source link

Chained exceptions not picked up (verror/nerror) #4180

Closed francisdb closed 2 years ago

francisdb commented 2 years ago

There are some libraries/implementations that allow chaining of javascript errors. These error causes are currently not picked up by sentry node.

Libraries https://github.com/joyent/node-verror https://github.com/Netflix/nerror

Implementations V8 v9.3 https://v8.dev/features/error-cause

It would be great if sentry could also pick up the traces of the chained errors those libraries implement If this is not an option some example code would be welcome on how to manually send this info to sentry.

Package + Version

Version:

6.15.0

Description

Sentry would have to check if errors have a function cause() or property cause defined and also process those (like with java)

Notes

I found an old similar ticket but it's unclear from the response if this is now supported and how it can be enabled. https://github.com/getsentry/sentry-javascript/issues/1381

AbhiPrasad commented 2 years ago

We do have the LinkedErrors integration that should do this for all errors. Do you have this enabled? See the Sentry docs also.

AbhiPrasad commented 2 years ago

Ah upon further review of those libraries, it seems they use .cause() as you mentioned, they don't follow the standardized proposal in https://v8.dev/features/error-cause that LinkedErrors takes advantage of.

You could probably make an integration similar to how LinkedErrors work but instead of doing this:

https://github.com/getsentry/sentry-javascript/blob/821857807c4b3d1e26f3d015b23f63c313da31b8/packages/node/src/integrations/linkederrors.ts#L84

You would:

      // make sure to validate that error has the .cause() functionality.
      void getExceptionFromError(error.cause())
francisdb commented 2 years ago

Thanks, missed that in the docs. So a (not so clean) workaround would be something like err.cause = err.cause() before sending to sentry which will then be picked up by the default integration? (needs recursion)

francisdb commented 2 years ago

Any chance that the current implementation could be broadened to check for functions? If not I think this can be closed.

AbhiPrasad commented 2 years ago

Don't think we will currently pursue broadening the current implementation (as .cause() doesn't follow the spec). Users are free to make their own integration to make it work for their use case.

Closing as such. Please open another issue if there is anything else you need :)