Skip to content

Commit 804cbb6

Browse files
mydeamydea
committed
put request session on isolation scope
1 parent 54f809a commit 804cbb6

File tree

5 files changed

+202
-138
lines changed

5 files changed

+202
-138
lines changed

‎packages/core/src/server-runtime-client.ts

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ import { eventFromMessage, eventFromUnknownInput, logger, resolvedSyncPromise, u
1515

1616
import { BaseClient } from './baseclient';
1717
import { createCheckInEnvelope } from './checkin';
18-
import { getClient } from './currentScopes';
18+
import { getClient, getIsolationScope } from './currentScopes';
1919
import { DEBUG_BUILD } from './debug-build';
2020
import { MetricsAggregator } from './metrics/aggregator';
2121
import type { Scope } from './scope';
@@ -80,13 +80,13 @@ export class ServerRuntimeClient<
8080
/**
8181
* @inheritDoc
8282
*/
83-
// eslint-disable-next-line @typescript-eslint/no-explicit-any, @typescript-eslint/explicit-module-boundary-types
83+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
8484
public captureException(exception: any, hint?: EventHint, scope?: Scope): string | undefined {
8585
// Check if the flag `autoSessionTracking` is enabled, and if `_sessionFlusher` exists because it is initialised only
8686
// when the `requestHandler` middleware is used, and hence the expectation is to have SessionAggregates payload
8787
// sent to the Server only when the `requestHandler` middleware is used
88-
if (this._options.autoSessionTracking && this._sessionFlusher && scope) {
89-
const requestSession = scope.getRequestSession();
88+
if (this._options.autoSessionTracking && this._sessionFlusher) {
89+
const requestSession = getIsolationScope().getRequestSession();
9090

9191
// Necessary checks to ensure this is code block is executed only within a request
9292
// Should override the status only if `requestSession.status` is `Ok`, which is its initial stage
@@ -105,14 +105,14 @@ export class ServerRuntimeClient<
105105
// Check if the flag `autoSessionTracking` is enabled, and if `_sessionFlusher` exists because it is initialised only
106106
// when the `requestHandler` middleware is used, and hence the expectation is to have SessionAggregates payload
107107
// sent to the Server only when the `requestHandler` middleware is used
108-
if (this._options.autoSessionTracking && this._sessionFlusher && scope) {
108+
if (this._options.autoSessionTracking && this._sessionFlusher) {
109109
const eventType = event.type || 'exception';
110110
const isException =
111111
eventType === 'exception' && event.exception && event.exception.values && event.exception.values.length > 0;
112112

113113
// If the event is of type Exception, then a request session should be captured
114114
if (isException) {
115-
const requestSession = scope.getRequestSession();
115+
const requestSession = getIsolationScope().getRequestSession();
116116

117117
// Ensure that this is happening within the bounds of a request, and make sure not to override
118118
// Session Status if Errored / Crashed

‎packages/core/src/sessionflusher.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ import type {
66
SessionFlusherLike,
77
} from '@sentry/types';
88
import { dropUndefinedKeys } from '@sentry/utils';
9-
import { getCurrentScope } from './currentScopes';
9+
import { getIsolationScope } from './currentScopes';
1010

1111
type ReleaseHealthAttributes = {
1212
environment?: string;
@@ -74,14 +74,14 @@ export class SessionFlusher implements SessionFlusherLike {
7474
if (!this._isEnabled) {
7575
return;
7676
}
77-
const scope = getCurrentScope();
78-
const requestSession = scope.getRequestSession();
77+
const isolationScope = getIsolationScope();
78+
const requestSession = isolationScope.getRequestSession();
7979

8080
if (requestSession && requestSession.status) {
8181
this._incrementSessionStatusCount(requestSession.status, new Date());
8282
// This is not entirely necessarily but is added as a safe guard to indicate the bounds of a request and so in
8383
// case captureRequestSession is called more than once to prevent double count
84-
scope.setRequestSession(undefined);
84+
isolationScope.setRequestSession(undefined);
8585
/* eslint-enable @typescript-eslint/no-unsafe-member-access */
8686
}
8787
}

‎packages/node/src/handlers.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -273,7 +273,7 @@ export function errorHandler(options?: {
273273
// The request should already have been stored in `scope.sdkProcessingMetadata` by `sentryRequestMiddleware`,
274274
// but on the off chance someone is using `sentryErrorMiddleware` without `sentryRequestMiddleware`, it doesn't
275275
// hurt to be sure
276-
_scope.setSDKProcessingMetadata({ request: _req });
276+
getIsolationScope().setSDKProcessingMetadata({ request: _req });
277277

278278
// For some reason we need to set the transaction on the scope again
279279
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
@@ -292,7 +292,7 @@ export function errorHandler(options?: {
292292
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
293293
const isSessionAggregatesMode = (client as any)._sessionFlusher !== undefined;
294294
if (isSessionAggregatesMode) {
295-
const requestSession = _scope.getRequestSession();
295+
const requestSession = getIsolationScope().getRequestSession();
296296
// If an error bubbles to the `errorHandler`, then this is an unhandled error, and should be reported as a
297297
// Crashed session. The `_requestSession.status` is checked to ensure that this error is happening within
298298
// the bounds of a request, and if so the status is updated

‎packages/node/test/client.test.ts

Lines changed: 87 additions & 68 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,10 @@
11
import * as os from 'os';
2-
import { Scope, SessionFlusher } from '@sentry/core';
2+
import { SessionFlusher, getCurrentScope, getGlobalScope, getIsolationScope, withIsolationScope } from '@sentry/core';
33
import type { Event, EventHint } from '@sentry/types';
44

5+
import type { Scope } from '@sentry/types';
56
import { NodeClient } from '../src';
7+
import { setNodeAsyncContextStrategy } from '../src/async';
68
import { getDefaultNodeClientOptions } from './helper/node-client-options';
79

810
const PUBLIC_DSN = 'https://username@domain/123';
@@ -13,19 +15,30 @@ describe('NodeClient', () => {
1315
afterEach(() => {
1416
if ('_sessionFlusher' in client) clearInterval((client as any)._sessionFlusher._intervalId);
1517
jest.restoreAllMocks();
18+
19+
getIsolationScope().clear();
20+
getGlobalScope().clear();
21+
getCurrentScope().clear();
22+
getCurrentScope().setClient(undefined);
23+
});
24+
25+
beforeEach(() => {
26+
setNodeAsyncContextStrategy();
1627
});
1728

1829
describe('captureException', () => {
1930
test('when autoSessionTracking is enabled, and requestHandler is not used -> requestStatus should not be set', () => {
2031
const options = getDefaultNodeClientOptions({ dsn: PUBLIC_DSN, autoSessionTracking: true, release: '1.4' });
2132
client = new NodeClient(options);
22-
const scope = new Scope();
23-
scope.setRequestSession({ status: 'ok' });
2433

25-
client.captureException(new Error('test exception'), undefined, scope);
34+
withIsolationScope(isolationScope => {
35+
isolationScope.setRequestSession({ status: 'ok' });
2636

27-
const requestSession = scope.getRequestSession();
28-
expect(requestSession!.status).toEqual('ok');
37+
client.captureException(new Error('test exception'));
38+
39+
const requestSession = isolationScope.getRequestSession();
40+
expect(requestSession!.status).toEqual('ok');
41+
});
2942
});
3043

3144
test('when autoSessionTracking is disabled -> requestStatus should not be set', () => {
@@ -35,13 +48,14 @@ describe('NodeClient', () => {
3548
// by the`requestHandler`)
3649
client.initSessionFlusher();
3750

38-
const scope = new Scope();
39-
scope.setRequestSession({ status: 'ok' });
51+
withIsolationScope(isolationScope => {
52+
isolationScope.setRequestSession({ status: 'ok' });
4053

41-
client.captureException(new Error('test exception'), undefined, scope);
54+
client.captureException(new Error('test exception'));
4255

43-
const requestSession = scope.getRequestSession();
44-
expect(requestSession!.status).toEqual('ok');
56+
const requestSession = isolationScope.getRequestSession();
57+
expect(requestSession!.status).toEqual('ok');
58+
});
4559
});
4660

4761
test('when autoSessionTracking is enabled + requestSession status is Crashed -> requestStatus should not be overridden', () => {
@@ -51,13 +65,14 @@ describe('NodeClient', () => {
5165
// by the`requestHandler`)
5266
client.initSessionFlusher();
5367

54-
const scope = new Scope();
55-
scope.setRequestSession({ status: 'crashed' });
68+
withIsolationScope(isolationScope => {
69+
isolationScope.setRequestSession({ status: 'crashed' });
5670

57-
client.captureException(new Error('test exception'), undefined, scope);
71+
client.captureException(new Error('test exception'));
5872

59-
const requestSession = scope.getRequestSession();
60-
expect(requestSession!.status).toEqual('crashed');
73+
const requestSession = isolationScope.getRequestSession();
74+
expect(requestSession!.status).toEqual('crashed');
75+
});
6176
});
6277

6378
test('when autoSessionTracking is enabled + error occurs within request bounds -> requestStatus should be set to Errored', () => {
@@ -67,28 +82,37 @@ describe('NodeClient', () => {
6782
// by the`requestHandler`)
6883
client.initSessionFlusher();
6984

70-
const scope = new Scope();
71-
scope.setRequestSession({ status: 'ok' });
85+
withIsolationScope(isolationScope => {
86+
isolationScope.setRequestSession({ status: 'ok' });
7287

73-
client.captureException(new Error('test exception'), undefined, scope);
88+
client.captureException(new Error('test exception'));
7489

75-
const requestSession = scope.getRequestSession();
76-
expect(requestSession!.status).toEqual('errored');
90+
const requestSession = isolationScope.getRequestSession();
91+
expect(requestSession!.status).toEqual('errored');
92+
});
7793
});
7894

79-
test('when autoSessionTracking is enabled + error occurs outside of request bounds -> requestStatus should not be set to Errored', () => {
95+
test('when autoSessionTracking is enabled + error occurs outside of request bounds -> requestStatus should not be set to Errored', done => {
8096
const options = getDefaultNodeClientOptions({ dsn: PUBLIC_DSN, autoSessionTracking: true, release: '1.4' });
8197
client = new NodeClient(options);
98+
8299
// It is required to initialise SessionFlusher to capture Session Aggregates (it is usually initialised
83100
// by the`requestHandler`)
84101
client.initSessionFlusher();
85102

86-
const scope = new Scope();
103+
let isolationScope: Scope;
104+
withIsolationScope(_isolationScope => {
105+
_isolationScope.setRequestSession({ status: 'ok' });
106+
isolationScope = _isolationScope;
107+
});
87108

88-
client.captureException(new Error('test exception'), undefined, scope);
109+
client.captureException(new Error('test exception'));
89110

90-
const requestSession = scope.getRequestSession();
91-
expect(requestSession).toEqual(undefined);
111+
setImmediate(() => {
112+
const requestSession = isolationScope.getRequestSession();
113+
expect(requestSession).toEqual({ status: 'ok' });
114+
done();
115+
});
92116
});
93117
});
94118

@@ -100,16 +124,12 @@ describe('NodeClient', () => {
100124
// by the`requestHandler`)
101125
client.initSessionFlusher();
102126

103-
const scope = new Scope();
104-
scope.setRequestSession({ status: 'ok' });
105-
client.captureEvent(
106-
{ message: 'message', exception: { values: [{ type: 'exception type 1' }] } },
107-
undefined,
108-
scope,
109-
);
110-
111-
const requestSession = scope.getRequestSession();
112-
expect(requestSession!.status).toEqual('ok');
127+
withIsolationScope(isolationScope => {
128+
isolationScope.setRequestSession({ status: 'ok' });
129+
client.captureEvent({ message: 'message', exception: { values: [{ type: 'exception type 1' }] } });
130+
const requestSession = isolationScope.getRequestSession();
131+
expect(requestSession!.status).toEqual('ok');
132+
});
113133
});
114134

115135
test('When captureEvent is called with an exception, requestSession status should be set to Errored', () => {
@@ -119,13 +139,14 @@ describe('NodeClient', () => {
119139
// by the`requestHandler`)
120140
client.initSessionFlusher();
121141

122-
const scope = new Scope();
123-
scope.setRequestSession({ status: 'ok' });
142+
withIsolationScope(isolationScope => {
143+
isolationScope.setRequestSession({ status: 'ok' });
124144

125-
client.captureEvent({ message: 'message', exception: { values: [{ type: 'exception type 1' }] } }, {}, scope);
145+
client.captureEvent({ message: 'message', exception: { values: [{ type: 'exception type 1' }] } });
126146

127-
const requestSession = scope.getRequestSession();
128-
expect(requestSession!.status).toEqual('errored');
147+
const requestSession = isolationScope.getRequestSession();
148+
expect(requestSession!.status).toEqual('errored');
149+
});
129150
});
130151

131152
test('When captureEvent is called without an exception, requestSession status should not be set to Errored', () => {
@@ -135,13 +156,14 @@ describe('NodeClient', () => {
135156
// by the`requestHandler`)
136157
client.initSessionFlusher();
137158

138-
const scope = new Scope();
139-
scope.setRequestSession({ status: 'ok' });
159+
withIsolationScope(isolationScope => {
160+
isolationScope.setRequestSession({ status: 'ok' });
140161

141-
client.captureEvent({ message: 'message' }, {}, scope);
162+
client.captureEvent({ message: 'message' });
142163

143-
const requestSession = scope.getRequestSession();
144-
expect(requestSession!.status).toEqual('ok');
164+
const requestSession = isolationScope.getRequestSession();
165+
expect(requestSession!.status).toEqual('ok');
166+
});
145167
});
146168

147169
test('When captureEvent is called with an exception but outside of a request, then requestStatus should not be set', () => {
@@ -151,15 +173,12 @@ describe('NodeClient', () => {
151173
// by the`requestHandler`)
152174
client.initSessionFlusher();
153175

154-
const scope = new Scope();
155-
156-
client.captureEvent(
157-
{ message: 'message', exception: { values: [{ type: 'exception type 1' }] } },
158-
undefined,
159-
scope,
160-
);
176+
withIsolationScope(isolationScope => {
177+
isolationScope.clear();
178+
client.captureEvent({ message: 'message', exception: { values: [{ type: 'exception type 1' }] } });
161179

162-
expect(scope.getRequestSession()).toEqual(undefined);
180+
expect(isolationScope.getRequestSession()).toEqual(undefined);
181+
});
163182
});
164183

165184
test('When captureEvent is called with a transaction, then requestSession status should not be set', () => {
@@ -169,28 +188,28 @@ describe('NodeClient', () => {
169188
// by the`requestHandler`)
170189
client.initSessionFlusher();
171190

172-
const scope = new Scope();
173-
scope.setRequestSession({ status: 'ok' });
174-
client.captureEvent({ message: 'message', type: 'transaction' }, undefined, scope);
191+
withIsolationScope(isolationScope => {
192+
isolationScope.setRequestSession({ status: 'ok' });
193+
194+
client.captureEvent({ message: 'message', type: 'transaction' });
175195

176-
const requestSession = scope.getRequestSession();
177-
expect(requestSession!.status).toEqual('ok');
196+
const requestSession = isolationScope.getRequestSession();
197+
expect(requestSession!.status).toEqual('ok');
198+
});
178199
});
179200

180201
test('When captureEvent is called with an exception but requestHandler is not used, then requestSession status should not be set', () => {
181202
const options = getDefaultNodeClientOptions({ dsn: PUBLIC_DSN, autoSessionTracking: true, release: '1.3' });
182203
client = new NodeClient(options);
183204

184-
const scope = new Scope();
185-
scope.setRequestSession({ status: 'ok' });
186-
client.captureEvent(
187-
{ message: 'message', exception: { values: [{ type: 'exception type 1' }] } },
188-
undefined,
189-
scope,
190-
);
205+
withIsolationScope(isolationScope => {
206+
isolationScope.setRequestSession({ status: 'ok' });
207+
208+
client.captureEvent({ message: 'message', exception: { values: [{ type: 'exception type 1' }] } });
191209

192-
const requestSession = scope.getRequestSession();
193-
expect(requestSession!.status).toEqual('ok');
210+
const requestSession = isolationScope.getRequestSession();
211+
expect(requestSession!.status).toEqual('ok');
212+
});
194213
});
195214
});
196215

0 commit comments

Comments
 (0)