getsentry / sentry-javascript

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

Unexpected token errors coming from import-in-the-middle #12422

Closed SerenModz21 closed 1 week ago

SerenModz21 commented 1 month ago

Is there an existing issue for this?

How do you use Sentry?

Sentry Saas (sentry.io)

Which SDK are you using?

@sentry/node

SDK Version

8.8.0

Framework Version

Node v20.14.0

Link to Sentry event

No response

SDK Setup

Sentry.init({
    dsn: envParseString("SENTRY_DSN"),
    integrations: [
        Sentry.nativeNodeFetchIntegration(),
        Sentry.consoleIntegration(),
        Sentry.onUncaughtExceptionIntegration(),
        Sentry.onUnhandledRejectionIntegration(),
        Sentry.functionToStringIntegration(),
        Sentry.modulesIntegration(),
        Sentry.linkedErrorsIntegration(),
        Sentry.httpIntegration(),
    ],
    tracesSampleRate: 1.0,
    environment: nodeEnvironment,
    release: `${name}@${version}`,
});

Steps to Reproduce

I simply had migrated from v7 to v8.

I was using 8.7.0 with globalThis._sentryEsmLoaderHookRegistered = true; until the "no such file or directory" got issue, which was the case with 8.8.0. Since installing the update, i commented out that line of code to check if everything had been fixed but i get spammed with the mentioned unexpected token errors.

Just like the "no such file or directory" error, i'm just gonna have to keep using globalThis._sentryEsmLoaderHookRegistered = true; or just fallback to v7 for the time being.

There is https://github.com/getsentry/sentry-javascript/issues/12357 which is quite similar, the difference being that I do not use TSX but the standard TypeScript compiler. Where my startup is simly tsc && node --enable-source-maps .. In addition, my project involves discord.js, @sapphire/framework, and other sapphire plugins/utilities (eg. decorators)

Expected Result

Errors should not appear.

Actual Result

SyntaxError [Error]: Unexpected token (3:40)
    at pp$4.raise (PATH_REDACTED\node_modules\acorn\dist\acorn.js:3573:15)
    at pp$9.unexpected (PATH_REDACTED\node_modules\acorn\dist\acorn.js:772:10)
    at pp$9.semicolon (PATH_REDACTED\node_modules\acorn\dist\acorn.js:749:68)
    at Parser.parseImport (PATH_REDACTED\node_modules\acorn-import-attributes\lib\index.js:242:12)
    at pp$8.parseStatement (PATH_REDACTED\node_modules\acorn\dist\acorn.js:948:51)
    at pp$8.parseTopLevel (PATH_REDACTED\node_modules\acorn\dist\acorn.js:829:23)
    at Parser.parse (PATH_REDACTED\node_modules\acorn\dist\acorn.js:601:17)
    at Function.parse (PATH_REDACTED\node_modules\acorn\dist\acorn.js:651:37)
    at getEsmExports (PATH_REDACTED\node_modules\@opentelemetry\instrumentation\node_modules\import-in-the-middle\lib\get-esm-exports.js:37:23)     
    at getExports (PATH_REDACTED\node_modules\@opentelemetry\instrumentation\node_modules\import-in-the-middle\lib\get-exports.js:73:12) {    
  pos: 166,
  loc: { line: 3, column: 40 },
  raisedAt: 170
}
Lms24 commented 1 month ago

Hi @SerenModz21 thanks for writing in and sorry for the troubles! I'm not yet sure how or why this error is thrown, especially because just transpiling with TSC shouldn't have an impact here. To debug this further:

Let's first check if the SDK is set up correctly and go from there.

SerenModz21 commented 1 month ago

I do indeed transpile to ESM. As for installation, that would be the late initialization method. Unfortunately however, I made the mistake of not trying with --import @sentry/node/preload; therefore, I will give such a try and provide an update on any further news. I am just a little confused how auto-instrumentation would make a difference in this situation.

Lms24 commented 1 month ago

The --import @sentry/node/preload option ensures that import-in-the-middle and OpenTelemetry instrumentation is added early enough. I'm not sure if it makes a difference with your issue but IITM has been known for interfering with import syntax. Let's see if it changes something!

timfish commented 1 month ago

It could be the --enable-source-maps option which I have a suspicion could be implemented via loader hooks or at least hits the resolution hook.

I guess import-in-the-middle should be passing non-js files straight through to the next loader. Maybe if there are any exceptions iitm should console.error them but fall back to the next loader rather than fail at this point. It would make it less of a blocker when it fails on things like this.

SerenModz21 commented 1 month ago

As an update to my previous comment, I have now tried the following three commands:

node --import @sentry/node/preload --enable-source-maps .
node --import @sentry/node/preload .
node --import @sentry/node/preload dist/index.js

However, they all show the "unexpected token" error, followed by the node version (20.14.0) and then it finishes executing, as in it just finishes and does not start the actual node app.

❯ node --import @sentry/node/preload .

node:internal/modules/run_main:129
    triggerUncaughtException(
    ^
SyntaxError [Error]: Unexpected token (3:40)
    at pp$4.raise (PATH_REDACTED\node_modules\acorn\dist\acorn.js:3573:15)     
    at pp$9.unexpected (PATH_REDACTED\node_modules\acorn\dist\acorn.js:772:10) 
    at pp$9.semicolon (PATH_REDACTED\node_modules\acorn\dist\acorn.js:749:68)  
    at Parser.parseImport (PATH_REDACTED\node_modules\acorn-import-attributes\lib\index.js:242:12)
    at pp$8.parseStatement (PATH_REDACTED\node_modules\acorn\dist\acorn.js:948:51)
    at pp$8.parseTopLevel (PATH_REDACTED\node_modules\acorn\dist\acorn.js:829:23)
    at Parser.parse (PATH_REDACTED\node_modules\acorn\dist\acorn.js:601:17)    
    at Function.parse (PATH_REDACTED\node_modules\acorn\dist\acorn.js:651:37)  
    at getEsmExports (PATH_REDACTED\node_modules\@opentelemetry\instrumentation\node_modules\import-in-the-middle\lib\get-esm-exports.js:37:23)
    at getExports (PATH_REDACTED\node_modules\@opentelemetry\instrumentation\node_modules\import-in-the-middle\lib\get-exports.js:73:12) {
  pos: 166,
  loc: { line: 3, column: 40 },
  raisedAt: 170
}

Node.js v20.14.0

My apologies for the delay in mentioning this update.

SerenModz21 commented 1 month ago

In addition, here is the output with debug logs:

❯ SENTRY_DEBUG=1 node --import @sentry/node/preload .
Sentry Logger [debug]: @opentelemetry/api: Registered a global for diag v1.9.0.
Sentry Logger [log]: [Sentry] Preloaded Http instrumentation
Sentry Logger [log]: [Sentry] Preloaded Express instrumentation
Sentry Logger [log]: [Sentry] Preloaded Connect instrumentation
Sentry Logger [log]: [Sentry] Preloaded Fastify instrumentation
Sentry Logger [log]: [Sentry] Preloaded Hapi instrumentation
Sentry Logger [log]: [Sentry] Preloaded Koa instrumentation
Sentry Logger [log]: [Sentry] Preloaded Nest instrumentation
Sentry Logger [log]: [Sentry] Preloaded Mongo instrumentation
Sentry Logger [log]: [Sentry] Preloaded Mongoose instrumentation
Sentry Logger [log]: [Sentry] Preloaded Mysql instrumentation
Sentry Logger [log]: [Sentry] Preloaded Mysql2 instrumentation
Sentry Logger [log]: [Sentry] Preloaded Postgres instrumentation
Sentry Logger [log]: [Sentry] Preloaded Hapi instrumentation
Sentry Logger [log]: [Sentry] Preloaded Graphql instrumentation
Sentry Logger [log]: [Sentry] Preloaded Redis instrumentation

node:internal/modules/run_main:129
    triggerUncaughtException(
    ^
SyntaxError [Error]: Unexpected token (3:40)
AnthonyDugarte commented 1 month ago

any update?

SENTRY_DEBUG=1 node -r dotenv/config --import @sentry/node/preload .
Sentry Logger [debug]: @opentelemetry/api: Registered a global for diag v1.9.0.
Sentry Logger [log]: [Sentry] Preloaded Http instrumentation
Sentry Logger [log]: [Sentry] Preloaded Express instrumentation
Sentry Logger [log]: [Sentry] Preloaded Connect instrumentation
Sentry Logger [log]: [Sentry] Preloaded Fastify instrumentation
Sentry Logger [log]: [Sentry] Preloaded Hapi instrumentation
Sentry Logger [log]: [Sentry] Preloaded Koa instrumentation
Sentry Logger [log]: [Sentry] Preloaded Nest instrumentation
Sentry Logger [log]: [Sentry] Preloaded Mongo instrumentation
Sentry Logger [log]: [Sentry] Preloaded Mongoose instrumentation
Sentry Logger [log]: [Sentry] Preloaded Mysql instrumentation
Sentry Logger [log]: [Sentry] Preloaded Mysql2 instrumentation
Sentry Logger [log]: [Sentry] Preloaded Postgres instrumentation
Sentry Logger [log]: [Sentry] Preloaded Hapi instrumentation
Sentry Logger [log]: [Sentry] Preloaded Graphql instrumentation
Sentry Logger [log]: [Sentry] Preloaded Redis instrumentation

node:internal/process/esm_loader:34
      internalBinding('errors').triggerUncaughtException(
                                ^
SyntaxError [Error]: Unexpected token (13:76)
    at pp$4.raise (/path-to-project/node_modules/.pnpm/acorn@8.10.0/node_modules/acorn/dist/acorn.js:3560:15)
    at pp$9.unexpected (/path-to-project/node_modules/.pnpm/acorn@8.10.0/node_modules/acorn/dist/acorn.js:768:10)
    at pp$9.semicolon (/path-to-project/node_modules/.pnpm/acorn@8.10.0/node_modules/acorn/dist/acorn.js:745:68)
    at Parser.parseImport (/path-to-project/node_modules/.pnpm/acorn-import-attributes@1.9.5_acorn@8.10.0/node_modules/acorn-import-attributes/lib/index.js:242:12)
    at pp$8.parseStatement (/path-to-project/node_modules/.pnpm/acorn@8.10.0/node_modules/acorn/dist/acorn.js:944:51)
    at pp$8.parseTopLevel (/path-to-project/node_modules/.pnpm/acorn@8.10.0/node_modules/acorn/dist/acorn.js:825:23)
    at Parser.parse (/path-to-project/node_modules/.pnpm/acorn@8.10.0/node_modules/acorn/dist/acorn.js:597:17)
    at Function.parse (/path-to-project/node_modules/.pnpm/acorn@8.10.0/node_modules/acorn/dist/acorn.js:647:37)
    at getEsmExports (/path-to-project/node_modules/.pnpm/import-in-the-middle@1.8.0/node_modules/import-in-the-middle/lib/get-esm-exports.js:37:23)
    at getExports (/path-to-project/node_modules/.pnpm/import-in-the-middle@1.8.0/node_modules/import-in-the-middle/lib/get-exports.js:73:12) {
  pos: 978,
  loc: { line: 13, column: 76 },
  raisedAt: 982
}

Node.js v20.12.1
timfish commented 1 month ago

I suspect this is caused by a combination of your dependencies and import-in-the-middle but I've not yet worked out the dependency that's causing this.

Lms24 commented 1 month ago

@AnthonyDugarte have you tried removing -r dotenv/config? Not sure if this changes something but if dotenv.config requires a JSON file, it might be problematic for IITM 🤔

AnthonyDugarte commented 1 month ago

@AnthonyDugarte have you tried removing -r dotenv/config? Not sure if this changes something but if dotenv.config requires a JSON file, it might be problematic for IITM 🤔

@Lms24 that gave me an idea, I have a internal dependency that imports json files:

import json from './file.json' with { type: 'json' };

Commented those out and the previous error went away so prob will make them js objects for now.

Now I got a different issue, checking that one

timfish commented 1 month ago

This is because the parser import-in-the-middle uses does not support import attributes because they've only reached TC39 Stage 3.

I've opened an issue for this and I've opened a PR that works around this: https://github.com/DataDog/import-in-the-middle/issues/105

timfish commented 1 month ago

This should hopefully have been fixed by the numerous PRs recently merged at import-in-the-middle.

While we wait for this to be released, there is a patch available that combines all the fixes. If anyone can confirm this patch fixes this issue that would be super helpful!

SerenModz21 commented 1 month ago

With the patch and using @sentry/node v8.9.2, I get the following output:

❯ SENTRY_DEBUG=1 SENTRY_PRELOAD_INTEGRATIONS="Http" node --import @sentry/node/preload .
Sentry Logger [debug]: @opentelemetry/api: Registered a global for diag v1.9.0.
Sentry Logger [log]: [Sentry] Preloaded Http instrumentation
Error: 'import-in-the-middle' failed to wrap 'file:///PATH_REDACTED/node_modules/@prisma/client/default.js'
    at getSource (PATH_REDACTED\node_modules\import-in-the-middle\hook.js:304:21)
    at async load (PATH_REDACTED\node_modules\import-in-the-middle\hook.js:319:26)
    at async nextLoad (node:internal/modules/esm/hooks:866:22)
    at async Hooks.load (node:internal/modules/esm/hooks:449:20)
    at async handleMessage (node:internal/modules/esm/worker:196:18) {
  cause: Error: Unexpected import statement in CJS module.
    at @:2:8
      at esmSyntaxErr (PATH_REDACTED\node_modules\cjs-module-lexer\lexer.js:1153:24)
      at throwIfImportStatement (PATH_REDACTED\node_modules\cjs-module-lexer\lexer.js:1183:13)
      at parseSource (PATH_REDACTED\node_modules\cjs-module-lexer\lexer.js:113:13)
      at parseCJS (PATH_REDACTED\node_modules\cjs-module-lexer\lexer.js:43:5)
      at getCjsExports (PATH_REDACTED\node_modules\import-in-the-middle\lib\get-exports.js:37:20)
      at getExports (PATH_REDACTED\node_modules\import-in-the-middle\lib\get-exports.js:100:12)
      at async PATH_REDACTED\node_modules\import-in-the-middle\lib\get-exports.js:48:28
      at async Promise.all (index 0)
      at async getCjsExports (PATH_REDACTED\node_modules\import-in-the-middle\lib\get-exports.js:40:5)
      at async PATH_REDACTED\node_modules\import-in-the-middle\lib\get-exports.js:48:28 {
    code: 'ERR_LEXER_ESM_SYNTAX',
    loc: 45
  }
}
node:internal/modules/esm/translators:118
  throw new ERR_INVALID_RETURN_PROPERTY_VALUE(
        ^

TypeError [ERR_INVALID_RETURN_PROPERTY_VALUE]: Expected string, array buffer, or typed array to be returned for the "source" from the "load" hook but got undefined.
    at assertBufferSource (node:internal/modules/esm/translators:118:9)
    at ModuleLoader.moduleStrategy (node:internal/modules/esm/translators:166:3)
    at callTranslator (node:internal/modules/esm/loader:272:14)
    at ModuleLoader.moduleProvider (node:internal/modules/esm/loader:278:30) {
  code: 'ERR_INVALID_RETURN_PROPERTY_VALUE'
}

Node.js v20.14.0

When i first made this comment, I originally had an import to package.json, using the json import attribute. However, the import attribute seems to not be needed and thus, I have removed it - causing for the SyntaxError: Unexpected token (3:40) error to no longer occur.

The changes being:

- import pkg from "../../../package.json" with { type: "json" };
+ import pkg from "../../../package.json";

Note: I got the idea to try without the import attribute from https://github.com/DataDog/import-in-the-middle/issues/105 and the above comment (https://github.com/getsentry/sentry-javascript/issues/12422#issuecomment-2163588248)

To avoid any confusion, I have rewrote my comment.

timfish commented 1 month ago

Thanks for testing this!

I'm trying to reproduce this locally. How are you importing @prisma/client?

SerenModz21 commented 1 month ago

I import prisma using an ESM import statement.

import { PrismaClient } from "@prisma/client";

If needed, I use prisma 5.15.0

timfish commented 1 month ago

modulesIntegration does not support ESM so I guess you're transpiling to CJS?

When I use that from ESM I get an error! (#12500)

Do you have a full minimal reproduction of the issue? I've tried to piece together what I think is your setup but everything I try either works or I hit weird compatibility issues like above.

SerenModz21 commented 1 month ago

Since the modules integration doesn't support ESM, I can always just remove it. I honestly don't remember why I've always had it there.

I will remove it and will let you know what output I get from it.

SerenModz21 commented 1 month ago

I tried removing the modules integration and also tried commenting out all integrations that I use but unfortuantely, the Unexpected import statement in CJS module error remains.

Here is a small repo that I have setup that reproduces the error: https://github.com/SerenModz21/sentry-error

timfish commented 1 month ago

Thanks for the repro, super helpful. I managed to track this down and create PR to fix this. I'll update the import-in-the-middle patch at a more sensible time tomorrow!

It's worth noting that this doesn't appear to work:

import { name, version } from "../../package.json" with { type: "json" };

The JSON is exported as the default export so you'll need to do:

import pkg from "../../package.json" with { type: "json" };
timfish commented 1 month ago

I've updated the patch here to include the above linked PR.

SerenModz21 commented 1 month ago

I have just tested the new patch and my application is now able to execute and be used, which is great news! The errors still get logged but I'm assuming that is to be expected right now, until the PRs finish getting merged and released.

So that you have some logs of everything: https://gist.github.com/SerenModz21/81b8a593d1935a3de45eaded861f4043 I have uploaded it to a gist as I don't want to make the history of this issue get a little too long.

timfish commented 4 weeks ago

The errors still get logged but I'm assuming that is to be expected right now, until the PRs finish getting merged and released

import-in-the-middle will continue to log warnings for these errors even after all PRs are merged.

SerenModz21 commented 4 weeks ago

The errors still get logged but I'm assuming that is to be expected right now, until the PRs finish getting merged and released

import-in-the-middle will continue to log warnings for these errors even after all PRs are merged.

Ah, that is fine. Im guess it's just a matter of time until the Acorn issue that you made gets addressed? That being the import attribute one.

In addition, I would just like to say that I have appreciated your time greatly in helping me resolve the issue I was having - thank you!

FozzieHi commented 1 week ago

@timfish I'm experiencing a similar issue, do you think this is related?

Error: 'import-in-the-middle' failed to wrap 'file:///app/node_modules/zlib-sync/index.js'
    at getSource (/app/node_modules/import-in-the-middle/hook.js:304:21)
    at async load (/app/node_modules/import-in-the-middle/hook.js:319:26)
    at async nextLoad (node:internal/modules/esm/hooks:866:22)
    at async Hooks.load (node:internal/modules/esm/hooks:449:20)
    at async handleMessage (node:internal/modules/esm/worker:196:18) {
  cause: Error: Unexpected closing brace.
    at @:4:142
      at parseSource (/app/node_modules/cjs-module-lexer/lexer.js:185:17)
      at parseCJS (/app/node_modules/cjs-module-lexer/lexer.js:43:5)
      at getCjsExports (/app/node_modules/import-in-the-middle/lib/get-exports.js:37:20)
      at getExports (/app/node_modules/import-in-the-middle/lib/get-exports.js:100:12)
      at async /app/node_modules/import-in-the-middle/lib/get-exports.js:48:28
      at async Promise.all (index 0)
      at async getCjsExports (/app/node_modules/import-in-the-middle/lib/get-exports.js:40:5)
      at async processModule (/app/node_modules/import-in-the-middle/hook.js:131:23)
      at async getSource (/app/node_modules/import-in-the-middle/hook.js:269:25)
      at async load (/app/node_modules/import-in-the-middle/hook.js:319:26) {
    loc: 973
  }
}

Is this an issue within import-in-the-middle or Sentry's implementation of it? I'm happy to open an issue wherever.

timfish commented 1 week ago

@FozzieHi I think this is just a warning being logged. Does this affect your application otherwise?

FozzieHi commented 1 week ago

@FozzieHi I think this is just a warning being logged. Does this affect your application otherwise?

It does seem to yes, zlib-sync is not used by me directly but discord.js uses it for compression, and it logs out this below the import-in-the-middle error:

WebSocketShard: Compression is enabled but zlib-sync is not installed, falling back to identify compress

AbhiPrasad commented 1 week ago

@FozzieHi mind opening a new issue? We can dive deeper there.

For everyone else:

With the newest release of import-in-the-middle v1.9.0 this should be fixed.

If you upgrade to a fresh install of the latest version of the Node SDK it should use import-in-the-middle@1.9.0 by default.

FozzieHi commented 1 week ago

@AbhiPrasad Will do, thanks for your work on this though!