10000 feat(node-experimental): Move `errorHandler` (#10728) · GingerAdonis/sentry-javascript@a02aa00 · GitHub
[go: up one dir, main page]
Skip to content

Commit a02aa00

Browse files
mydeamydea
authored
feat(node-experimental): Move errorHandler (getsentry#10728)
Also stop exporting `requestHandler`, as that should be handled bu the otel http instrumentation. I also fixed the otel stuff to properly take care of sessions.
1 parent 28402cb commit a02aa00

File tree

7 files changed

+237
-10
lines changed

7 files changed

+237
-10
lines changed

‎dev-packages/node-integration-tests/suites/express/tracing-experimental/server.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,6 @@ app.get(['/test/arr/:id', /\/test\/arr[0-9]*\/required(path)?(\/optionalPath)?\/
3434
res.send({ response: 'response 4' });
3535
});
3636

37-
app.use(Sentry.Handlers.errorHandler());
37+
app.use(Sentry.errorHandler());
3838

3939
startExpressServerAndSendPortToRunner(app);

‎packages/node-experimental/src/index.ts

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,20 @@
1+
export { errorHandler } from './sdk/handlers/errorHandler';
2+
3+
export { httpIntegration } from './integrations/http';
4+
export { nativeNodeFetchIntegration } from './integrations/node-fetch';
15
export { expressIntegration } from './integrations/tracing/express';
26
export { fastifyIntegration } from './integrations/tracing/fastify';
37
export { graphqlIntegration } from './integrations/tracing/graphql';
4-
export { httpIntegration } from './integrations/http';
58
export { mongoIntegration } from './integrations/tracing/mongo';
69
export { mongooseIntegration } from './integrations/tracing/mongoose';
710
export { mysqlIntegration } from './integrations/tracing/mysql';
811
export { mysql2Integration } from './integrations/tracing/mysql2';
912
export { nestIntegration } from './integrations/tracing/nest';
10-
export { nativeNodeFetchIntegration } from './integrations/node-fetch';
1113
export { postgresIntegration } from './integrations/tracing/postgres';
1214
export { prismaIntegration } from './integrations/tracing/prisma';
1315

1416
export { init, getDefaultIntegrations } from './sdk/init';
1517
export { getAutoPerformanceIntegrations } from './integrations/tracing';
16-
export * as Handlers from './sdk/handlers';
1718
export type { Span } from './types';
1819

1920
export { startSpan, startSpanManual, startInactiveSpan, getActiveSpan, withActiveSpan } from '@sentry/opentelemetry';

‎packages/node-experimental/src/integrations/http.ts

9E7A
Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import { addBreadcrumb, defineIntegration, getIsolationScope, isSentryRequestUrl
88
import { _INTERNAL, getClient, getSpanKind, setSpanMetadata } from '@sentry/opentelemetry';
99
import type { IntegrationFn } from '@sentry/types';
1010

11+
import type { NodeClient } from '../sdk/client';
1112
import { setIsolationScope } from '../sdk/scope';
1213
import type { HTTPModuleRequestIncomingMessage } from '../transports/http-module';
1314
import { addOriginToSpan } from '../utils/addOriginToSpan';
@@ -86,14 +87,25 @@ const _httpIntegration = ((options: HttpOptions = {}) => {
8687
if (getSpanKind(span) === SpanKind.SERVER) {
8788
const isolationScope = getIsolationScope().clone();
8889
isolationScope.setSDKProcessingMetadata({ request: req });
89-
isolationScope.setRequestSession({ status: 'ok' });
90+
91+
const client = getClient<NodeClient>();
92+
if (client && client.getOptions().autoSessionTracking) {
93+
isolationScope.setRequestSession({ status: 'ok' });
94+
}
9095
setIsolationScope(isolationScope);
9196
}
9297
},
9398
responseHook: (span, res) => {
9499
if (_breadcrumbs) {
95100
_addRequestBreadcrumb(span, res);
96101
}
102+
103+
const client = getClient<NodeClient>();
104+
if (client && client.getOptions().autoSessionTracking) {
105+
setImmediate(() => {
106+
client['_captureRequestSession']();
107+
});
108+
}
97109
},
98110
}),
99111
];

‎packages/node-experimental/src/sdk/handlers.ts

Lines changed: 0 additions & 5 deletions
This file was deleted.

‎packages/node-experimental/src/sdk/handlers/errorHandler.ts

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
import type * as http from 'http';
2+
import { captureException, getClient, getIsolationScope } from '@sentry/core';
3+
import type { NodeClient } from '../client';
4+
5+
interface MiddlewareError extends Error {
6+
status?: number | string;
7+
statusCode?: number | string;
8+
status_code?: number | string;
9+
output?: {
10+
statusCode?: number | string;
11+
};
12+
}
13+
14+
/**
15+
* An Express-compatible error handler.
16+
*/
17+
export function errorHandler(options?: {
18+
/**
19+
* Callback method deciding whether error should be captured and sent to Sentry
20+
* @param error Captured middleware error
21+
*/
22+
shouldHandleError?(this: void, error: MiddlewareError): boolean;
23+
}): (
24+
error: MiddlewareError,
25+
req: http.IncomingMessage,
26+
res: http.ServerResponse,
27+
next: (error: MiddlewareError) => void,
28+
) => void {
29+
return function sentryErrorMiddleware(
30+
error: MiddlewareError,
31+
_req: http.IncomingMessage,
32+
res: http.ServerResponse,
33+
next: (error: MiddlewareError) => void,
34+
): void {
35+
const shouldHandleError = options?.shouldHandleError || defaultShouldHandleError;
36+
37+
if (shouldHandleError(error)) {
38+
const client = getClient<NodeClient>();
39+
if (client && client.getOptions().autoSessionTracking) {
40+
// Check if the `SessionFlusher` is instantiated on the client to go into this branch that marks the
41+
// `requestSession.status` as `Crashed`, and this check is necessary because the `SessionFlusher` is only
42+
// instantiated when the the`requestHandler` middleware is initialised, which indicates that we should be
43+
// running in SessionAggregates mode
44+
const isSessionAggregatesMode = client['_sessionFlusher'] !== undefined;
45+
if (isSessionAggregatesMode) {
46+
const requestSession = getIsolationScope().getRequestSession();
47+
// If an error bubbles to the `errorHandler`, then this is an unhandled error, and should be reported as a
48+
// Crashed session. The `_requestSession.status` is checked to ensure that this error is happening within
49+
// the bounds of a request, and if so the status is updated
50+
if (requestSession && requestSession.status !== undefined) {
51+
requestSession.status = 'crashed';
52+
}
53+
}
54+
}
55+
56+
const eventId = captureException(error, { mechanism: { type: 'middleware', handled: false } });
57+
(res as { sentry?: string }).sentry = eventId;
58+
next(error);
59+
60+
return;
61+
}
62+
63+
next(error);
64+
};
65+
}
66+
67+
function getStatusCodeFromResponse(error: MiddlewareError): number {
68+
const statusCode = error.status || error.statusCode || error.status_code || (error.output && error.output.statusCode);
69+
return statusCode ? parseInt(statusCode as string, 10) : 500;
70+
}
71+
72+
/** Returns true if response code is internal server error */
73+
function defaultShouldHandleError(error: MiddlewareError): boolean {
74+
const status = getStatusCodeFromResponse(error);
75+
return status >= 500;
76+
}

‎packages/node-experimental/src/sdk/init.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import {
22
endSession,
3+
getClient,
34
getCurrentScope,
45
getIntegrationsToSetup,
56
getIsolationScope,
@@ -184,6 +185,11 @@ function updateScopeFromEnvVariables(): void {
184185
* Enable automatic Session Tracking for the node process.
185186
*/
186187
function startSessionTracking(): void {
188+
const client = getClient<NodeClient>();
189+
if (client && client.getOptions().autoSessionTracking) {
190+
client.initSessionFlusher();
191+
}
192+
187193
startSession();
188194

189195
// Emitted in the case of healthy sessions, error of `mechanism.handled: true` and unhandledrejections because

‎packages/node-experimental/test/sdk/handlers/errorHandler.test.ts

Lines changed: 137 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,137 @@
1+
import * as http from 'http';
2+
import { getCurrentScope, getIsolationScope, setAsyncContextStrategy, setCurrentClient, withScope } from '@sentry/core';
3+
import type { Scope } from '@sentry/types';
4+
import { NodeClient } from '../../../src/sdk/client';
5+
import { errorHandler } from '../../../src/sdk/handlers/errorHandler';
6+
import { getDefaultNodeClientOptions } from '../../helpers/getDefaultNodePreviewClientOptions';
7+
8+
describe('errorHandler()', () => {
9+
beforeEach(() => {
10+
getCurrentScope().clear();
11+
getIsolationScope().clear();
12+
13+
setAsyncContextStrategy(undefined);
14+
});
15+
16+
const headers = { ears: 'furry', nose: 'wet', tongue: 'spotted', cookie: 'favorite=zukes' };
17+
const method = 'wagging';
18+
const protocol = 'mutualsniffing';
19+
const hostname = 'the.dog.park';
20+
const path = '/by/the/trees/';
21+
const queryString = 'chase=me&please=thankyou';
22+
23+
const sentryErrorMiddleware = errorHandler();
24+
25+
let req: http.IncomingMessage, res: http.ServerResponse, next: () => undefined;
26+
let client: NodeClient;
27+
28+
function createNoOpSpy() {
29+
const noop = { noop: () => undefined }; // this is wrapped in an object so jest can spy on it
30+
return jest.spyOn(noop, 'noop') as any;
31+
}
32+
33+
beforeEach(() => {
34+
req = {
35+
headers,
36+
method,
37+
protocol,
38+
hostname,
39+
originalUrl: `${path}?${queryString}`,
40+
} as unknown as http.IncomingMessage;
41+
res = new http.ServerResponse(req);
42+
next = createNoOpSpy();
43+
});
44+
45+
afterEach(() => {
46+
if (client['_sessionFlusher']) {
47+
clearInterval(client['_sessionFlusher']['_intervalId']);
48+
}
49+
jest.restoreAllMocks();
50+
});
51+
it('when autoSessionTracking is disabled, does not set requestSession status on Crash', done => {
52+
const options = getDefaultNodeClientOptions({ autoSessionTracking: false, release: '3.3' });
53+
client = new NodeClient(options);
54+
// It is required to initialise SessionFlusher to capture Session Aggregates (it is usually initialised
55+
// by the`requestHandler`)
56+
client.initSessionFlusher();
57+
58+
setCurrentClient(client);
59+
60+
jest.spyOn<any, any>(client, '_captureRequestSession');
61+
62+
getIsolationScope().setRequestSession({ status: 'ok' });
63+
64+
let isolationScope: Scope;
65+
sentryErrorMiddleware({ name: 'error', message: 'this is an error' }, req, res, () => {
66+
isolationScope = getIsolationScope();
67+
return next();
68+
});
69+
70+
setImmediate(() => {
71+
expect(isolationScope.getRequestSession()).toEqual({ status: 'ok' });
72+
done();
73+
});
74+
});
75+
76+
it('autoSessionTracking is enabled + requestHandler is not used -> does not set requestSession status on Crash', done => {
77+
const options = getDefaultNodeClientOptions({ autoSessionTracking: false, release: '3.3' });
78+
client = new NodeClient(options);
79+
setCurrentClient(client);
80+
81+
jest.spyOn<any, any>(client, '_captureRequestSession');
82+
83+
getIsolationScope().setRequestSession({ status: 'ok' });
84+
85+
let isolationScope: Scope;
86+
sentryErrorMiddleware({ name: 'error', message: 'this is an error' }, req, res, () => {
87+
isolationScope = getIsolationScope();
88+
return next();
89+
});
90+
91+
setImmediate(() => {
92+
expect(isolationScope.getRequestSession()).toEqual({ status: 'ok' });
93+
done();
94+
});
95+
});
96+
97+
it('when autoSessionTracking is enabled, should set requestSession status to Crashed when an unhandled error occurs within the bounds of a request', () => {
98+
const options = getDefaultNodeClientOptions({ autoSessionTracking: true, release: '1.1' });
99+
client = new NodeClient(options);
100+
// It is required to initialise SessionFlusher to capture Session Aggregates (it is usually initialised
101+
// by the`requestHandler`)
102+
client.initSessionFlusher();
103+
104+
setCurrentClient(client);
105+
106+
jest.spyOn<any, any>(client, '_captureRequestSession');
107+
108+
withScope(() => {
109+
getIsolationScope().setRequestSession({ status: 'ok' });
110+
sentryErrorMiddleware({ name: 'error', message: 'this is an error' }, req, res, () => {
111+
expect(getIsolationScope().getRequestSession()).toEqual({ status: 'crashed' });
112+
});
113+
});
114+
});
115+
116+
it('when autoSessionTracking is enabled, should not set requestSession status on Crash when it occurs outside the bounds of a request', done => {
117+
const options = getDefaultNodeClientOptions({ autoSessionTracking: true, release: '2.2' });
118+
client = new NodeClient(options);
119+
// It is required to initialise SessionFlusher to capture Session Aggregates (it is usually initialised
120+
// by the`requestHandler`)
121+
client.initSessionFlusher();
122+
setCurrentClient(client);
123+
124+
jest.spyOn<any, any>(client, '_captureRequestSession');
125+
126+
let isolationScope: Scope;
127+
sentryErrorMiddleware({ name: 'error', message: 'this is an error' }, req, res, () => {
128+
isolationScope = getIsolationScope();
129+
return next();
130+
});
131+
132+
setImmediate(() => {
133+
expect(isolationScope.getRequestSession()).toEqual(undefined);
134+
done();
135+
});
136+
});
137+
});

0 commit comments

Comments
 (0)
0