Feature: Support cause property on errors

[Turbo87]

Do you want to request a feature or report a bug?

feature

What is the current behavior?

try {
  somethingThatThrows();
} catch (innerError) {
  let error = new Error('something failed!');
  error.cause = innerError;
  throw error;
}

With the current behavior only the outer/wrapping Error is shown in the stack trace.

What is the desired behavior?

It would be great if the inner Error was also displayed in the stacktrace view, roughly similar to how it is done for Java and Python.

I'm happy to help implement this with a bit of guidance :)

• does the serverside need changes or does it already support this due to support in Java?
• what changes to the submitted payload are necessary?

[kamilogorek]

Can be done with event hints in new SDK now

[Turbo87]

@kamilogorek can you explain what you mean by "event hints" and how that will work with the cause property?

[kamilogorek]

Event and Breadcrumb hints are objects containing various information used to put together an event or a breadcrumb. For events, those are things like event_id, originalException, syntheticException (used internally to generate cleaner stacktrace), and any other arbitrary data that user attaches. For breadcrumbs it's all implementation dependent. For XHR requests, hint contains xhr object itself, for user interactions it contains DOM element and event name etc.

They are available in two places. beforeSend/beforeBreadcrumb and eventProcessors. Those are two ways we'll allow users to modify what we put together.

Examples based on your cause property (I use message for ease of reading, but there's nothing stopping you from modifying event stacktrace frames).

beforeSend/beforeBreadcrumb:

import { init } from '@sentry/browser';

init({
  dsn: 'https://some@dsn.com/123',
  beforeSend(event, hint) {
    const processedEvent = { ...event };
    const cause = hint.originalException.cause;

    if (cause) {
      processedEvent.message = cause.message;
    }

    return processedEvent;
  },
  beforeBreadcrumb(breadcrumb, hint) {
    if (breadcrumb.category === 'ui.click') {
      const target = hint.event.target;
      if (target.ariaLabel) breadcrumb.message = target.ariaLabel;
    }
    return breadcrumb;
  },
});

eventProcessor (this will be not used that often, but is great for writing custom plugins or share them across multiple projects - in form of an integration, more on this soon):

import { getCurrentHub } from '@sentry/browser';

getCurrentHub().configureScope(scope => {
  scope.addEventProcessor(async (event, hint) => {
    const processedEvent = { ...event };
    const cause = hint.originalException.cause;

    if (cause) {
      processedEvent.message = cause.message;
    }

    return processedEvent;
  });
});

[Turbo87]

hmm, if I understand correctly this would overwrite the original exception though, right? my hope was that it could display them in parallel like in Java/Python 🤔

[kamilogorek]

Can you show me an example of desired output?

[Turbo87]

This is an example of how it looks like in Java:

Exception in thread "main" java.lang.IllegalStateException: A book has a null property
        at com.example.myproject.Author.getBookIds(Author.java:38)
        at com.example.myproject.Bootstrap.main(Bootstrap.java:14)
Caused by: java.lang.NullPointerException
        at com.example.myproject.Book.getId(Book.java:22)
        at com.example.myproject.Author.getBookIds(Author.java:36)
        ... 1 more

---

try {
  somethingThatThrows();
} catch (innerError) {
  let error = new Error('something failed!');
  error.cause = innerError;
  throw error;
}

Without the cause assignment in the snippet above you lose any information on the original cause of the exception, but if you throw the innerError it might be cryptic for the end user and you can't include other information that might help resolve the issue. Ideally it would be possible to show both inner and outer exception (and possible even more nesting levels) like in the Java example above.

[kamilogorek]

The platform itself doesn't support this use-case, so there's only this much we can do on the SDK side. In JS, there's only one "main" exception, and we don't have an easy way to display more.

I'd add this as an extra data in the event itself, which'd give you enough information on the UI:

beforeSend(event, hint) {
  const processedEvent = { ...event };
  const cause = hint.originalException.cause;
 
 if (cause) {
    processedEvent.extra = {
      ...processedEvent.extra,
      cause: {
        type: cause.type
        message: cause.message,
        stack: cause.stack
      }
    }
  }

  return processedEvent;
}

[Turbo87]

I'd add this as an extra data in the event itself, which'd give you enough information on the UI

yeah, unfortunately the cause.stack will be displayed without sourcemaps though :-/

The platform itself doesn't support this use-case

do you not support this for Java/Python either?

[kamilogorek]

@Turbo87 like this? :)

[Turbo87]

exactly! 😍

I'm not sure, but I think the order needs to be flipped though. I'd expect to see the "Parent process error" on top of the other one 🤔

[kamilogorek]

It's PoC so far. I'll make it happen before the final release :)

[Turbo87]

Great, thanks a lot!!

[Turbo87]

@kamilogorek did this land yet?

[Turbo87]

nevermind, I just found https://github.com/getsentry/sentry-javascript/blob/4.2.3/packages/browser/src/integrations/linkederrors.ts 🎉

[lily-controlzee]

Is a .cause property (standardized in https://github.com/tc39/proposal-error-cause) supported out of the box, or does one have to set it up to report as extra data?

[kamilogorek]

@lily-controlzee yes, it's a default, out-of-the-box integration (called LinkedErrors).

sentry-javascript/packages/browser/src/sdk.ts

Lines 24 to 33 in 11bb8ed

	export const defaultIntegrations = [
	new CoreIntegrations.InboundFilters(),
	new CoreIntegrations.FunctionToString(),
	new TryCatch(),
	new Breadcrumbs(),
	new GlobalHandlers(),
	new LinkedErrors(),
	new Dedupe(),
	new HttpContext(),
	];

[lily-controlzee]

Thanks!

[public]

The TC39 error-cause proposal does not define a type for cause, and in fact in real implementations you can attach whatever metadata you want as a "cause" and in TypeScript it's defined as any.

LinkedErrors just gives up if the cause is not an Error instance. I would expect it to capture non-Error cause values in some way rather than just chucking them out.

[Turbo87]

the same is true for throw. you can throw anything, not just Error instances. but in reality it is highly recommended to only use it with Error, and IMHO the cause field should be treated similarly.

[public]

Sentry supports throwing non-Error objects just fine so I think that is actually a good example of why it should work with non-Error cause values.

If you think this shouldn't be supported please take it up with TC39 (someone should, exceptions in JS are...not great) but since the spec and implementations allow it, people are going to do this even in code I don't control.

[ljharb]

cause can be anything by design. We even made sure you can differentiate between a cause being undefined or entirely absent.

The recommendation to throw an Error is because of stack information, and you don't always need stack information. All tooling and type systems for JS should acknowledge and allow any value to be thrown or caught, or set in errors or cause.

[Merott]

Yes, would love to be able to attach more info to the cause property.

Here's an example where I'm attaching the original error inside a custom object to capture more info:

try {
  const json = await res.json()
} catch (error) {
  throw new Error('Something went wrong!', {
    cause: {
      problem: `Non-JSON response from GraphQL endpoint: ${GRAPHQL_PATH}`,
      responseBody: res.body,
      originalError: error,
    },
  })
}

[kamilogorek]

@Merott you can already do that for years now :)

https://docs.sentry.io/platforms/javascript/configuration/integrations/plugin/#extraerrordata

[daniel-nagy]

I believe the React error boundary feature is breaking the Error chain.

sentry-javascript/packages/react/src/errorboundary.tsx

Lines 84 to 91 in fd4db0d

	if (isAtLeastReact17(React.version) && isError(error)) {
	const errorBoundaryError = new Error(error.message);
	errorBoundaryError.name = `React ErrorBoundary ${errorBoundaryError.name}`;
	errorBoundaryError.stack = componentStack;
	// Using the `LinkedErrors` integration to link the errors together.
	error.cause = errorBoundaryError;
	}

I guess it should set

errorBoundaryError.cause = error.cause;

before overwriting the error cause.

Edit

I'm confused why it's not just creating a new error and setting the cause of the new error equal to the original error? Something like

const boundaryError = new ReactBoundaryError("...");
boundaryError.cause = error;
captureException(boundaryError, ...);

[AbhiPrasad]

I believe the React error boundary feature is breaking the Error chain.

Yup, you're right that we overwrite the error cause in this case, which we shouldn't. We set the cause this way because we want grouping to work off of the original error, not the synthetic componentStack error.

Let me see if this is a situation we can improve though

[slorber]

Any update @AbhiPrasad ?

Was also wondering if the Sentry UI supports rendering error cause properly? (including nested stacktraces?)

[kamilogorek]

Was also wondering if the Sentry UI supports rendering error cause properly? (including nested stacktraces?)

Yup, it should work just fine by default, as SDK has LinkedErrors integration that translates cause chain into multiple separate errors in a single event.

[AbhiPrasad]

@slorber opened a PR here: #7557