getsentry / profiling-node

The code for this repo now lives in https://github.com/getsentry/sentry-javascript/tree/develop/packages/profiling-node
MIT License
29 stars 10 forks source link

`__dirname is not defined in ES module scope` error when setting up profiling for Remix #217

Open jackie-greenbaum opened 11 months ago

jackie-greenbaum commented 11 months ago

Is there an existing issue for this?

How do you use Sentry?

Sentry Saas (sentry.io)

SDK Version

7.81.1

Link to Sentry event

N/A

What environment is your node script running in?

Remix 2.1.0

How is your code deployed and bundled?

Remix running in AWS Lambda

Steps to Reproduce

Followed the steps to set up my Remix application with profiling as described by the sidebar that appears when clicking Set up Profiling

Screenshot 2023-12-06 at 2 30 20 PM

Below is my code for setting up @sentry/remix in my entry.server.ts file.

 import * as SentryRemix from "@sentry/remix"

 SentryRemix.init({
   dsn: "omitted",
   tracesSampleRate: 1,
   profilesSampleRate: 1,
   integrations: [
     new SentryRemix.Integrations.LocalVariables({
       captureAllExceptions: true,
     }),
     new ProfilingIntegration()
   ],
 })

Expected Result

Profiling works correctly

Actual Result

When I deploy this, I see the following 500 error

2023-12-06T19:44:49.312Z    undefined   ERROR   Uncaught Exception  
{
    "errorType": "ReferenceError",
    "errorMessage": "__dirname is not defined in ES module scope",
    "stack": [
        "ReferenceError: __dirname is not defined in ES module scope",
        "    at node_modules/@sentry/profiling-node/lib/index.js (file:///var/task/ui/build/index.mjs:48006:777)",
        "    at file:///var/task/ui/build/index.mjs:26:50",
        "    at file:///var/task/ui/build/index.mjs:108280:37"
    ]
}
JonasBa commented 11 months ago

@jackie-greenbaum could you provide us with a small reproducible example by any chance?

anonrig commented 11 months ago

Which Node.js version do you use in your AWS Lambda?

jackie-greenbaum commented 11 months ago

Which Node.js version do you use in your AWS Lambda?

18.12.0

@jackie-greenbaum could you provide us with a small reproducible example by any chance?

So, I was able to create a new remix app with sentry from scratch which just runs on localhost and successfully configure profiling. My current guess is that this issue something to do with my AWS configuration

anonrig commented 11 months ago

I think that you are seeing is an issue with the bundling process. You're trying to bundle your application as a ESM application, but use commonjs functionality like __dirname in the same file.

I ran the following code with Node 18.12.0

~/coding/testing via ⬢ v18.12.0 took 2s
❯ cat commonjs-file.js
console.log('this is a commonjs file', __dirname)

~/coding/testing via ⬢ v18.12.0
❯ cat esm-file.mjs
import('./commonjs-file.js')

~/coding/testing via ⬢ v18.12.0
❯ node esm-file.mjs
this is a commonjs file /Users/yagiz/coding/testing

And it works.

It seems your error call stack hints towards issues in bundling:

"    at node_modules/@sentry/profiling-node/lib/index.js (file:///var/task/ui/build/index.mjs:48006:777)",

If you want to use a commonjs file in an ESM application, it needs to trigger ESM loader - which triggers the CJS loader in Node.js. Since they're bundled in the same file, it doesn't trigger any loader.

If you're using ESBuild or a similar bundler, you need add @sentry/profiling-node to external. For example:

{
  entryPoints: ['index.js'],
  platform: 'node',
  external: ['@sentry/profiling-node'],
}
brettdh commented 11 months ago

HI, I work with @jackie-greenbaum . Marking @sentry/profiling-node as external isn't going to work for us, as we're intentionally bundling everything to avoid deploying node_modules to Lambda, which has a limited total deployment package size.

I do vaguely recall other issues with node builtins that needed to be polyfilled but I haven't been able to track down the issues/discussions I was looking at. It may have been back when we were trying to bundle this as CommonJS, or maybe I'm misremembering things.

@jackie-greenbaum Were you testing the minimal repro app with remix dev? I wonder what happens if we run remix build with "serverDependenciesToBundle": "all", and run it with remix serve, which would be closer to our setup (minus the AWS part).

jackie-greenbaum commented 11 months ago

Here's a small sample project exhibiting the issue in case it would be at all helpful

As @brettdh mentioned, we are unable to treat @sentry/profiling as external due to Lambda package size limits