sentry-electron icon indicating copy to clipboard operation
sentry-electron copied to clipboard

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

Open finnlaymfarmor opened this issue 1 year ago • 12 comments

Is there an existing issue for this?

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

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

finnlaymfarmor avatar Dec 29 '23 07:12 finnlaymfarmor

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.

lforst avatar Dec 29 '23 09:12 lforst

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

finnlaymfarmor avatar Dec 29 '23 09:12 finnlaymfarmor

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

timfish avatar Dec 29 '23 10:12 timfish

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

finnlaymfarmor avatar Dec 29 '23 10:12 finnlaymfarmor

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?

timfish avatar Dec 29 '23 11:12 timfish

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

finnlaymfarmor avatar Dec 29 '23 22:12 finnlaymfarmor

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

lforst avatar Jan 03 '24 13:01 lforst

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.

ofarnill avatar Jan 04 '24 08:01 ofarnill

@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.

lforst avatar Jan 04 '24 11:01 lforst

npx @sentry/wizard@latest -i sourcemaps

image

Then following the prompts for webpack config. Thanks.

ofarnill avatar Jan 05 '24 01:01 ofarnill

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!

lforst avatar Jan 05 '24 08:01 lforst

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

finnlaymfarmor avatar Jan 06 '24 12:01 finnlaymfarmor