Improve `EventHint` typing to account for error-like objects

[pfdzm]

Is there an existing issue for this?

• I have checked for existing issues https://github.com/getsentry/sentry-javascript/issues
• I have reviewed the documentation https://docs.sentry.io/
• I am using the latest SDK release https://github.com/getsentry/sentry-javascript/releases

How do you use Sentry?

Sentry Saas (sentry.io)

Which SDK are you using? If you use the CDN bundles, please specify the exact bundle (e.g. bundle.tracing.min.js) in your SDK setup.

@sentry/vue

SDK Version

7.35.0

Framework Version

No response

Link to Sentry event

https://afilio.sentry.io/issues/3897188605/events/30d6c162b1844dbd912886cab3b414e0/

SDK Setup

{
  enabled: environment !== "development" && !browserCheckFailed,
  dsn: [redacted],
  tunnel: "/track-error",
  integrations: [
    new BrowserTracing(),
    new Sentry.Replay({
      maskAllText: false,
    }),
  ],
  tracesSampleRate: 0.01,
  release: process.env.CODE_VERSION,
  environment,
  ignoreErrors: [
    /Non-Error promise rejection captured with value: Object Not Found Matching Id:\d+/,
  ],
  beforeSend: (event, hint) => {
    const cap = levelCap(hint);
    const levels: Sentry.SeverityLevel[] = [
      "debug",
      "info",
      "warning",
      "error",
      "fatal",
    ];
    if (levels.indexOf(cap) < levels.indexOf(event.level ?? "fatal")) {
      event.level = cap;
    }
    return discardChromeExtensionErrors(enrichWithAdditionalData(event, hint));
  },
  beforeBreadcrumb: (breadcrumb) => {
    if (breadcrumb.category === "fetch" || breadcrumb.category === "xhr") {
      const ignoredHosts = ["redacted", "redacted"];
      const url = new URL(expectString(breadcrumb.data?.url));
      if (ignoredHosts.includes(url.hostname)) {
        return null;
      }
    }
    return breadcrumb;
  },
  replaysSessionSampleRate: 0.1,
  replaysOnErrorSampleRate: 1.0,
}

Steps to Reproduce

The type definitions provided for beforeSend or more specifically the EventHint's originalException say it will be string | Error | undefined | null. However, we have errors from our beforeSend hook where we apply a "level cap" (i.e. downgrade some error levels) based on the error name (please see the linked event).

Expected Result

I would expect the name property of an Error to always exist (https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Error/name) - otherwise the types should be updated to reflect what can actually happen.

Actual Result

name of an Error is undefined, causing an uncaught exception to happen in our beforeSend hook.

[AbhiPrasad]

Hey @pfdzm could you provide the implementation of the levelCap function?

[pfdzm]

@AbhiPrasad sure, sorry, here you go (abbreviated, the error is thrown here already, specifically when doing error.name.includes towards the end):

function levelCap(eventHint: Sentry.EventHint): Sentry.SeverityLevel {
  const error = eventHint.originalException ?? null;
  if (error === null || typeof error === "string") {
    return "fatal";
  }
  if (error.name.includes("NS_ERROR_")) {
    return "info";
  }
}

[AbhiPrasad]

Yeah the type is inaccurate here - we need to improve it.

This breaks in the case of the following:

function myFunc() {
  throw { key: "val" };
}

try {
  myFunc();
} catch (e) {
  // logs out { key: 'val' }
  console.log(e);
}

In this case the originalException is an object, which is valid js that may written. You probably need to add a guard to enforce that the original exception is an error object, like with a instanceof Error check.

[pfdzm]

Thank you @AbhiPrasad for the repro 🙂 We are dealing with it in a manner similar to your suggestion, but also agree that the types should be improved.

[mydea]

In reality, I fear the type of this has to be unknown as it could be anything. We'll update this to reflect the reality of this type!