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

ESM require('http') breaks SSR in SolidStart #7938

Closed
3 tasks done
jchatard opened this issue Apr 23, 2023 · 11 comments
Closed
3 tasks done

ESM require('http') breaks SSR in SolidStart #7938

jchatard opened this issue Apr 23, 2023 · 11 comments
Labels
Meta: Breaking Package: node Issues related to the Sentry Node SDK Type: Bug
Milestone

Comments

@jchatard
Copy link

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

7.49.0

Framework Version

solid-start version 0.2.26

Link to Sentry event

No response

SDK Setup

No response

Steps to Reproduce

  1. Clone repro https://github.com/jchatard/sentry-solid
  2. cd sentry-solid
  3. pnpm install
  4. pnpm build
  5. pnpm start

I think if you get SvelteKit working, my guess is that Solid Start would almost identical.

The work done here: #7377

Was probably the way to go, but it was reverted.

Expected Result

  1. Application start and is reachable on port 3000
  2. The example exception is logged to Sentry.io https://github.com/jchatard/sentry-solid/blob/main/src/entry-server.tsx#L24

Actual Result

Server won't start with this output:

file:///Users/jchatard/Desktop/sentry-solid/dist/server.js:26181
    const httpModule = require('http');
                       ^

ReferenceError: require is not defined in ES module scope, you can use import instead
This file is being treated as an ES module because it has a '.js' file extension and '/Users/jchatard/Desktop/sentry-solid/package.json' contains "type": "module". To treat it as a CommonJS script, rename it to use the '.cjs' file extension.
    at Http.setupOnce (file:///Users/jchatard/Desktop/sentry-solid/dist/server.js:26181:24)
    at setupIntegration (file:///Users/jchatard/Desktop/sentry-solid/dist/server.js:21977:17)
    at file:///Users/jchatard/Desktop/sentry-solid/dist/server.js:21965:7
    at Array.forEach (<anonymous>)
    at setupIntegrations (file:///Users/jchatard/Desktop/sentry-solid/dist/server.js:21962:16)
    at NodeClient.setupIntegrations (file:///Users/jchatard/Desktop/sentry-solid/dist/server.js:22480:28)
    at Hub.bindClient (file:///Users/jchatard/Desktop/sentry-solid/dist/server.js:20241:14)
    at initAndBind (file:///Users/jchatard/Desktop/sentry-solid/dist/server.js:22946:7)
    at init (file:///Users/jchatard/Desktop/sentry-solid/dist/server.js:28588:3)
    at file:///Users/jchatard/Desktop/sentry-solid/dist/server.js:30282:1

Node.js v18.9.0
 ELIFECYCLE  Command failed with exit code 1
@AbhiPrasad
Copy link
Member

Hey @jchatard, thanks for writing in! Unfortunately we had to revert the changes in #7377 because we need to monkeypatch the http library for our instrumentation.

ESM has no in-process solution for this, since es modules are considered immutable on import. Their recommended solution is to use esm loaders, but this is considered experimental API, so we hesitant in adding anything until it gets more stable (it also requires you to provide a cmd line argument to add a loader, which makes DX suffer a lot).

I would recommend you use the CJS bundle instead or remove the Http integration.

@AbhiPrasad AbhiPrasad added Meta: Breaking Package: node Issues related to the Sentry Node SDK Status: Backlog labels Apr 24, 2023
@AbhiPrasad AbhiPrasad added this to the 8.0.0 milestone Apr 24, 2023
@jchatard
Copy link
Author

@AbhiPrasad
Copy link
Member

@jchatard Yes!

@jchatard
Copy link
Author

jchatard commented Apr 24, 2023

Ok, so I just tried that:

import * as Sentry from "@sentry/node";

Sentry.init({
  dsn: "https://.....",

  // We recommend adjusting this value in production, or using tracesSampler
  // for finer control
  tracesSampleRate: 1.0,
  defaultIntegrations:[
    new Sentry.Integrations.Console(),
    //new Sentry.Integrations.Http(), // Disable me
    new Sentry.Integrations.OnUncaughtException(),
    new Sentry.Integrations.OnUnhandledRejection(),
    new Sentry.Integrations.ContextLines(),
    new Sentry.Integrations.LocalVariables({
      captureAllExceptions: true,
    }),
    new Sentry.Integrations.RequestData(),
  ],
  // integrations: function(integrations) {
  //   // integrations will be all default integrations
  //   return integrations.filter(integration => integration.name !== "Http");
  // },
});

But now I get:

Listening on port 3000
file:///Users/jchatard/Desktop/sentry-solid/dist/server.js:29758
  const base = (require && require.main && require.main.filename && dir) || global.process.cwd();
               ^

ReferenceError: require is not defined
    at getModule (file:///Users/jchatard/Desktop/sentry-solid/dist/server.js:29758:16)
    at file:///Users/jchatard/Desktop/sentry-solid/dist/server.js:17206:29
    at file:///Users/jchatard/Desktop/sentry-solid/dist/server.js:17263:23
    at parseStackFrames (file:///Users/jchatard/Desktop/sentry-solid/dist/server.js:24488:10)
    at exceptionFromError (file:///Users/jchatard/Desktop/sentry-solid/dist/server.js:24500:18)
    at eventFromUnknownInput (file:///Users/jchatard/Desktop/sentry-solid/dist/server.js:24548:16)
    at NodeClient.eventFromException (file:///Users/jchatard/Desktop/sentry-solid/dist/server.js:24708:32)
    at NodeClient.captureException (file:///Users/jchatard/Desktop/sentry-solid/dist/server.js:22448:12)
    at NodeClient.captureException (file:///Users/jchatard/Desktop/sentry-solid/dist/server.js:24651:18)
    at file:///Users/jchatard/Desktop/sentry-solid/dist/server.js:20347:14

Node.js v18.9.0
 ELIFECYCLE  Command failed with exit code 7.

Which comes from node/src/module.ts export function getModule():

//...
const base = (require && require.main && require.main.filename && dir) || global.process.cwd();
//...

So I add to change this line to this:

const base = global.process.cwd();

Now I can see the test failure being sent to Sentry.io.

@AbhiPrasad
Copy link
Member

Ah yeah this is because of the stacktrace parsing needing require 🤔

Backlogging since we need to re-visits and fix this!

@rchl
Copy link
Contributor

rchl commented Jun 12, 2023

I was going to report an issue for that but found that this one exists already.

I guess the issue is pretty much the same even if I'm in a different environment and using different SDK.

I'm trying to run a Lambda in AWS on Node 18 using native Node ES support. I have an esm bundle of the lambda code that includes the code of @sentry/serverless and that throws the same error. Not surprising as it uses @sentry/node under the hood.

I guess the only option for me right now is to create a CJS bundle instead.

@Lms24
Copy link
Member

Lms24 commented Aug 3, 2023

Not really a concrete update unfortunately but we briefly discussed making the Serverless SDK no longer depend on the Node SDK once #8693 is completed. Also, we're currently experimenting with @sentry/node-experimental using loaders for ESM support. Perhaps one of these changes might help moving forward with this one. In the meantime this remains backlogged.

@peterhirn
Copy link

peterhirn commented Aug 15, 2023

Same problem here, bundling my node backend to ESM using vite (similar to SolidStart).

I used pnpm patch to make @sentry/node stop throwing internal errors. Now at least some tracing is reaching sentry.io again, but I'm not yet convinced it is working as intended.

diff --git a/esm/integrations/http.js b/esm/integrations/http.js
index a37a69b2b0cde0c5f0ac41ef9dfd6857e68a0b5f..56ca380be34133eb5839a8683db17ae8662a787c 100644
--- a/esm/integrations/http.js
+++ b/esm/integrations/http.js
@@ -5,6 +5,9 @@ import { LRUMap } from 'lru_map';
 import { NODE_VERSION } from '../nodeVersion.js';
 import { normalizeRequestArgs, extractRawUrl, extractUrl, isSentryRequest, cleanSpanDescription } from './utils/http.js';
 
+import { createRequire } from "module"
+const require = createRequire(import.meta.url)
+
 /**
  * The http module integration instruments Node's internal http module. It creates breadcrumbs, transactions for outgoing
  * http requests and attaches trace data when tracing is enabled via its `tracing` option.
diff --git a/esm/integrations/localvariables.js b/esm/integrations/localvariables.js
index 7faad1039e59ed51acdf103ae82f14c29645ed59..8d08c3e74936bc2445494101973c5fc59b1651ec 100644
--- a/esm/integrations/localvariables.js
+++ b/esm/integrations/localvariables.js
@@ -3,6 +3,9 @@ import { logger } from '@sentry/utils';
 import { LRUMap } from 'lru_map';
 import { NODE_VERSION } from '../nodeVersion.js';
 
+import { createRequire } from "module"
+const require = createRequire(import.meta.url)
+
 /** Creates a container for callbacks to be called sequentially */
 function createCallbackList(complete) {
   // A collection of callbacks to be executed last to first
diff --git a/esm/integrations/undici/index.js b/esm/integrations/undici/index.js
index 06785c3c2cc36efffec32905e12b02f166f707c4..86bd58f6c4efa19eb44453c2aeb0a9f727cfdfcb 100644
--- a/esm/integrations/undici/index.js
+++ b/esm/integrations/undici/index.js
@@ -5,6 +5,9 @@ import { LRUMap } from 'lru_map';
 import { NODE_VERSION } from '../../nodeVersion.js';
 import { isSentryRequest } from '../utils/http.js';
 
+import { createRequire } from "module"
+const require = createRequire(import.meta.url)
+
 var ChannelName;(function (ChannelName) {
   // https://github.com/nodejs/undici/blob/e6fc80f809d1217814c044f52ed40ef13f21e43c/docs/api/DiagnosticsChannel.md#undicirequestcreate
   const RequestCreate = 'undici:request:create'; ChannelName["RequestCreate"] = RequestCreate;
@@ -66,7 +69,7 @@ class Undici  {
     let ds;
     try {
       // eslint-disable-next-line @typescript-eslint/no-var-requires
-      ds = dynamicRequire(module, 'diagnostics_channel') ;
+      ds = require('diagnostics_channel');
     } catch (e) {
       // no-op
     }
diff --git a/esm/module.js b/esm/module.js
index a6fad0044835c5657e83380438cda1d5688e4f24..79ac7727036b39e5f7d9f32e1eb2d118f795fe35 100644
--- a/esm/module.js
+++ b/esm/module.js
@@ -23,7 +23,7 @@ function getModuleFromFilename(
   // eslint-disable-next-line prefer-const
   let { root, dir, base: basename, ext } = posix.parse(normalizedFilename);
 
-  const base = (require && require.main && require.main.filename && dir) || global.process.cwd();
+  const base = global.process.cwd();
 
   const normalizedBase = `${base}/`;
 
diff --git a/esm/sdk.js b/esm/sdk.js
index 4e7a9174c701e163dd54a188b4ad9792dce41497..62ff95fb26d557575e2d553b2fd472480aea724b 100644
--- a/esm/sdk.js
+++ b/esm/sdk.js
@@ -34,7 +34,6 @@ const defaultIntegrations = [
   new ContextLines(),
   new LocalVariables(),
   new Context(),
-  new Modules(),
   new RequestData(),
   // Misc
   new LinkedErrors(),

@avegao
Copy link

avegao commented Mar 27, 2024

I have a similar issue.

We're updating our projects to use ESM and we have issues when is a Serverless Framework for AWS Lambda with Typescript and esbuild. When is CommonJS project there isn't any problem. The issues are in local (sls invoke local) and deployed in AWS.

In esbuild config, we need to use mainFields: ['module', 'main'] because some dependencies like @sentry/serverless has cjs and esm versions, and esbuild can't select esm version by default.

After resolve this, when we invoke lambda functions, it throws:

{
  "errorType": "Error",
  "errorMessage": "Dynamic require of \"http\" is not supported",
  "trace": [
    "Error: Dynamic require of \"http\" is not supported",
    "    at file:///var/task/src/handler/CellDataHandler.mjs:12:9",
    "    at _Http.setupOnce (/node_modules/.pnpm/@sentry+node@7.108.0/node_modules/@sentry/src/integrations/http.ts:200:24)",
    "    at setupIntegration (/node_modules/.pnpm/@sentry+core@7.108.0/node_modules/@sentry/src/integration.ts:134:105)",
    "    at <anonymous> (/node_modules/.pnpm/@sentry+core@7.108.0/node_modules/@sentry/src/integration.ts:104:7)",
    "    at Array.forEach (<anonymous>)",
    "    at setupIntegrations (/node_modules/.pnpm/@sentry+core@7.108.0/node_modules/@sentry/src/integration.ts:101:16)",
    "    at NodeClient._setupIntegrations (/node_modules/.pnpm/@sentry+core@7.108.0/node_modules/@sentry/src/baseclient.ts:559:20)",
    "    at NodeClient.init (/node_modules/.pnpm/@sentry+core@7.108.0/node_modules/@sentry/src/baseclient.ts:333:12)",
    "    at initializeClient (/node_modules/.pnpm/@sentry+core@7.108.0/node_modules/@sentry/src/sdk.ts:59:12)",
    "    at initAndBind (/node_modules/.pnpm/@sentry+core@7.108.0/node_modules/@sentry/src/sdk.ts:38:3)"
  ]
}

Finally we tried to mark @sentry/serverless as external, but then lambda throws an error with @serverless/utils package:

{
  "errorType": "Runtime.ImportModuleError",
  "errorMessage": "Error: Cannot find module '@sentry/utils'\nRequire stack:\n- /var/task/node_modules/@sentry/serverless/cjs/awslambda.js\n- /var/task/node_modules/@sentry/serverless/cjs/index.js",
  "trace": [
    "Runtime.ImportModuleError: Error: Cannot find module '@sentry/utils'",
    "Require stack:",
    "- /var/task/node_modules/@sentry/serverless/cjs/awslambda.js",
    "- /var/task/node_modules/@sentry/serverless/cjs/index.js",
    "    at _loadUserApp (file:///var/runtime/index.mjs:1087:17)",
    "    at async UserFunction.js.module.exports.load (file:///var/runtime/index.mjs:1119:21)",
    "    at async start (file:///var/runtime/index.mjs:1282:23)",
    "    at async file:///var/runtime/index.mjs:1288:1"
  ]
}

If we comment every code related with sentry our code is executed correctly.

Our tsconfig.json:

{
  "$schema": "https://json.schemastore.org/tsconfig",
  "display": "Node 20",
  "compilerOptions": {
    "allowSyntheticDefaultImports": true,
    "emitDecoratorMetadata": true,
    "esModuleInterop": true,
    "experimentalDecorators": true,
    "forceConsistentCasingInFileNames": true,
    "incremental": true,
    "lib": ["ES2023"],
    "module": "nodenext",
    "moduleResolution": "bundler",
    "outDir": "build",
    "removeComments": true,
    "sourceMap": true,
    "skipLibCheck": true,
    "target": "ES2022",
    "strictNullChecks": false
  },
  "include": [
    "./src/**/*.ts"
  ],
}

And our esbuild.config.js

// @ts-check

module.exports = (serverless) => ({
    mainFields: ['module', 'main'],
    packager: 'pnpm',
    platform: 'node',
    target: 'node20',
    format: 'esm',
    bundle: true,
    outputFileExtension: '.mjs',
    sourcemap: 'inline',
    tsconfig: './tsconfig.json',
    // minify: true,
    // external: ['@sentry/serverless']
});

I've contact with Sentry support but they said that they never seen this error before and refer to this issue 🤷

@AbhiPrasad
Copy link
Member

We're updating our projects to use ESM and we have issues when is a Serverless Framework for AWS Lambda with Typescript and esbuild. When is CommonJS project there isn't any problem. The issues are in local (sls invoke local) and deployed in AWS.

The current serverless package is not esm compatible, we ship with require calls in the node package. We'll fix the node version issues with our next major version release #9508

@AbhiPrasad
Copy link
Member

Hi this should be resolved with 8.x of the Sentry Node SDK: https://docs.sentry.io/platforms/javascript/guides/node/migration/v7-to-v8/

We are also actively building an SDK for solidstart: #11293

and we released an official SDK for solid! https://www.npmjs.com/package/@sentry/solid

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Meta: Breaking Package: node Issues related to the Sentry Node SDK Type: Bug
Projects
Archived in project
Archived in project
Development

No branches or pull requests

7 participants