Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cause of error is not sent to Sentry #747

Open
3 tasks done
nbolton opened this issue Sep 18, 2023 · 7 comments
Open
3 tasks done

Cause of error is not sent to Sentry #747

nbolton opened this issue Sep 18, 2023 · 7 comments

Comments

@nbolton
Copy link

nbolton commented Sep 18, 2023

Is there an existing issue for this?

How do you use Sentry?

Sentry Saas (sentry.io)

Electron SDK Version

4.11.0

Electron Version

22.3.14

What platform are you using?

Windows

Link to Sentry event

https://symless.sentry.io/issues/4485943131/events/a3e5813f0df746099578daa0ea1c40ed/

Steps to Reproduce

This bug is in both the Electron Sentry plugin and the React Sentry plugin (when React is used with Electron).

  1. Catch an error raised in either React front-end or Electron back-end
  2. Re-throw error and set cause (see below)
  3. Catch error and call Sentry.captureException(error)
  4. Print the error object using console.log and confirm that the cause is set and is type of Error

Example:

try {
  // i.e. call a function that throws
  throw Error("bar");
} catch (error) {
  // re-throw as new error with cause to add context
  throw Error("foo", { cause: error });
}

Expected Result

Sentry error unpacks the error chain and shows the error and cause separately (this works great with the Sentry Node plugin).

image

Actual Result

The cause is dropped (and also isn't actually found in the JSON sent to Sentry), indicating that both the Electron and React Sentry plugins don't use the cause.

image

@nbolton
Copy link
Author

nbolton commented Sep 18, 2023

I see that something was merged in to fix this, but I still see the problem in the latest version of the React and Electron plugins:
getsentry/sentry-javascript@8e78e6e

If this is actually fixed and I'm doing something wrong or am mistaken, then I'd appreciate knowing.

@timfish
Copy link
Collaborator

timfish commented Sep 18, 2023

this works great with the Sentry Node plugin

I still see the problem in the latest version of the React and Electron plugins

Do you mean that in the Electorn main process it's working as expected but not in the Electron renderer ptocesses?

The Electron SDK uses @sentry/node in the main process and @sentry/browser in the renderers. Both these SDKs use the LinkedErrors integration to handle mapping the casue.

How are you using the React and Electron SDKs together?

@nbolton
Copy link
Author

nbolton commented Sep 19, 2023

I mean that when sending the exception to Sentry directly (not over IPC) from React or Electron, the cause is lost.

In Electron, for example, printing the exception with console.log just before calling Sentry.captureException shows:

sending error data to sentry: {
  type: 'Error',
  name: 'Error',
  message: 'test rejection from gui back-end => [cause] test cause',
  stack: 'Error: test cause\n' +
    '    at throwTestCause (/path/to/gui/build/electron.js:1784:9)\n' +
    '    at throwTestRejection (/path/to/gui/build/electron.js:1804:5)\n' +
    '    at new Promise (<anonymous>)\n' +
    '    at testElectronError (/path/to/gui/build/electron.js:1827:3)\n' +
    '    at click (/path/to/gui/build/electron.js:1779:7)\n' +
    '    at MenuItem.click (node:electron/js2c/browser_init:2:30166)\n' +
    '    at a._executeCommand (node:electron/js2c/browser_init:2:35562)',
  cause: {
    type: 'Error',
    name: 'Error',
    message: 'test rejection from gui back-end',
    stack: 'Error: test rejection from gui back-end\n' +
      '    at throwTestRejection (/path/to/gui/build/electron.js:1807:11)\n' +
      '    at new Promise (<anonymous>)\n' +
      '    at testElectronError (/path/to/gui/build/electron.js:1827:3)\n' +
      '    at click (/path/to/gui/build/electron.js:1779:7)\n' +
      '    at MenuItem.click (node:electron/js2c/browser_init:2:30166)\n' +
      '    at a._executeCommand (node:electron/js2c/browser_init:2:35562)',
    cause: {
      type: 'Error',
      name: 'Error',
      message: 'test cause',
      stack: 'Error: test cause\n' +
        '    at throwTestCause (/path/to/gui/build/electron.js:1784:9)\n' +
        '    at throwTestRejection (/path/to/gui/build/electron.js:1804:5)\n' +
        '    at new Promise (<anonymous>)\n' +
        '    at testElectronError (/path/to/gui/build/electron.js:1827:3)\n' +
        '    at click (/path/to/gui/build/electron.js:1779:7)\n' +
        '    at MenuItem.click (node:electron/js2c/browser_init:2:30166)\n' +
        '    at a._executeCommand (node:electron/js2c/browser_init:2:35562)',
      cause: null
    }
  }
}
sentry event id: 92a866cfbe424f5e944c1319b2f08c5b

The type is added in by us for verbosity, to make sure we're actually sending the right types to Sentry.

But what appears in Sentry is much leaner:
Screenshot 2023-09-19 at 17 34 27

We created the flattened Error at the start of the chain to workaround this bug:
test rejection from gui back-end => [cause] test cause

@timfish
Copy link
Collaborator

timfish commented Sep 28, 2023

With the latest SDK and the following code:

index.js

const path = require("path");
const { inspect } = require("util");
const { app, BrowserWindow } = require("electron");
const { init } = require("@sentry/electron");

init({
  dsn: "__DSN__",
  debug: true,
  autoSessionTracking: false,
  onFatalError: () => {},
  beforeSend: (event) => {
    console.log(inspect(event, false, null, true));
    return event;
  },
});

app.on("ready", () => {
  const mainWindow = new BrowserWindow({
    webPreferences: {
      nodeIntegration: true,
      contextIsolation: false,
    },
  });

  mainWindow.loadFile(path.join(__dirname, "index.html"));
});

index.html

<!DOCTYPE html>
<html>
  <head>
    <meta charset="UTF-8" />
  </head>
  <body>
    <script>
      const { init } = require("@sentry/electron");

      init({
        debug: true,
      });

      setTimeout(() => {
        try {
          throw new Error("Something failed.");
        } catch (err) {
          throw new Error("Another thing failed.", { cause: err });
        }
      }, 500);
    </script>
  </body>
</html>

I get an event logged to the console with two exceptions:

   values: [
      {
        type: 'Error',
        value: 'Something failed.',
        stacktrace: {
          frames: [
            {
              filename: 'app:///index.html',
              function: '?',
              in_app: true,
              lineno: 16,
              colno: 17
            }
          ]
        },
        mechanism: {
          type: 'instrument',
          handled: false,
          source: 'cause',
          exception_id: 1,
          parent_id: 0,
          data: { function: 'setTimeout' }
        }
      },
      {
        type: 'Error',
        value: 'Another thing failed.',
        stacktrace: {
          frames: [
            {
              filename: 'app:///index.html',
              function: '?',
              in_app: true,
              lineno: 18,
              colno: 17
            }
          ]
        },
        mechanism: {
          type: 'generic',
          handled: true,
          is_exception_group: true,
          exception_id: 0
        }
      }
    ]

And this is displayed in Sentry like this:
image

This appears to be as expected.

Can you supply a full code example that reproduces your issue?

@nbolton
Copy link
Author

nbolton commented Oct 16, 2023

Still digging around with this but I've had a small breakthrough. We're also using the Sentry Node plugin (different repo) and are having similar issues.

In our REST service, we're calling:

app.use(Sentry.Handlers.requestHandler());

This was recently removed since we thought we weren't using it, and the breadcrumbs and cause stopped working for the rest of the app.

Curiously though, our handled override code magically started working again (after we stopped using the REST middleware).

    return Sentry.captureException(error, (scope) => {
      scope.addEventProcessor((event) => {
        addExceptionMechanism(event, {
          handled: handled,
        });
        return event;
      });
      return scope;
    });

The odd thing is, that turning the middleware on/off affects other areas of the application (i.e. things that have nothing to do with REST).

I'm left wondering: What is Sentry.Handlers.requestHandler doing that makes things magically work/stop working in other areas of the app unrelated to our REST server?

@nbolton
Copy link
Author

nbolton commented Oct 16, 2023

Update: Seems I have discovered the cause for both Node, Electron, and React.

We were setting enabled: false in Sentry.init and then later turning it on (when the customer consented to error reports) via Sentry.getCurrentHub().getClient()?.getOptions().enabled = true.

The solution was to leave the value of enabled alone, allowing it to be the default, which is true, and just not call the capture functions if the customer doesn't want to report errors.

We also disabled the OnUncaughtException and OnUnhandledRejection integrations, as we handle them ourselves.

    Sentry.init({
      // just for illustration, as `true` is the default value.
      enabled: true,

      integrations: (integrations) => {
        // remove the default integrations that catch unhandled exceptions and rejections,
        // since we're handling this ourselves.
        const newIntegrations = integrations.filter(
          (integration) =>
            integration.name !== "OnUncaughtException" &&
            integration.name !== "OnUnhandledRejection",
        );

        return newIntegrations;
      },
     );

I suppose this might be a different bug report, namely:
Setting enabled to false in Sentry.init and then re-enabling it via Sentry.getCurrentHub().getClient()?.getOptions().enabled = true causes super weird behavior.

@timfish
Copy link
Collaborator

timfish commented Oct 17, 2023

The Electron SDKs default transport handles offline queueing but also has some callbacks which can be used to create a more effective opt-in mechanism since events can be queued to disk until the user opts-in.

https://docs.sentry.io/platforms/javascript/guides/electron/#offline-support

In the beforeSend callback, return send, queue or drop depending in opt-in status.
The queuedLengthChanged callback could be used to notify users when there are queued events.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants