Skip to content

Commit f2a4caa

Browse files
authored
feat(core): Update withScope to return callback return value (#9866)
To align this with OpenTelemetry and make some things possible that are currently not easily doable without `pushScope` / `popScope`. Noticed this because currently it's not easily possible to e.g. use `withScope` in places like [this](#9862 (comment)). This should be backwards compatible because any code that previously relied on this returning `void` should still work.
1 parent db4bef1 commit f2a4caa

File tree

5 files changed

+99
-55
lines changed

5 files changed

+99
-55
lines changed

packages/core/src/exports.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -163,8 +163,8 @@ export function setUser(user: User | null): ReturnType<Hub['setUser']> {
163163
*
164164
* @param callback that will be enclosed into push/popScope.
165165
*/
166-
export function withScope(callback: (scope: Scope) => void): ReturnType<Hub['withScope']> {
167-
getCurrentHub().withScope(callback);
166+
export function withScope<T>(callback: (scope: Scope) => T): T {
167+
return getCurrentHub().withScope(callback);
168168
}
169169

170170
/**

packages/core/src/hub.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -161,10 +161,10 @@ export class Hub implements HubInterface {
161161
/**
162162
* @inheritDoc
163163
*/
164-
public withScope(callback: (scope: Scope) => void): void {
164+
public withScope<T>(callback: (scope: Scope) => T): T {
165165
const scope = this.pushScope();
166166
try {
167-
callback(scope);
167+
return callback(scope);
168168
} finally {
169169
this.popScope();
170170
}
+53
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,53 @@
1+
import { Hub, Scope, getCurrentScope, makeMain, withScope } from '../../src';
2+
import { TestClient, getDefaultTestClientOptions } from '../mocks/client';
3+
4+
function getTestClient(): TestClient {
5+
return new TestClient(
6+
getDefaultTestClientOptions({
7+
dsn: 'https://username@domain/123',
8+
}),
9+
);
10+
}
11+
12+
describe('withScope', () => {
13+
beforeEach(() => {
14+
const client = getTestClient();
15+
const hub = new Hub(client);
16+
makeMain(hub);
17+
});
18+
19+
it('works without a return value', () => {
20+
const scope1 = getCurrentScope();
21+
expect(scope1).toBeInstanceOf(Scope);
22+
23+
scope1.setTag('foo', 'bar');
24+
25+
const res = withScope(scope => {
26+
expect(scope).toBeInstanceOf(Scope);
27+
expect(scope).not.toBe(scope1);
28+
expect(scope['_tags']).toEqual({ foo: 'bar' });
29+
30+
expect(getCurrentScope()).toBe(scope);
31+
});
32+
33+
expect(getCurrentScope()).toBe(scope1);
34+
expect(res).toBe(undefined);
35+
});
36+
37+
it('works with a return value', () => {
38+
const res = withScope(scope => {
39+
return 'foo';
40+
});
41+
42+
expect(res).toBe('foo');
43+
});
44+
45+
it('works with an async function', async () => {
46+
const res = withScope(async scope => {
47+
return 'foo';
48+
});
49+
50+
expect(res).toBeInstanceOf(Promise);
51+
expect(await res).toBe('foo');
52+
});
53+
});

packages/feedback/src/util/sendFeedbackRequest.ts

+41-50
Original file line numberDiff line numberDiff line change
@@ -33,67 +33,58 @@ export async function sendFeedbackRequest(
3333
type: 'feedback',
3434
};
3535

36-
return new Promise((resolve, reject) => {
37-
withScope(async scope => {
38-
// No use for breadcrumbs in feedback
39-
scope.clearBreadcrumbs();
40-
41-
if ([FEEDBACK_API_SOURCE, FEEDBACK_WIDGET_SOURCE].includes(String(source))) {
42-
scope.setLevel('info');
43-
}
36+
return withScope(async scope => {
37+
// No use for breadcrumbs in feedback
38+
scope.clearBreadcrumbs();
39+
40+
if ([FEEDBACK_API_SOURCE, FEEDBACK_WIDGET_SOURCE].includes(String(source))) {
41+
scope.setLevel('info');
42+
}
43+
44+
const feedbackEvent = await prepareFeedbackEvent({
45+
scope,
46+
client,
47+
event: baseEvent,
48+
});
4449

45-
const feedbackEvent = await prepareFeedbackEvent({
46-
scope,
47-
client,
48-
event: baseEvent,
49-
});
50+
if (!feedbackEvent) {
51+
return;
52+
}
5053

51-
if (!feedbackEvent) {
52-
resolve();
53-
return;
54-
}
54+
if (client.emit) {
55+
client.emit('beforeSendFeedback', feedbackEvent, { includeReplay: Boolean(includeReplay) });
56+
}
5557

56-
if (client.emit) {
57-
client.emit('beforeSendFeedback', feedbackEvent, { includeReplay: Boolean(includeReplay) });
58-
}
58+
const envelope = createEventEnvelope(feedbackEvent, dsn, client.getOptions()._metadata, client.getOptions().tunnel);
5959

60-
const envelope = createEventEnvelope(
61-
feedbackEvent,
62-
dsn,
63-
client.getOptions()._metadata,
64-
client.getOptions().tunnel,
65-
);
60+
let response: void | TransportMakeRequestResponse;
6661

67-
let response: void | TransportMakeRequestResponse;
62+
try {
63+
response = await transport.send(envelope);
64+
} catch (err) {
65+
const error = new Error('Unable to send Feedback');
6866

6967
try {
70-
response = await transport.send(envelope);
71-
} catch (err) {
72-
const error = new Error('Unable to send Feedback');
73-
74-
try {
75-
// In case browsers don't allow this property to be writable
76-
// @ts-expect-error This needs lib es2022 and newer
77-
error.cause = err;
78-
} catch {
79-
// nothing to do
80-
}
81-
reject(error);
68+
// In case browsers don't allow this property to be writable
69+
// @ts-expect-error This needs lib es2022 and newer
70+
error.cause = err;
71+
} catch {
72+
// nothing to do
8273
}
74+
throw error;
75+
}
8376

84-
// TODO (v8): we can remove this guard once transport.send's type signature doesn't include void anymore
85-
if (!response) {
86-
resolve(response);
87-
return;
88-
}
77+
// TODO (v8): we can remove this guard once transport.send's type signature doesn't include void anymore
78+
if (!response) {
79+
return;
80+
}
8981

90-
// Require valid status codes, otherwise can assume feedback was not sent successfully
91-
if (typeof response.statusCode === 'number' && (response.statusCode < 200 || response.statusCode >= 300)) {
92-
reject(new Error('Unable to send Feedback'));
93-
}
82+
// Require valid status codes, otherwise can assume feedback was not sent successfully
83+
if (typeof response.statusCode === 'number' && (response.statusCode < 200 || response.statusCode >= 300)) {
84+
throw new Error('Unable to send Feedback');
85+
}
9486

95-
resolve(response);
96-
});
87+
return response;
9788
});
9889
}
9990

packages/types/src/hub.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -65,7 +65,7 @@ export interface Hub {
6565
*
6666
* @param callback that will be enclosed into push/popScope.
6767
*/
68-
withScope(callback: (scope: Scope) => void): void;
68+
withScope<T>(callback: (scope: Scope) => T): T;
6969

7070
/** Returns the client of the top stack. */
7171
getClient(): Client | undefined;

0 commit comments

Comments
 (0)