Very uninformative onunhandledrejection errors when rejecting with Event

[rchl]

• Review the documentation: https://docs.sentry.io/
• Search for existing issues: https://github.com/getsentry/sentry-javascript/issues
• Use the latest release: https://github.com/getsentry/sentry-javascript/releases
• Provide a link to the affected event from your Sentry account

Package + Version

• @sentry/browser
• @sentry/node
• raven-js
• raven-node (raven for node)
• other:

Version:

5.6.2

Description

Errors captured through onunhandledrejection mechanism never really provide any useful information that would help in figuring out the problem.

In my app, I have thousands (17K as of now for one of them, after few months) of UnhandledRejection errors. All info that those events provide is just a an error text like UnhandledRejection: "CustomEvent" (note that with latest version of Sentry it became a little bit more informative with UnhandledRejection: Non-Error promise rejection captured with keys: isTrusted text). There is no stack nor extra context and those errors happen on routes and paths that work perfectly for me and everyone I know so I'm unable figure out how to address them.

Now, I assume, that the problem is only with unhandled promise rejections that reject with non-Error type like primitive values (string, null, etc.) or (like in this case) an Event object. While I don't see how to improve cases with primitive values, I think for event objects, more data could be logged.

It seems the UnhandledRejection: Non-Error promise rejection captured with keys: isTrusted message comes from the fact that isTrusted is the only iterable key on the event object (try console.log(Object.keys(new CustomEvent('test')))). But event objects have more useful (non-iterable) keys that could provide more information about the error. Namely I'm thinking about detail and type properties. I think those could help identifying from which code event is coming from.

So I'm suggesting to add some custom code (if there isn't already) for handling that special case (unhandledrejection with event object) better.

Some example events:
https://sentry.io/share/issue/4d294f75e9704103b7c3b49e62b6e278/
https://sentry.io/share/issue/c36904ab4c224fbda6f1afff63c106ee/
(these are minimal, sharable versions but full events don't really provide any other meaningful information either.)

[rchl]

For the time being, I've added a beforeSend hook like:

beforeSend: function(event, hint) {
    if (hint && hint.originalException instanceof Event) {
        event.extra.isTrusted = hint.originalException.isTrusted;
        event.extra.detail = hint.originalException.detail;
        event.extra.type = hint.originalException.type;
    }

    return event;
},

BTW. detail is a property of CustomEvent, not Event, but for my purposes I don't care.

[ajhool]

I'm experiencing this, too, and I think that Sentry needs to handle non-Error types properly, rather than treating them like an error.

I'm still working through the problem and I tried adding this code to wrap objects in an Error type, but this didn't fix the problem:

  const wrappedError = (err instanceof Error || typeof err === 'string') ? err : new Error(JSON.stringify(err));
  Sentry.captureException(wrappedError);

Next, I'm going to try using Sentry.captureMessage if the instance type is not an Error, but a lot of third party libraries throw non-Error types that should be treated as Errors in Sentry IMO

I would actually disagree with the title of this issue -- I don't think that Sentry is being uninformative, I think it's actually informing developers that Sentry doesn't properly handle some types of common javascript errors (ie. thrown objects that aren't of instance type: Error). I see this as being an actual bug in Sentry, because a thrown Object in javascript is still an Exception/Error, even if the thrown object is not of Error type IMO. I'm not sure where the javascript community would stand on this, admittedly (https://stackoverflow.com/questions/9156176/what-is-the-difference-between-throw-new-error-and-throw-someobject)

[rchl]

What would it achieve to wrap non-error with Error? Are you expecting to get stack trace as a result? I I'm not sure that would work with native unhandledrejection handler. If it would provide meaningful stacktrace then I would agree with you that it would be worth doing. Otherwise I don't really see the point.

[ajhool]

Wrapping a non-Error in Error was just to get the correct format for compatibility and also typing.

I have experienced several third party libraries that throw objects that have some information about the error but the nature of the error is . It is somewhere between an event and an error.

For instance, Google Analytics throws this object:

 {
    details: Not a valid origin for the client: https://mywebsite.com has not been whitelisted for client ID myid.apps.googleusercontent.com. Please go to https://console.developers.google.com/ and whitelist this origin for your project's client ID.,
     error: idpiframe_initialization_failed
}

Which is an error that I'd like to know about in Sentry and it makes it all of the way to Sentry's captureException, but google throws it as an object instead of an Error (as far as I can tell), so Sentry doesn't handle it in the a way that is intuitive to me.

Javascript users should throw Errors, but the language allows them to throw plain objects, and sometimes they do. Those objects often contain error information that is helpful for debugging (the google example tells me everything I need to know) even without a stacktrace

A dynamic version of @rchl 's strategy would make sense to me -- iterate through the object keys and attach them to event.extra

[rchl]

With unhandledrejection errors, it's not really possible to get meaningful stack strace (I've just tried - stack starts at unhandledrejection handler). And if you don't have stack trace, the message is all that can be used for grouping (I guess). So you would probably get a lot of separate issues in Sentry because all stringified objects would differ slightly (for example with the URL). So I'm not sure that would be the best approach.

But your usecase is already handled in latest Sentry version. You get extra.__stringified__ field that contains stringified object. It doesn't work with Event types because those don't have any meaningful toJSON/toString implementation though... But it should work fine with plain objects.

[Neaox]

Sometimes the object/event has a message key, I would much prefer it if that were used for the wrapped exception message...

[cynicaloptimist]

I'm currently receiving thousands of instances of this error from a relatively small number of users. Since I'm just testing out Sentry right now with a free plan, there's no way to filter them out. Spike Protection also does not seem to react to this issue.

[ajhool]

@cynicaloptimist I hope this isn't pointing out the obvious, but a quick patch is just to filter them out on your client, assuming the errors are all the same type. In my case, it was a warning from social login buttons when I was in dev mode.

[cynicaloptimist]

@ajhool yep, I'll probably do that if the issue persists and I decide to keep using Sentry. I thought it might be helpful to report the impact to this Issue discussion.

[mogelbrod]

We're getting the same kind of UnhandledRejections reported in Sentry, even for errors that we know was instanceof Error when the error was thrown. We're adding the enumerable error properties to the extra object, and thus can verify that the errors which Sentry don't detect as being proper Errors in fact are that.

 beforeSend(event, hint) {
  event.extra = event.extra || {}

  if (hint.originalException != null) {
    // Assign enumerable error properties to extras
    try {
      const keys = Object.keys(hint.originalException)
      if (keys.length) {
        event.extra.errorProperties = {}
        keys.forEach(key => {
          event.extra.errorProperties[key] = hint.originalException[key]
        })
      }
    } catch (error) {
      console.warn('[sentry] Failed to assign enumerable error properties to extras', error)
    }
  }

  return event
}

These errors are displayed like this in the Sentry UI:

The extras for these errors look like the following:

{
  '__serialized__': '**non-serializable**',
  'errorProperties': {
    description: '....',
    stack: '....',
    // additional properties set on error
  }

As you can see the Sentry serialization (__serialized__) appears to fail, while our own beforeSend() code is successful in serializing the error properties.

[kamilogorek]

I scheduled better Event/CustomEvent handling work for the next week. Will keep everyone posted on this one.

@mogelbrod is there a way you can provide a repro-case for this? We never had an issue with TraceKit incorrectly detecting Error instances.

As for your additional enumerables, we have an integration for this – https://docs.sentry.io/platforms/javascript/#extraerrordata

import * as Sentry from '@sentry/browser';
import { ExtraErrorData } from '@sentry/integrations';

Sentry.init({
  integrations: [new Integrations.ExtraErrorData()]
});

is basically equivalent of your beforeSend snippet.