ignoreErrors ignores the wrong linked error

[lobsterkatie]

In cases where there are linked errors, the event.exception.values array contains the errors in chronological order: first the cause error and then the main error:

and indeed, this is how they come through in the resulting JSON:

But in the UI, the order is reversed:

Also, it is the second error which shows up as the title:

This suggests that we should be filtering on the second error, since everything in the UI points to it being the "main" one. But for ignoreErrors, when we're getting the strings to compare against the ignored values, we take the first error, not the second:

sentry-javascript/packages/core/src/integrations/inboundfilters.ts

Line 156 in b624bff

const { type = '', value = '' } = (event.exception.values && event.exception.values[0]) || {};

Two options for fixing this:

1. Take the last error instead of the first error. For any event with only one (which is most of them), the first and the last are the same, so nothing would change. For events with multiple errors, we'd be filtering on the right one, albeit a different one than we've been filtering on.
2. Check all of the errors in event.exception.values. This has the advantage of being backwards compatible (errors which are currently being ignored will continue to be ignored), but might be less intuitive behavior.

I'm happy to make a PR for either solution. Which do y'all think is best?

(For context, this is coming up because we're trying to be better dogfooders of the SDK in the Sentry FE, and go through the process of de-noising our JS project. In order to do that, it'd be nice to be able to use ignoreErrors, but it doesn't currently filter in the way we need (and in the way, IMHO, it should).)

[Lms24]

Hey @lobsterkatie great to hear from you! :)

So we discussed this and and we just need some clarification around how these linked errors show up in Sentry. Can we assume that for an event with multiple errors (i.e. event.exception.values.length > 1) always only one Sentry issues is created?

Then I think we should go with option 1 and match the top error, i.e. the last one in the array. If this one matches, the entire event should be dropped. I think this is most the intuitive approach for users. Obviously this is "behaviourally" breaking but I think this can be justified as we're essentially fixing a bug.

In case our assumption doesn't hold up and multiple issues are created for such events (I think this is unlikely but just to be sure), we should iterate through the individual exceptions and only drop those that match entries in ignoreErrors.

Will bring this up at our Daily today, too.

[lobsterkatie]

Yup - it only ever makes one event. Here is a real-life version and here is the event shown in the screenshots, which came from shoving the following

const err = createRequestError(
  {
    getResponseHeader: (_header: string) => null,
    responseJSON: '',
    responseText: 'hi',
    status: 401,
    statusText: 'nope',
  },
  new Error('this error is the cause'),
  'GET',
  '/dogs/are/great'
);
Sentry.captureException(err);

(which uses the helper defined here) into initializeSdk.tsx, right after our call to Sentry.init().

In any case, great, and thanks for the input. Filtering on the last/main error is the way I already did it in order to test stuff out, so I can put up a PR today.

Cheers!