Provide fingerprint on error object

[jeengbe]

Problem Statement

It is not possible for Sentry to always group errors correctly, as "correct" is subjective to the developer. If Sentry provides an option to enrich a thrown error object with a fingerprint, the developer is in full control over the fingerprint.

• Either Sentry groups to aggressively and groups errors that don't belong together -> Different errors (conceptually) that share the same stack trace.
• Conceptually the same errors that originate from different call stacks.

The current solutions has the following issues:

• scope.setFingerprint only works if the error is captured in the scope in which it occurs. If, instead, the error is thrown and caught in one central place, all scope information (and thus the fingerprint) is lost.
• beforeSend adds needless complexity by requiring both copy-pasta setup code for every project, and at least one custom class that must also be copied alongside.

Solution Brainstorm

Imagine the following simplified code snippet:

export async function getThing() {
  const res = await fetch('https://api.example.com/thing');

  if (!res.ok) {
    throw new Error(
      `Failed to fetch the thing: ${res.status} ${res.statusText}.`,
      {
        cause: {
          status: res.status,
          statusText: res.statusText,
          body: await res.text(),
        },
      },
    );
  }

  try {
    return await res.json();
  } catch (err) {
    throw new Error('Failed to fetch the thing: invalid JSON.', {
      cause: {
        body: await res.text(),
      },
    });
  }
}

Any "Failed to fetch the thing: {status} {text}" error would be grouped as one issue by the same stack trace. There are several solutions to this:

• Sentry.captureException instead of throwing the error. This, however, is not always an option. If there is no logger instance available, there is no way to otherwise log the error, and depending on the project, the option return types this solution implies, don't always work.
• Attach fingerprint to the error object:
  Either with an error derivate that Sentry provides and attaches fingerprint from a constructor parameter, or alternatively:

throw Object.assign(
  new Error(`Failed to fetch the thing: ${res.status} ${res.statusText}.`, {
    cause: {
      status: res.status,
      statusText: res.statusText,
      body: await res.text(),
    },
  }),
  {
    fingerprint: ['fetch-thing-failed', res.status],
  },
);

Then, the Sentry SDK would check for .fingerprint properties on errors out of the box.

• Attach the scope to any thrown errors. The initial problem is that any errors thrown within a scope lose any context. This could be fixed with a try-catch within Sentry.withScope that simply attaches itself to any errors it catches, so that any higher-level error handler still has access to the scope in which the error occurred. This would be the best solution in my opinion, as it also retains any other metadata set on the scope, such as tags and data, and makes those available to Sentry, as well as any other error logger, such as >stderr, which could then also benefit from scope data.

[AbhiPrasad]

Thanks for the really detailed writeup! I guess the decision here is if we want to do the following:

Then, the Sentry SDK would check for .fingerprint properties on errors out of the box.

Right now you can already attach arbitrary properties to an error, and then in beforeSend set event.fingerprint accordingly based on the originalException in the hint.

My worry with adopting an out of the box default is that server-side generated fingerprints are right now dependent on stacktraces. Isolating this to just a error object potentially introduces a footgun that degrades the issue experience

Also we have some grouping improvements lined up! See: getsentry/sentry#66319

[jeengbe]

Isolating this to just a error object potentially introduces a footgun that degrades the issue experience

What do you mean with this?

Unless you willingly set .fingerprint on an error, nothing changes at all. Only if you do so, a fingerprint is already set, otherwise, the stack trace is still present.

[jeengbe]

Also we have some grouping improvements lined up! See: getsentry/sentry#66319

Great discussion you've linked there, however, I feel that's only partly applicable here. This issue is fully about enabling developers to provide custom fingerprints, as in the end, we decide what is considered the same issue.

[lforst]

I personally generally like the idea, but I am not fully convinced that we should add it to the SDK by default.

@jeengbe I second what Abhi was saying that you can pretty easily implement this yourself right now with a simple combination between setting a fingerprint property on the error and then reading it off and populating event.fingerprint in beforeSend. I see your point about it being potentially a lot of duplicated code though.

I wanna let this simmer a bit. This is something we can add at any time. Maybe it is not high prio right now. But thanks again for the idea and feedback. Much appreciated!

[jeengbe]

Perhaps focus the discussion on the last point;

Attach the scope to any thrown errors. The initial problem is that any errors thrown within a scope lose any context. This could be fixed with a try-catch within Sentry.withScope that simply attaches itself to any errors it catches, so that any higher-level error handler still has access to the scope in which the error occurred. This would be the best solution in my opinion, as it also retains any other metadata set on the scope, such as tags and data, and makes those available to Sentry, as well as any other error logger, such as >stderr, which could then also benefit from scope data.

The benefit with this would be that no new classes/properties/API would be required, and all other and future features a scope provides would come out of the box too. Might be worth to port this to a separate issue/discussion, even.

[Lms24]

The initial problem is that any errors thrown within a scope lose any context

I think this is a great point and I actually had to write some tests to believe that this is in fact the current behavior (see #11204) 😅 I'll take your suggestion about attaching the scope to the error back to the team. I think it sounds good but there are probably edge cases where this isn't a great idea.

[Lms24]

Following up on this issue: As discussed in getsentry/sentry-python#2857, general consensus across SDKs is that for the moment we're not going to attach things to the error object but instead keep scope validity as we currently do.

I know this is not the outcome you were looking for but for now my best advice would be to take care of this yourself in the code. as suggested in the comments above. Thanks though for making us/me aware of this still somewhat unintuitive behaviour. Closing this issue for the time being. Please feel free to ping me if you want to see this reopened. Thanks!