Skip to content

Commit b4643af

Browse files
mydeamydea
committed
Revert "revert: set request session on current scope"
This reverts commit fdb6bb5.
1 parent fdb6bb5 commit b4643af

File tree

2 files changed

+48
-74
lines changed

2 files changed

+48
-74
lines changed

‎packages/node/src/handlers.ts

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@ import {
2828
normalize,
2929
} from '@sentry/utils';
3030

31-
import { ServerRuntimeClient } from '@sentry/core';
3231
import type { NodeClient } from './client';
3332
import { DEBUG_BUILD } from './debug-build';
3433
// TODO (v8 / XXX) Remove this import
@@ -166,9 +165,9 @@ export function requestHandler(
166165
client.initSessionFlusher();
167166

168167
// If Scope contains a Single mode Session, it is removed in favor of using Session Aggregates mode
169-
const scope = getCurrentScope();
170-
if (scope.getSession()) {
171-
scope.setSession();
168+
const isolationScope = getIsolationScope();
169+
if (isolationScope.getSession()) {
170+
isolationScope.setSession();
172171
}
173172
}
174173

@@ -201,17 +200,19 @@ export function requestHandler(
201200
const client = getClient<NodeClient>();
202201
if (isAutoSessionTrackingEnabled(client)) {
203202
// Set `status` of `RequestSession` to Ok, at the beginning of the request
204-
getCurrentScope().setRequestSession({ status: 'ok' });
203+
isolationScope.setRequestSession({ status: 'ok' });
205204
}
206205

207206
res.once('finish', () => {
208-
const client = getClient<ServerRuntimeClient>();
209-
if (client && isAutoSessionTrackingEnabled(client)) {
207+
const client = getClient<NodeClient>();
208+
if (isAutoSessionTrackingEnabled(client)) {
210209
setImmediate(() => {
211-
if (client['_captureRequestSession']) {
210+
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
211+
if (client && (client as any)._captureRequestSession) {
212212
// Calling _captureRequestSession to capture request session at the end of the request by incrementing
213213
// the correct SessionAggregates bucket i.e. crashed, errored or exited
214-
client['_captureRequestSession']();
214+
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
215+
(client as any)._captureRequestSession();
215216
}
216217
});
217218
}
@@ -272,7 +273,7 @@ export function errorHandler(options?: {
272273
// The request should already have been stored in `scope.sdkProcessingMetadata` by `sentryRequestMiddleware`,
273274
// but on the off chance someone is using `sentryErrorMiddleware` without `sentryRequestMiddleware`, it doesn't
274275
// hurt to be sure
275-
getIsolationScope().setSDKProcessingMetadata({ request: _req });
276+
_scope.setSDKProcessingMetadata({ request: _req });
276277

277278
// For some reason we need to set the transaction on the scope again
278279
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access

‎packages/node/test/handlers.test.ts

Lines changed: 37 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ import {
1212
mergeScopeData,
1313
spanToJSON,
1414
} from '@sentry/core';
15-
import type { Event, PropagationContext, Scope } from '@sentry/types';
15+
import type { Event, PropagationContext } from '@sentry/types';
1616
import { SentryError } from '@sentry/utils';
1717

1818
import { NodeClient } from '../src/client';
@@ -61,24 +61,18 @@ describe('requestHandler', () => {
6161
jest.restoreAllMocks();
6262
});
6363

64-
it('autoSessionTracking is enabled, sets requestSession status to ok, when handling a request', done => {
64+
it('autoSessionTracking is enabled, sets requestSession status to ok, when handling a request', () => {
6565
const options = getDefaultNodeClientOptions({ autoSessionTracking: true, release: '1.2' });
6666
client = new NodeClient(options);
6767
// eslint-disable-next-line deprecation/deprecation
6868
const hub = new Hub(client);
6969
// eslint-disable-next-line deprecation/deprecation
7070
makeMain(hub);
7171

72-
let scope: Scope;
73-
sentryRequestMiddleware(req, res, () => {
74-
scope = getCurrentScope();
75-
return next();
76-
});
72+
sentryRequestMiddleware(req, res, next);
7773

78-
setImmediate(() => {
79-
expect(scope.getRequestSession()).toEqual({ status: 'ok' });
80-
done();
81-
});
74+
const isolationScope = getIsolationScope();
75+
expect(isolationScope.getRequestSession()).toEqual({ status: 'ok' });
8276
});
8377

8478
it('autoSessionTracking is disabled, does not set requestSession, when handling a request', () => {
@@ -89,18 +83,13 @@ describe('requestHandler', () => {
8983
// eslint-disable-next-line deprecation/deprecation
9084
makeMain(hub);
9185

92-
let scope: Scope;
93-
sentryRequestMiddleware(req, res, () => {
94-
scope = getCurrentScope();
95-
return next();
96-
});
86+
sentryRequestMiddleware(req, res, next);
9787

98-
setImmediate(() => {
99-
expect(scope.getRequestSession()).toBeUndefined();
100-
});
88+
const scope = getCurrentScope();
89+
expect(scope?.getRequestSession()).toBeUndefined();
10190
});
10291

103-
it('autoSessionTracking is enabled, calls _captureRequestSession, on response finish xxx', done => {
92+
it('autoSessionTracking is enabled, calls _captureRequestSession, on response finish', done => {
10493
const options = getDefaultNodeClientOptions({ autoSessionTracking: true, release: '1.2' });
10594
client = new NodeClient(options);
10695
// eslint-disable-next-line deprecation/deprecation
@@ -110,16 +99,13 @@ describe('requestHandler', () => {
11099

111100
const captureRequestSession = jest.spyOn<any, any>(client, '_captureRequestSession');
112101

113-
let scope: Scope;
114-
sentryRequestMiddleware(req, res, () => {
115-
scope = getCurrentScope();
116-
return next();
117-
});
102+
sentryRequestMiddleware(req, res, next);
118103

104+
const isolationScope = getIsolationScope();
119105
res.emit('finish');
120106

121107
setImmediate(() => {
122-
expect(scope.getRequestSession()).toEqual({ status: 'ok' });
108+
expect(isolationScope.getRequestSession()).toEqual({ status: 'ok' });
123109
expect(captureRequestSession).toHaveBeenCalled();
124110
done();
125111
});
@@ -136,10 +122,11 @@ describe('requestHandler', () => {
136122
const captureRequestSession = jest.spyOn<any, any>(client, '_captureRequestSession');
137123

138124
sentryRequestMiddleware(req, res, next);
125+
const scope = getCurrentScope();
139126
res.emit('finish');
140127

141128
setImmediate(() => {
142-
expect(getCurrentScope().getRequestSession()).toBeUndefined();
129+
expect(scope?.getRequestSession()).toBeUndefined();
143130
expect(captureRequestSession).not.toHaveBeenCalled();
144131
done();
145132
});
@@ -492,8 +479,10 @@ describe('tracingHandler', () => {
492479
const options = getDefaultNodeClientOptions({ tracesSampleRate: 1.0 });
493480
// eslint-disable-next-line deprecation/deprecation
494481
const hub = new Hub(new NodeClient(options));
482+
483+
jest.spyOn(sentryCore, 'getCurrentHub').mockReturnValue(hub);
495484
// eslint-disable-next-line deprecation/deprecation
496-
makeMain(hub);
485+
jest.spyOn(sentryCore, 'getCurrentScope').mockImplementation(() => hub.getScope());
497486

498487
sentryTracingMiddleware(req, res, next);
499488

@@ -549,40 +538,31 @@ describe('errorHandler()', () => {
549538
const scope = getCurrentScope();
550539
// eslint-disable-next-line deprecation/deprecation
551540
const hub = new Hub(client);
552-
// eslint-disable-next-line deprecation/deprecation
553-
makeMain(hub);
554541

555542
jest.spyOn<any, any>(client, '_captureRequestSession');
543+
jest.spyOn(sentryCore, 'getCurrentHub').mockReturnValue(hub);
556544

557-
scope.setRequestSession({ status: 'ok' });
545+
scope?.setRequestSession({ status: 'ok' });
558546
sentryErrorMiddleware({ name: 'error', message: 'this is an error' }, req, res, next);
559-
const requestSession = scope.getRequestSession();
547+
const requestSession = scope?.getRequestSession();
560548
expect(requestSession).toEqual({ status: 'ok' });
561549
});
562550

563-
it('autoSessionTracking is enabled + requestHandler is not used -> does not set requestSession status on Crash', done => {
551+
it('autoSessionTracking is enabled + requestHandler is not used -> does not set requestSession status on Crash', () => {
564552
const options = getDefaultNodeClientOptions({ autoSessionTracking: false, release: '3.3' });
565553
client = new NodeClient(options);
566554

555+
const scope = getCurrentScope();
567556
// eslint-disable-next-line deprecation/deprecation
568557
const hub = new Hub(client);
569-
// eslint-disable-next-line deprecation/deprecation
570-
makeMain(hub);
571558

572559
jest.spyOn<any, any>(client, '_captureRequestSession');
560+
jest.spyOn(sentryCore, 'getCurrentHub').mockReturnValue(hub);
573561

574-
getCurrentScope().setRequestSession({ status: 'ok' });
575-
576-
let scope: Scope;
577-
sentryErrorMiddleware({ name: 'error', message: 'this is an error' }, req, res, () => {
578-
scope = getCurrentScope();
579-
return next();
580-
});
581-
582-
setImmediate(() => {
583-
expect(scope.getRequestSession()).toEqual({ status: 'ok' });
584-
done();
585-
});
562+
scope?.setRequestSession({ status: 'ok' });
563+
sentryErrorMiddleware({ name: 'error', message: 'this is an error' }, req, res, next);
564+
const requestSession = scope?.getRequestSession();
565+
expect(requestSession).toEqual({ status: 'ok' });
586566
});
587567

588568
it('when autoSessionTracking is enabled, should set requestSession status to Crashed when an unhandled error occurs within the bounds of a request', () => {
@@ -600,16 +580,16 @@ describe('errorHandler()', () => {
600580
jest.spyOn<any, any>(client, '_captureRequestSession');
601581

602582
hub.run(() => {
603-
scope.setRequestSession({ status: 'ok' });
583+
scope?.setRequestSession({ status: 'ok' });
604584
sentryErrorMiddleware({ name: 'error', message: 'this is an error' }, req, res, () => {
605585
const scope = getCurrentScope();
606-
const requestSession = scope.getRequestSession();
586+
const requestSession = scope?.getRequestSession();
607587
expect(requestSession).toEqual({ status: 'crashed' });
608588
});
609589
});
610590
});
611591

612-
it('when autoSessionTracking is enabled, should not set requestSession status on Crash when it occurs outside the bounds of a request', done => {
592+
it('when autoSessionTracking is enabled, should not set requestSession status on Crash when it occurs outside the bounds of a request', () => {
613593
const options = getDefaultNodeClientOptions({ autoSessionTracking: true, release: '2.2' });
614594
client = new NodeClient(options);
615595
// It is required to initialise SessionFlusher to capture Session Aggregates (it is usually initialised
@@ -618,21 +598,13 @@ describe('errorHandler()', () => {
618598
const scope = new ScopeClass();
619599
// eslint-disable-next-line deprecation/deprecation
620600
const hub = new Hub(client, scope);
621-
// eslint-disable-next-line deprecation/deprecation
622-
makeMain(hub);
623601

624602
jest.spyOn<any, any>(client, '_captureRequestSession');
603+
jest.spyOn(sentryCore, 'getCurrentHub').mockReturnValue(hub);
625604

626-
let currentScope: Scope;
627-
sentryErrorMiddleware({ name: 'error', message: 'this is an error' }, req, res, () => {
628-
currentScope = getCurrentScope();
629-
return next();
630-
});
631-
632-
setImmediate(() => {
633-
expect(currentScope.getRequestSession()).toEqual(undefined);
634-
done();
635-
});
605+
sentryErrorMiddleware({ name: 'error', message: 'this is an error' }, req, res, next);
606+
const requestSession = scope?.getRequestSession();
607+
expect(requestSession).toEqual(undefined);
636608
});
637609

638610
it('stores request in `sdkProcessingMetadata`', () => {
@@ -648,8 +620,9 @@ describe('errorHandler()', () => {
648620
// `captureException` in order to examine the scope as it exists inside the `withScope` callback
649621
// eslint-disable-next-line deprecation/deprecation
650622
hub.captureException = function (this: Hub, _exception: any) {
651-
const scope = getIsolationScope();
652-
expect(scope.getScopeData().sdkProcessingMetadata.request).toEqual(req);
623+
// eslint-disable-next-line deprecation/deprecation
624+
const scope = this.getScope();
625+
expect((scope as any)._sdkProcessingMetadata.request).toEqual(req);
653626
} as any;
654627

655628
sentryErrorMiddleware({ name: 'error', message: 'this is an error' }, req, res, next);

0 commit comments

Comments
 (0)