Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork 1.8k
feat(node): Capture Express errors automatically via expressIntegration#23464
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
Uh oh!
There was an error while loading. Please reload this page.
Changes from all commits
55e28fceab3719e416f86eed35f70de5fc7054cbb2c42ffe90688da4d4ca9dbaee661d028c80fd72ff90File filter
Filter by extension
Conversations
Uh oh!
There was an error while loading. Please reload this page.
Jump to
Uh oh!
There was an error while loading. Please reload this page.
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -6,4 +6,7 @@ Sentry.init({ | ||
| dsn: 'https://public@dsn.ingest.sentry.io/1337', | ||
| release: '1.0', | ||
| transport: loggingTransport, | ||
| // With tracing off, `expressIntegration()` is not a default integration, so opt in explicitly to | ||
| // get automatic error capture. | ||
Comment on lines
+9
to
+10
Member There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this is no longer true. Express integration is in the "framework" set, so it's on by default, tracing or no. MemberAuthor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👍 can you include in a follow up? | ||
| integrations: [Sentry.expressIntegration()], | ||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| import * as Sentry from '@sentry/node'; | ||
| import { loggingTransport } from '@sentry-internal/node-integration-tests'; | ||
| // Disable the channel-based `expressIntegration()` so the deprecated `setupExpressErrorHandler` | ||
| // middleware is the sole error capturer (mechanism `auto.middleware.express`) — the fallback for | ||
| // setups where the channel-based auto capture is unavailable. | ||
| Sentry.init({ | ||
| traceLifecycle: 'static', | ||
| dsn: 'https://public@dsn.ingest.sentry.io/1337', | ||
| release: '1.0', | ||
| transport: loggingTransport, | ||
| integrations: integrations => integrations.filter(integration => integration.name !== 'Express'), | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,16 @@ | ||
| import * as Sentry from '@sentry/node'; | ||
| import { loggingTransport } from '@sentry-internal/node-integration-tests'; | ||
| Sentry.init({ | ||
| traceLifecycle: 'static', | ||
| dsn: 'https://public@dsn.ingest.sentry.io/1337', | ||
| release: '1.0', | ||
| transport: loggingTransport, | ||
| integrations: [ | ||
| Sentry.expressIntegration({ | ||
| shouldHandleError: error => { | ||
| return error.message === 'error_2'; | ||
| }, | ||
| }), | ||
| ], | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| import * as Sentry from '@sentry/node'; | ||
| import { startExpressServerAndSendPortToRunner } from '@sentry-internal/node-integration-tests'; | ||
| import cors from 'cors'; | ||
| import express from 'express'; | ||
| const app = express(); | ||
| app.use(cors()); | ||
| app.get('/test1', (_req, _res) => { | ||
| throw new Error('error_1'); | ||
| }); | ||
| app.get('/test2', (_req, _res) => { | ||
| throw new Error('error_2'); | ||
| }); | ||
| // With `expressIntegration` disabled (see the instrument file), the deprecated middleware is the sole | ||
| // capturer and its own `shouldHandleError` applies. | ||
| Sentry.setupExpressErrorHandler(app, { | ||
| shouldHandleError: error => error.message === 'error_2', | ||
| }); | ||
| startExpressServerAndSendPortToRunner(app); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,24 @@ | ||
| import * as Sentry from '@sentry/node'; | ||
| import { startExpressServerAndSendPortToRunner } from '@sentry-internal/node-integration-tests'; | ||
| import cors from 'cors'; | ||
| import express from 'express'; | ||
| const app = express(); | ||
| app.use(cors()); | ||
| app.get('/test1', (_req, _res) => { | ||
| throw new Error('error_1'); | ||
| }); | ||
| app.get('/test2', (_req, _res) => { | ||
| throw new Error('error_2'); | ||
| }); | ||
| // Deprecated, and here with a permissive (default) predicate that would capture both errors. But the | ||
| // channel-based `expressIntegration` (configured with `shouldHandleError: error_2` in the instrument) | ||
| // takes precedence: it is the single registered handler, so its predicate decides what is captured and | ||
| // this middleware must neither capture `error_1` nor double-capture `error_2`. | ||
| Sentry.setupExpressErrorHandler(app); | ||
| startExpressServerAndSendPortToRunner(app); |
Uh oh!
There was an error while loading. Please reload this page.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
q: A lot of
parent_span_ids were added in the assertions. Was anything changed that this changed or was added?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this is more correct now then before I think, what has changed here is in what span context the error is captured, we ensure to capture this in the span context of the route handler now instead of in the separate middleware which should be better than before.