Skip to content

Commit 733cd67

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

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
@@ -27,7 +27,6 @@ import {
2727
normalize,
2828
} from '@sentry/utils';
2929

30-
import { ServerRuntimeClient } from '@sentry/core';
3130
import type { NodeClient } from './client';
3231
import { DEBUG_BUILD } from './debug-build';
3332
import { isAutoSessionTrackingEnabled } from './sdk';
@@ -132,9 +131,9 @@ export function requestHandler(
132131
client.initSessionFlusher();
133132

134133
// If Scope contains a Single mode Session, it is removed in favor of using Session Aggregates mode
135-
const scope = getCurrentScope();
136-
if (scope.getSession()) {
137-
scope.setSession();
134+
const isolationScope = getIsolationScope();
135+
if (isolationScope.getSession()) {
136+
isolationScope.setSession();
138137
}
139138
}
140139

@@ -165,17 +164,19 @@ export function requestHandler(
165164
const client = getClient<NodeClient>();
166165
if (isAutoSessionTrackingEnabled(client)) {
167166
// Set `status` of `RequestSession` to Ok, at the beginning of the request
168-
getCurrentScope().setRequestSession({ status: 'ok' });
167+
isolationScope.setRequestSession({ status: 'ok' });
169168
}
170169

171170
res.once('finish', () => {
172-
const client = getClient<ServerRuntimeClient>();
173-
if (client && isAutoSessionTrackingEnabled(client)) {
171+
const client = getClient<NodeClient>();
172+
if (isAutoSessionTrackingEnabled(client)) {
174173
setImmediate(() => {
175-
if (client['_captureRequestSession']) {
174+
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
175+
if (client && (client as any)._captureRequestSession) {
176176
// Calling _captureRequestSession to capture request session at the end of the request by incrementing
177177
// the correct SessionAggregates bucket i.e. crashed, errored or exited
178-
client['_captureRequestSession']();
178+
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
179+
(client as any)._captureRequestSession();
179180
}
180181
});
181182
}
@@ -236,7 +237,7 @@ export function errorHandler(options?: {
236237
// The request should already have been stored in `scope.sdkProcessingMetadata` by `sentryRequestMiddleware`,
237238
// but on the off chance someone is using `sentryErrorMiddleware` without `sentryRequestMiddleware`, it doesn't
238239
// hurt to be sure
239-
getIsolationScope().setSDKProcessingMetadata({ request: _req });
240+
_scope.setSDKProcessingMetadata({ request: _req });
240241

241242
// For some reason we need to set the transaction on the scope again
242243
// 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
});
@@ -491,8 +478,10 @@ describe('tracingHandler', () => {
491478
const options = getDefaultNodeClientOptions({ tracesSampleRate: 1.0 });
492479
// eslint-disable-next-line deprecation/deprecation
493480
const hub = new Hub(new NodeClient(options));
481+
482+
jest.spyOn(sentryCore, 'getCurrentHub').mockReturnValue(hub);
494483
// eslint-disable-next-line deprecation/deprecation
495-
makeMain(hub);
484+
jest.spyOn(sentryCore, 'getCurrentScope').mockImplementation(() => hub.getScope());
496485

497486
sentryTracingMiddleware(req, res, next);
498487

@@ -548,40 +537,31 @@ describe('errorHandler()', () => {
548537
const scope = getCurrentScope();
549538
// eslint-disable-next-line deprecation/deprecation
550539
const hub = new Hub(client);
551-
// eslint-disable-next-line deprecation/deprecation
552-
makeMain(hub);
553540

554541
jest.spyOn<any, any>(client, '_captureRequestSession');
542+
jest.spyOn(sentryCore, 'getCurrentHub').mockReturnValue(hub);
555543

556-
scope.setRequestSession({ status: 'ok' });
544+
scope?.setRequestSession({ status: 'ok' });
557545
sentryErrorMiddleware({ name: 'error', message: 'this is an error' }, req, res, next);
558-
const requestSession = scope.getRequestSession();
546+
const requestSession = scope?.getRequestSession();
559547
expect(requestSession).toEqual({ status: 'ok' });
560548
});
561549

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

554+
const scope = getCurrentScope();
566555
// eslint-disable-next-line deprecation/deprecation
567556
const hub = new Hub(client);
568-
// eslint-disable-next-line deprecation/deprecation
569-
makeMain(hub);
570557

571558
jest.spyOn<any, any>(client, '_captureRequestSession');
559+
jest.spyOn(sentryCore, 'getCurrentHub').mockReturnValue(hub);
572560

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

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

601581
hub.run(() => {
602-
scope.setRequestSession({ status: 'ok' });
582+
scope?.setRequestSession({ status: 'ok' });
603583
sentryErrorMiddleware({ name: 'error', message: 'this is an error' }, req, res, () => {
604584
const scope = getCurrentScope();
605-
const requestSession = scope.getRequestSession();
585+
const requestSession = scope?.getRequestSession();
606586
expect(requestSession).toEqual({ status: 'crashed' });
607587
});
608588
});
609589
});
610590

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

623601
jest.spyOn<any, any>(client, '_captureRequestSession');
602+
jest.spyOn(sentryCore, 'getCurrentHub').mockReturnValue(hub);
624603

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

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

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

0 commit comments

Comments
 (0)