getsentry / sentry-javascript

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

@sentry/bun throws "TypeError: Cannot replace module namespace object's binding with configurable attribute" #12891

Open EvHaus opened 2 months ago

EvHaus commented 2 months ago

Is there an existing issue for this?

How do you use Sentry?

Sentry Saas (sentry.io)

Which SDK are you using?

@sentry/bun

SDK Version

8.17.0

Framework Version

Bun 1.1.18

Link to Sentry event

https://reserve-sense.sentry.io/issues/5597727361/?project=4506890995499008

SDK Setup/Reproduction Example

import { init } from '@sentry/bun';

init({
    dsn: '__MY_DSN__',
    tracesSampleRate: 0.5,
});

Steps to Reproduce

I'm adding Sentry to my Bun API and I simply added a basic init() call at the start of my server and it immediately crashes with the error below.

I'm using node-file-router and looks like combining it with the init() call causes issues. My code for that is:

import { initFileRouter } from 'node-file-router';
const runFileRouter = await initFileRouter({
    ignoreFilesRegex: [/.\.test/],
});

Expected Result

No failure

Actual Result

App crashes with:

 9 |
10 | // Sets a property on an object, preserving its enumerability.
11 | // This function assumes that the property is already writable.
12 | function defineProperty (obj, name, value) {
13 |   var enumerable = !!obj[name] && obj.propertyIsEnumerable(name)
14 |   Object.defineProperty(obj, name, {
     ^
TypeError: Cannot replace module namespace object's binding with configurable attribute
      at defineProperty (native:1:1)
      at defineProperty (/myproject/node_modules/.pnpm/shimmer@1.2.1/node_modules/shimmer/index.js:14:10)
      at wrap (/myproject/node_modules/.pnpm/shimmer@1.2.1/node_modules/shimmer/index.js:74:3)
      at /myproject/node_modules/.pnpm/@opentelemetry+instrumentation-http@0.52.1_@opentelemetry+api@1.9.0/node_modules/@opentelemetry/instrumentation-http/build/src/http.js:78:13
      at /myproject/node_modules/.pnpm/@opentelemetry+instrumentation@0.52.1_@opentelemetry+api@1.9.0/node_modules/@opentelemetry/instrumentation/build/src/platform/node/RequireInTheMiddleSingleton.js:67:27
      at /myproject/node_modules/.pnpm/require-in-the-middle@7.3.0/node_modules/require-in-the-middle/index.js:281:28
      at /myproject/node_modules/.pnpm/@smithy+node-http-handler@3.1.2/node_modules/@smithy/node-http-handler/dist-cjs/index.js:150:3
      at anonymous (native:1:1)
      at /myproject/node_modules/.pnpm/require-in-the-middle@7.3.0/node_modules/require-in-the-middle/index.js:295:32
      at /myproject/node_modules/.pnpm/@smithy+util-stream@3.0.6/node_modules/@smithy/util-stream/dist-cjs/sdk-stream-mixin.js:4:1
chargome commented 2 months ago

@EvHaus thanks for reaching out. I could set up a small repo and use node-file-router without any issues. Are you using any auto-instrumentation e.g. for express? Also could you provide the output when you add debug: trueto your init call? Anything that could help me reproduce your error would be helpful!

EvHaus commented 2 months ago

Thanks for the quick response @chargome. Looks like I was missing one crucial element. Seems like it's an issue with @aws-sdk/client-s3. Simply importing from there causes the error.

The full repro is as follows:

// index.ts

import { initFileRouter } from 'node-file-router';
import { init } from '@sentry/bun';

init({dsn: '__MY_DSN__'});
await initFileRouter();
// api/index.ts

import { DeleteObjectCommand } from '@aws-sdk/client-s3';

const index = () => {
    console.log(DeleteObjectCommand);
};

export default index;

Then run the app via bun index.ts.

andreiborza commented 1 month ago

Hello, this seems to be an issue upstream with opentelemetry and shimmer. I'm afraid there's not much we can do right now, but we're looking into that.

To get you unstuck, you could set up a manual client and drop the http integration. This way you'll at least have error tracking and basic performance data (if you use Bun.serve), but you won't get opentelemetry instrumentation, for example db integrations.

import {
  BunClient,
  defaultStackParser,
  getDefaultIntegrations,
  makeFetchTransport,
  setCurrentClient
} from '@sentry/bun'

const integrations = getDefaultIntegrations({}).filter((integration) => integration.name !== 'Http')

const client = new BunClient({
  dsn: '__MY_DSN__',
  stackParser: defaultStackParser,
  transport: makeFetchTransport,
  tracesSampleRate: 1.0,
  integrations,
})

setCurrentClient(client)
client.init()

Hope this helps!

Tunanika commented 1 month ago

Dear @andreiborza I am currently facing the same

TypeError: Cannot replace module namespace object's binding with configurable attribute`

error. I was using @sentry/node bun after this issue, I have switched to @sentry/bun however, whenever I add the preload = ["./instrument.mjs"] in bunfig.toml I face the same error mentioned in this post. And when I remove it, it works in bun run index.mts but it does not work in production since the preload happens when we build the project. Is there any way to overcome this?

For reference: I am using Express with typescript and ESM syntax and this is the sentry debug output:

Sentry Logger [log]: Initializing Sentry: process: 73610, thread: main.
Sentry Logger [log]: Integration installed: InboundFilters
Sentry Logger [log]: Integration installed: FunctionToString
Sentry Logger [log]: Integration installed: LinkedErrors
Sentry Logger [log]: Integration installed: RequestData
Sentry Logger [log]: Integration installed: Console
Sentry Logger [log]: Integration installed: Http
Sentry Logger [log]: Integration installed: NodeFetch
Sentry Logger [log]: Integration installed: OnUncaughtException
Sentry Logger [log]: Integration installed: OnUnhandledRejection
Sentry Logger [log]: Integration installed: ContextLines
Sentry Logger [log]: Integration installed: Context
Sentry Logger [log]: Integration installed: Modules
Sentry Logger [log]: Integration installed: BunServer
Sentry Logger [log]: Running in CommonJS mode.
Sentry Logger [debug]: @opentelemetry/api: Registered a global for diag v1.9.0.
Sentry Logger [debug]: @opentelemetry/api: Registered a global for trace v1.9.0.
Sentry Logger [debug]: @opentelemetry/api: Registered a global for context v1.9.0.
Sentry Logger [debug]: @opentelemetry/api: Registered a global for propagation v1.9.0.
[Sentry] express is not instrumented. This is likely because you required/imported express before calling `Sentry.init()`.
Server is listening on port 4001

I have also realized that sentry is running in CommonJS mode. Is this normal? If not, what can I do to fix that issue too?

Thank you for your time, Tuna

chargome commented 1 month ago

@Tunanika there is no need to preload the instrument file in bunfig.toml, just import the instrumentation first thing in your index file.

You will still face the issue that Sentry.setupExpressErrorHandler(app) is currently not working as expected with bun. Your express routes will however be automatically instrumented with the httpIntegration, for additional performance tracing you can make use of custom instrumentation until there is a fix available.

Regarding capturing errors you could capture exceptions using a global error handler like this:

function customErrorHandler(
  err: Error,
  _req: Request,
  res: Response,
  _next: NextFunction
) {
  Sentry.captureException(err);
  // handle your error response
  res.status(500).send("sadness...");
}

app.use(customErrorHandler);

Hope that helps you get going for now!

Tunanika commented 1 month ago

Dear @chargome Thank you very much for your response. Yes sentry currently does capture any errors in express endpoints. However upon building the project withbun build index.mts --target bun --outdir ./out --sourcemap=externalwhen running the final index.js file with bun run index.js I face the same Cannot replace module namespace object's binding with configurable attribute issue like the following:

 9 | 
10 | // Sets a property on an object, preserving its enumerability.
11 | // This function assumes that the property is already writable.
12 | function defineProperty (obj, name, value) {
13 |   var enumerable = !!obj[name] && obj.propertyIsEnumerable(name)
14 |   Object.defineProperty(obj, name, {
     ^
TypeError: Cannot replace module namespace object's binding with configurable attribute

This is my instrument.mjs file:

import * as Sentry from "@sentry/bun";
// Ensure to call this before importing any other modules!
Sentry.init({
  dsn: "dsn",

  // Add Performance Monitoring by setting tracesSampleRate
  // Set tracesSampleRate to 1.0 to capture 100% of transactions
  // We recommend adjusting this value in production
  tracesSampleRate: 1.0,
});

Index.mts:

import "./instrument.mjs";
import * as Sentry from "@sentry/bun";
import express from "express";
import cors from "cors";
import morgan from "Morgan";
import cookieParser from "cookie-parser";
import type { Request, Response, NextFunction } from "express";
.
.
.
Sentry.setupExpressErrorHandler(app);

What can I do to fix the issue that it fails after building? (I have removed the preload from bunfig.toml )

chargome commented 1 month ago

Hm yeah that should not be the case, we're running into the same issue again this way.. Since you're not running this code in a browser would it be an option for you to just run your server using bun run index.ts for now?

Tunanika commented 1 month ago

Yes I can keep running the server that way it's not a problem. Thank you for your response, good to know it's not a problem with my integration.

i-void commented 1 month ago

I have the same issue with graphql-yoga integration, is there a workaround?

andreiborza commented 1 month ago

@i-void do you have a reproduction repo or a stackblitz of sorts we can look at? Otherwise, this is likely still an issue with bun and shimmer.

i-void commented 1 month ago

@andreiborza not have a public repo but I think you can reproduce by installing using their docs (its simple) https://the-guild.dev/graphql/yoga-server/docs/integrations/integration-with-bun

after this just add @sentry/bun and try to init it with Sentry.init. It gives error on init. I overcome the situation by disabling Http integration and switching to @sentry/node instead of Bun. Also I had another error says startTransaction function is undefined . I fix it by changing the @sentry/node's version to same as @sentry/tracing (v7.114.0). Same error happens in @sentry/bun also and changing version didn't help.

andreiborza commented 1 month ago

Thanks, we will take a look.

bephrem1 commented 3 weeks ago

Facing this same issue on v8.26.0 (@sentry/bun). Using @andreiborza's workaround in the meantime.

andreiborza commented 3 weeks ago

Hey thanks for chiming in. We're still looking into this a bit closer but are currently on limited support due to a company-wide hackweek. We'll get back to our regular support next week.

Jonatthu commented 3 weeks ago

@andreiborza I am experiencing the same issue in bun when I preload sentry.ts with the init config

chargome commented 2 weeks ago

@Jonatthu @bephrem1 we're aware of this, there is is an open upstream issue for that: https://github.com/oven-sh/bun/issues/13165