You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
I know it comes with sentry because I tried to remove Sentry.init and the error disappears.
In my project, I'm using the async_hooks API and I fear a conflict with your tracing SDK. I am using async_hooks this way:
Sentry.init({environment: process.env.NODE_ENV,dsn: '__',integrations: [// enable HTTP calls tracingnewSentry.Integrations.Http({tracing: true}),// enable Express.js middleware tracingnewTracing.Integrations.Express({app: router}),],// Set tracesSampleRate to 1.0 to capture 100%// of transactions for performance monitoring.// We recommend adjusting this value in productiontracesSampleRate: 1.0,});// RequestHandler creates a separate execution context using domains, so that every// transaction/span/breadcrumb is attached to its own Hub instancerouter.use(Sentry.Handlers.requestHandler());// TracingHandler creates a trace for every incoming requestrouter.use(Sentry.Handlers.tracingHandler());
Update: issue seems to not appear with node:14-alpine and node:15-alpine. (Only my analyze, maybe I'm wrong)
Is issue also exists but it is not shown ? Or Node v16 brings a breaking change ?
importexpressfrom"express";import*asSentryfrom"@sentry/node";// or using CommonJS// const express = require('express');// const Sentry = require('@sentry/node');constapp=express();Sentry.init({dsn: "https://examplePublicKey@o0.ingest.sentry.io/0"});// The request handler must be the first middleware on the appapp.use(Sentry.Handlers.requestHandler());// All controllers should live hereapp.get("/",functionrootHandler(req,res){res.end("Hello world!");});// The error handler must be before any other error middleware and after all controllersapp.use(Sentry.Handlers.errorHandler());// Optional fallthrough error handlerapp.use(functiononError(err,req,res,next){// The error id is attached to `res.sentry` to be returned// and optionally displayed to the user for support.res.statusCode=500;res.end(res.sentry+"\n");});app.listen(3000);
The server can start successfully, however, if hit any endpoint such as going to http://localhost:3000 in this case, the server will crash with error
It's most likely that error thrown within async_hooks context is not correctly propagated to the domain, and it crashes the process instead of being handled there. Might be related:
One additional information here though, is that what's causing an actual error that is crashing the process, is TCP not being able to connect to the OpenTelemetry server, which you can see when using Node v16.1.0
{"stack":"Error: connect ECONNREFUSED 127.0.0.1:55681\n at TCPConnectWrap.afterConnect [as oncomplete] (node:net:1133:16)\n at TCPConnectWrap.callbackTrampoline (node:internal/async_hooks:131:17)","message":"connect ECONNREFUSED 127.0.0.1:55681","errno":"-61","code":"ECONNREFUSED","syscall":"connect","address":"127.0.0.1","port":"55681","name":"Error"}
55681 is default port for OpenTelemetry, and because you are using CollectorTraceExporter, and not providing url override, it'll try to send request to localhost:55681, which will reject, and effectively fail and crash the process.
There's nothing that we can do from our side right now, as it's infrastructure specific issue, not the SDK itself.
We need to wait for Node to state clearly if it's going to be fixed, and if not, then update docs appropriately.
Thanks @kamilogorek!
I am aware of I don't have OpenTelemetry server in this demo repo. I am using this as a demo to reproduce the bug.
Yeah just confirmed it only started from 16.2.0. Opened a ticket on Node.js side at nodejs/node#38814.
Activity
Tchoupinax commentedon May 22, 2021
Update: issue seems to not appear with
node:14-alpineandnode:15-alpine. (Only my analyze, maybe I'm wrong)Is issue also exists but it is not shown ? Or Node v16 brings a breaking change ?
hongbo-miao commentedon May 26, 2021
Got same error, only appears in Node.js 16. Not in Node.js 14 or 15. Copy useful info here:
Package + Version
@sentry/nodeVersion:
Description
Use Node.js 16.2.0 (latest version writing this), and simply use the demo at
https://docs.sentry.io/platforms/node/guides/express/
The server can start successfully, however, if hit any endpoint such as going to http://localhost:3000 in this case, the server will crash with error
I located, without line
app.use(Sentry.Handlers.requestHandler());, it won't have this issue.kamilogorek commentedon May 26, 2021
I just verified it locally, using exactly version
16.2.0and had no luck reproducing it 🤔hongbo-miao commentedon May 26, 2021
@kamilogorek I will try to provide a repo with more detailed steps to reproduce today. Thanks!
hongbo-miao commentedon May 26, 2021
@kamilogorek sorry for early misleading demo code.
Here you are! https://github.com/Hongbo-Miao/bug-nodejs-16
Just follow the step in the readme to reproduce.
I further did some research.
I found if removing OpenTelemetry by commenting out
https://github.com/Hongbo-Miao/bug-nodejs-16/blob/main/src/index.ts#L2
The server won't have any issue.
Or commenting out Sentry this line
https://github.com/Hongbo-Miao/bug-nodejs-16/blob/main/src/app.ts#L7
The server won't have any issue neither.
So I am guessing some code in OpenTelemetry affects Sentry.
kamilogorek commentedon May 27, 2021
This is some kind of regression in Node v16.2.0. It's working fine in v16.1.0.
The problem however, is incorrect request rejections handling between
domainsandasync_hooks. Even simplestdomain.create().run(() => next())as an express middleware, when used withasync_hooks(which are used in opentelemetry for context management - https://github.com/open-telemetry/opentelemetry-js/blob/dfb77372d311587c3462a43497c2ec4b3867e9fa/packages/opentelemetry-node/src/NodeTracerProvider.ts#L61-L63)` will throw an error. For some strange reason, when used together, they cannot talk with each other correctly.It's most likely that error thrown within
async_hookscontext is not correctly propagated to thedomain, and it crashes the process instead of being handled there. Might be related:nodejs/node#38503
nodejs/node#38145
One additional information here though, is that what's causing an actual error that is crashing the process, is TCP not being able to connect to the OpenTelemetry server, which you can see when using Node
v16.1.055681is default port for OpenTelemetry, and because you are usingCollectorTraceExporter, and not providingurloverride, it'll try to send request tolocalhost:55681, which will reject, and effectively fail and crash the process.There's nothing that we can do from our side right now, as it's infrastructure specific issue, not the SDK itself.
We need to wait for Node to state clearly if it's going to be fixed, and if not, then update docs appropriately.
hongbo-miao commentedon May 27, 2021
Thanks @kamilogorek!
I am aware of I don't have OpenTelemetry server in this demo repo. I am using this as a demo to reproduce the bug.
Yeah just confirmed it only started from 16.2.0. Opened a ticket on Node.js side at nodejs/node#38814.