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

Unexpected closing brace error from import-in-the-middle #12807

Closed
3 tasks done
Tracked by #12806
FozzieHi opened this issue Jul 8, 2024 · 8 comments
Closed
3 tasks done
Tracked by #12806

Unexpected closing brace error from import-in-the-middle #12807

FozzieHi opened this issue Jul 8, 2024 · 8 comments
Assignees
Labels

Comments

@FozzieHi
Copy link

FozzieHi commented Jul 8, 2024

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

Framework Version

No response

Link to Sentry event

No response

SDK Setup/Reproduction Example

No response

Steps to Reproduce

Branching off this issue, when running import-in-the-middle v1.8.1, I'm getting the following error:

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
  }
}

This causes this from discord.js:

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

It still occurs with import-in-the-middle v1.9.0, but as a warning, this is the output with --trace-warnings enabled:

WebSocketShard: Compression is enabled but zlib-sync is not installed, falling back to identify compress
(node:1) Warning: 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:366:21)
    at async load (/app/node_modules/import-in-the-middle/hook.js:381:26)
    ... 2 lines matching cause stack trace ...
    at async handleMessage (node:internal/modules/esm/worker:196:18) {
  cause: Error: Failed to parse 'file:///app/node_modules/zlib-sync/index.js'
      at getExports (/app/node_modules/import-in-the-middle/lib/get-exports.js:118:17)
      at async processModule (/app/node_modules/import-in-the-middle/hook.js:172:23)
      ... 3 lines matching cause stack trace ...
      at async Hooks.load (node:internal/modules/esm/hooks:449:20)
      at async handleMessage (node:internal/modules/esm/worker:196:18) {
    cause: Error: Failed to parse 'file:///app/node_modules/zlib-sync/build/Release/zlib_sync.node'
        at getExports (/app/node_modules/import-in-the-middle/lib/get-exports.js:118:17)
        at async /app/node_modules/import-in-the-middle/lib/get-exports.js:51:28
        ... 4 lines matching cause stack trace ...
        at async getSource (/app/node_modules/import-in-the-middle/hook.js:331:25)
        at async load (/app/node_modules/import-in-the-middle/hook.js:381:26)
        at async nextLoad (node:internal/modules/esm/hooks:866:22)
        at async Hooks.load (node:internal/modules/esm/hooks:449:20) {
      cause: [Error]
    }
  }
}
    at emitWarning (/app/node_modules/import-in-the-middle/hook.js:156:11)
    at getSource (/app/node_modules/import-in-the-middle/hook.js:368:9)
    at async load (/app/node_modules/import-in-the-middle/hook.js:381: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)

Expected Result

N/A

Actual Result

N/A

@AbhiPrasad
Copy link
Member

Hey @FozzieHi thanks for the new issue. I'll let @timfish handle taking a look at this, and it's also being tracked under our main umbrella issue for all instrumentation problems: #12806

@timfish
Copy link
Collaborator

timfish commented Jul 9, 2024

Handling of native modules needs improving in import-in-the-middle and I've opened an issue to address this but currently this is just logging a warning when it's encountered and it shouldn't impact @sentry/node.

@FozzieHi
Copy link
Author

FozzieHi commented Jul 9, 2024

Thanks @timfish, if it is just a warning, I'm wondering why it's causing discord.js to log this out:

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

I've taken a brief look at the discord.js code, and it looks like it is affecting this line, which if it's falsy, causes the message to be logged out:

const getZlibSync = lazy(async () => import('zlib-sync').then((mod) => mod.default).catch(() => null));

https://github.com/discordjs/discord.js/blob/311aaf26058c7a95f6d973686e49818c9ecb0fb1/packages/ws/src/ws/WebSocketShard.ts#L177

@timfish
Copy link
Collaborator

timfish commented Jul 15, 2024

A fix was release in [email protected]. Could you try it out and see if it fixes this issue?

You can either delete your lock file and reinstall or npm/yarn update import-in-the-middle

@FozzieHi
Copy link
Author

That works, thanks for your help on this! As an aside, do we know why the warning actually affected discord.js in this instance? If these sort of warnings can cause application issues, shouldn't it be changed to an error?

@getsantry getsantry bot moved this to Waiting for: Product Owner in GitHub Issues with 👀 3 Jul 15, 2024
@timfish
Copy link
Collaborator

timfish commented Jul 15, 2024

import-in-the-middle instruments every ES module and previosly it threw errors when failing to instrument a file which terminates the process and leaves users unable to use the SDK.

We changed import-in-the-middle to catch these errors with the only downside being that these specific modules cannot be instrumented. For example in your case, the Sentry SDK doesn't instrument zlib-sync, so it doesn't matter:

Error: 'import-in-the-middle' failed to wrap 'file:///app/node_modules/zlib-sync/index.js'

Catching these errors helps SDK users get on with their work but we still log them as warnings because then users open helpful issues like this one which let us know where import-in-the-middle might have bugs or need improvement!

@FozzieHi
Copy link
Author

Right, but in this case the warning in import-in-the-middle did cause this variable to somehow become falsy, is it worth opening a new issue on the import-in-the-middle repo to investigate more?

const getZlibSync = lazy(async () => import('zlib-sync').then((mod) => mod.default).catch(() => null));

@timfish
Copy link
Collaborator

timfish commented Jul 15, 2024

If the issue is still reproducible with v1.9.1, I would open a new issue.

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

No branches or pull requests

3 participants