getsentry / sentry-electron

The official Sentry SDK for Electron
https://sentry.io/
MIT License
224 stars 58 forks source link

Electron-React ;npm run package' throws a Token error, unable to package. #805

Open finnlaymfarmor opened 8 months ago

finnlaymfarmor commented 8 months ago

Is there an existing issue for this?

How do you use Sentry?

Sentry Saas (sentry.io)

Electron SDK Version

4.15.1

Electron Version

25.0.1

What platform are you using?

Windows

Link to Sentry event

No response

Steps to Reproduce

  1. Setup sentry as per the electron wizard
  2. On your main.ts file initialize sentry
  3. In CLI run 'npm run package' We are using electron-react-boilerplate. Have only setup sentry for main.ts to replicate this.

Expected Result

Application to package. For reference, the setup sentry.init works in development environment, but cannot seem to package into a production environment .

Actual Result

npm run package

package ts-node ./.erb/scripts/clean.js dist && npm run build && electron-builder build --publish never

build concurrently "npm run build:main" "npm run build:renderer"


[0] 
[0] > build:main
[0] > cross-env NODE_ENV=production TS_NODE_TRANSPILE_ONLY=true webpack --config ./.erb/configs/webpack.config.main.prod.ts
[0]
[1] 
[1] > build:renderer
[1] > cross-env NODE_ENV=production TS_NODE_TRANSPILE_ONLY=true webpack --config ./.erb/configs/webpack.config.renderer.prod.ts
[1]
[0] ERROR in ./src/main/main.ts + 219 modules
[0] Unexpected token (400:99)
[0] |     // parse node.js major version
[0] |     // Next.js will complain if we directly use `proces.versions` here because of edge runtime.
[0] |     const [major] = __WEBPACK_MODULE_REFERENCE__145_5b22474c4f42414c5f4f424a225d_call_asiSafe1__._ ).process.versions.node.split('.').map(Number);
[0] |
[0] |     // allow call extractOriginalRoute only if node version support Regex d flag, node 16+
[0] while analyzing module C:\Users\finnm\repos\Fluency-FE\node_modules\@sentry-internal\tracing\esm\node\integrations\express.js for concatenation   
[0]
[0] webpack compiled with 1 error
[0] npm run build:main exited with code 1
[1] npm run build:renderer exited with code 0
lforst commented 8 months ago

Hi, this looks like a bug in your webpack configuration. There is indeed a syntax error in the output in the terminal but this is not code that is in our npm package. Please check that you don't have webpack plugins that do anything funky.

finnlaymfarmor commented 8 months ago

@lforst thanks for swift reply. I will have a look. This only happens after I've installed sentry, for reference.

timfish commented 8 months ago

We have an electron-react-boilerplate example that is built and tested for our integration tests although this does not test a packaged build: https://github.com/getsentry/sentry-electron/tree/master/examples/electron-react-boilerplate

finnlaymfarmor commented 8 months ago

@timfish yep used as reference, works fine in dev. It's on package it fails

timfish commented 8 months ago

As far as I can see, the package script just runs the build and then runs Electron builder to package the app.

It looks like the following error comes from this code: https://github.com/getsentry/sentry-javascript/blob/679e149495190b682131aa1575cb96c5f4efcd2a/packages/tracing-internal/src/node/integrations/express.ts#L491

[0] |     // Next.js will complain if we directly use `proces.versions` here because of edge runtime.
[0] |     const [major] = __WEBPACK_MODULE_REFERENCE__145_5b22474c4f42414c5f4f424a225d_call_asiSafe1__._ ).process.versions.node.split('.').map(Number);

Our ERB example does not use Next.js. Do you have an example repository showing exactly the code you are trying to compile?

If you're using Next.js, how are you using the server side stuff with Electron?

finnlaymfarmor commented 8 months ago

Hi @timfish We aren't using next. If I change the const[major] variable to directly reference the major node version number, being 18, it packages fine.

  if (!lrp) {
    // parse node.js major version
    // Next.js will complain if we directly use `proces.versions` here because of edge runtime.
    // const [major] = (GLOBAL_OBJ ).process.versions.node.split('.').map(Number); **// Changing this**
    const [major] = 18; // ** // To this**

    // allow call extractOriginalRoute only if node version support Regex d flag, node 16+
    if (major >= 16) {

This is definitely a bandaid solution I assume as we would like to be exposed to all new versions of sentry. The above change is in express.js from the sentry files

lforst commented 8 months ago

@finnlaymfarmor is this happening when you run our example without making any changes or in your own setup?

ofarnill commented 8 months ago

Hi I'm working with @finnlaymfarmor. Your build seems to work with npm run build. We are attempting npm run package per the ERB boilerplate. Sentry installed with the wizard using webpack config as prompted. I can't see anything related to sentry in your webpack config files, which method of installation have you used? Thanks.

lforst commented 8 months ago

@ofarnill What exact wizard did you use to set up Sentry? The examples were last touched 2 years ago and the wizard most certainly is newer than that.

ofarnill commented 8 months ago
npx @sentry/wizard@latest -i sourcemaps

image

Then following the prompts for webpack config. Thanks.

lforst commented 8 months ago

The wizard will not set up the entire SDK for you, just how to upload sourcemaps for a given setup. I honestly 100% think this not an issue within the SDK, our Wizard, or the example, but rather a webpack bug. https://github.com/getsentry/sentry-javascript/issues/10048 reports a similar issue. Would you mind raising this over at https://github.com/webpack/webpack? Thank you!

finnlaymfarmor commented 8 months ago

Have opened an issue over at webpack. Will let you know if resolved. Thanks for your help.