getsentry / sentry-javascript

Official Sentry SDKs for JavaScript
https://sentry.io
MIT License
7.97k stars 1.57k forks source link

Calling Sentry.init in Electron will result in an error. #14133

Open HaoZhouInRC opened 1 day ago

HaoZhouInRC commented 1 day ago

Is there an existing issue for this?

How do you use Sentry?

Sentry Saas (sentry.io)

Which SDK are you using?

@sentry/browser

SDK Version

8.27.0

Framework Version

No response

Link to Sentry event

No response

Reproduction Example/SDK Setup

sentry.init({

})

Steps to Reproduce

  1. run in electron
  2. init sentry

Expected Result

Can be initialized normally.

Actual Result

Image

Image

Cause by #12668

Electron chrome runtime https://www.electronjs.org/docs/latest/api/extensions#chromeruntime

Lms24 commented 1 day ago

Hi, can you confirm that you're using @sentry/electron and you followed our electron docs? https://docs.sentry.io/platforms/javascript/guides/electron/

HaoZhouInRC commented 1 day ago

Hi, can you confirm that you're using @sentry/electron and you followed our electron docs? https://docs.sentry.io/platforms/javascript/guides/electron/

My page can run in both Electron and Chrome, so I believe I should use @sentry/browser.

And I am collecting issues from web pages, not from Electron.

chargome commented 1 day ago

@HaoZhouInRC can you give us more context on your setup? Does your app run inside of a browser extension? Please also share the config of your init call.

HaoZhouInRC commented 1 day ago
Sentry.init({
    "environment": "develop",
    "dsn": "xxxxxx",
    "debug": false,
    "release": "24.4.20",
    "ignoreErrors": [],
    "maxBreadcrumbs": 50,
    "integrations": [Breadcrumbs],
    "transportOptions": {},
    "initialScope": {"tags": {"desktopRelease": "24.3.360"}}
})

My application is a React web page that can run in Electron or a browser.

@chargome

HaoZhouInRC commented 1 day ago

Add an additional question: How does Sentry capture web page crashes?

Lms24 commented 19 hours ago

My application is a React web page that can run in Electron or a browser.

I'm afraid I still don't fully understand why this is a problem in your case and not generally in Electron apps. Do you have a special electron configuration that sets the window.chrome.runtime properties? I think there must be some additional configuration that causes this situation because our Electron SDK also calls browserInit() when initializing the renderer SDK: https://github.com/getsentry/sentry-electron/blob/f4ea1213fc77dd131519bda51aa1471c77d586d6/src/renderer/sdk.ts#L44

How does Sentry capture web page crashes

Do you mean full crashes where the browser shows that the tab crashed? Short answer: We can't report these unfortunately.

In case you mean, how do we catch errors? By listening to window.onerror in our globalHandlersIntegration.

Lms24 commented 19 hours ago

That being said, we previously already discussed if we should provide an escape hatch for the browser extension check because it already triggered falsely multiple times. This is probably more bundle-size efficient than adding yet another special case in this check.

HaoZhouInRC commented 15 hours ago

Background

First, let's describe the situation of our application. Our app is a web page developed using React, which can run directly in the browser.

However, to expand the capabilities of the app, we have also developed an Electron client for it. This client directly loads our web page for users to use, and at the same time, we can provide more native system capabilities to the web page.

Electron Supports Some Chrome Extension Capabilities

As described in this document https://www.electronjs.org/docs/latest/api/extensions#chromeruntime, web pages opened in Electron will have corresponding APIs injected into window.chrome.

Reason for the Bug

const windowWithMaybeExtension =
    typeof WINDOW.window !== 'undefined' && (WINDOW as typeof WINDOW & ExtensionProperties);
  if (!windowWithMaybeExtension) {
    // No need to show the error if we're not in a browser window environment (e.g. service workers)
    return false;
  }

  const extensionKey = windowWithMaybeExtension.chrome ? 'chrome' : 'browser';
  const extensionObject = windowWithMaybeExtension[extensionKey];

  const runtimeId = extensionObject && extensionObject.runtime && extensionObject.runtime.id; // Can read runtimeId
  const href = (WINDOW.location && WINDOW.location.href) || ''; // href is a web page starting with https://

  const extensionProtocols = ['chrome-extension:', 'moz-extension:', 'ms-browser-extension:', 'safari-web-extension:'];

  // Running the SDK in a dedicated extension page and calling Sentry.init is fine; no risk of data leakage
  const isDedicatedExtensionPage = // This is also false
    !!runtimeId && WINDOW === WINDOW.top && extensionProtocols.some(protocol => href.startsWith(`${protocol}//`));

  // Running the SDK in NW.js, which appears like a browser extension but isn't, is also fine
  // see: https://github.com/getsentry/sentry-javascript/issues/12668
  const isNWjs = typeof windowWithMaybeExtension.nw !== 'undefined'; // This is false

  return !!runtimeId && !isDedicatedExtensionPage && !isNWjs; // Overall is true
HaoZhouInRC commented 15 hours ago

That being said, we previously already discussed if we should provide an escape hatch for the browser extension check because it already triggered falsely multiple times. This is probably more bundle-size efficient than adding yet another special case in this check.

This can be considered a solution, thank you.

Lms24 commented 12 hours ago

I'm still very surprised that window.chrome.runtime.id this is available by default in electron. This would mean that currently the entire renderer SDK for @sentry/electro users would not initialize. To me the docs you linked convey that these APIs/variables are available for extensions running in an electron app (primarily dev tools), not for the app itself (?).

Gonna tag @timfish - any ideas how this could happen?

In other news, I opened and merged https://github.com/getsentry/sentry-javascript/pull/14147 which will let you bypass the check so that you can call Sentry.init. Unless we detect that our current logic actually fails in electron (which I doubt), I think we can close this issue once the next SDK version is released.

timfish commented 11 hours ago

I haven't seen this before with the Electron SDK and it doesn't affect any of our tests and examples.

@HaoZhouInRC are you using any chrome extensions?