Skip to content

feat(core): Update withScope to return callback return value #9866

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Dec 18, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions packages/core/src/exports.ts
Original file line number Diff line number Diff line change
Expand Up @@ -163,8 +163,8 @@ export function setUser(user: User | null): ReturnType<Hub['setUser']> {
*
* @param callback that will be enclosed into push/popScope.
*/
export function withScope(callback: (scope: Scope) => void): ReturnType<Hub['withScope']> {
getCurrentHub().withScope(callback);
export function withScope<T>(callback: (scope: Scope) => T): T {
return getCurrentHub().withScope(callback);
}

/**
Expand Down
4 changes: 2 additions & 2 deletions packages/core/src/hub.ts
Original file line number Diff line number Diff line change
Expand Up @@ -161,10 +161,10 @@ export class Hub implements HubInterface {
/**
* @inheritDoc
*/
public withScope(callback: (scope: Scope) => void): void {
public withScope<T>(callback: (scope: Scope) => T): T {
const scope = this.pushScope();
try {
callback(scope);
return callback(scope);
} finally {
this.popScope();
}
Expand Down
53 changes: 53 additions & 0 deletions packages/core/test/lib/exports.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,53 @@
import { Hub, Scope, getCurrentScope, makeMain, withScope } from '../../src';
import { TestClient, getDefaultTestClientOptions } from '../mocks/client';

function getTestClient(): TestClient {
return new TestClient(
getDefaultTestClientOptions({
dsn: 'https://username@domain/123',
}),
);
}

describe('withScope', () => {
beforeEach(() => {
const client = getTestClient();
const hub = new Hub(client);
makeMain(hub);
});

it('works without a return value', () => {
const scope1 = getCurrentScope();
expect(scope1).toBeInstanceOf(Scope);

scope1.setTag('foo', 'bar');

const res = withScope(scope => {
expect(scope).toBeInstanceOf(Scope);
expect(scope).not.toBe(scope1);
expect(scope['_tags']).toEqual({ foo: 'bar' });

expect(getCurrentScope()).toBe(scope);
});

expect(getCurrentScope()).toBe(scope1);
expect(res).toBe(undefined);
});

it('works with a return value', () => {
const res = withScope(scope => {
return 'foo';
});

expect(res).toBe('foo');
});

it('works with an async function', async () => {
const res = withScope(async scope => {
return 'foo';
});

expect(res).toBeInstanceOf(Promise);
expect(await res).toBe('foo');
});
});
91 changes: 41 additions & 50 deletions packages/feedback/src/util/sendFeedbackRequest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,67 +33,58 @@ export async function sendFeedbackRequest(
type: 'feedback',
};

return new Promise((resolve, reject) => {
withScope(async scope => {
// No use for breadcrumbs in feedback
scope.clearBreadcrumbs();

if ([FEEDBACK_API_SOURCE, FEEDBACK_WIDGET_SOURCE].includes(String(source))) {
scope.setLevel('info');
}
return withScope(async scope => {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This actually shows how much better this is :D

// No use for breadcrumbs in feedback
scope.clearBreadcrumbs();

if ([FEEDBACK_API_SOURCE, FEEDBACK_WIDGET_SOURCE].includes(String(source))) {
scope.setLevel('info');
}

const feedbackEvent = await prepareFeedbackEvent({
scope,
client,
event: baseEvent,
});

const feedbackEvent = await prepareFeedbackEvent({
scope,
client,
event: baseEvent,
});
if (!feedbackEvent) {
return;
}

if (!feedbackEvent) {
resolve();
return;
}
if (client.emit) {
client.emit('beforeSendFeedback', feedbackEvent, { includeReplay: Boolean(includeReplay) });
}

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

const envelope = createEventEnvelope(
feedbackEvent,
dsn,
client.getOptions()._metadata,
client.getOptions().tunnel,
);
let response: void | TransportMakeRequestResponse;

let response: void | TransportMakeRequestResponse;
try {
response = await transport.send(envelope);
} catch (err) {
const error = new Error('Unable to send Feedback');

try {
response = await transport.send(envelope);
} catch (err) {
const error = new Error('Unable to send Feedback');

try {
// In case browsers don't allow this property to be writable
// @ts-expect-error This needs lib es2022 and newer
error.cause = err;
} catch {
// nothing to do
}
reject(error);
// In case browsers don't allow this property to be writable
// @ts-expect-error This needs lib es2022 and newer
error.cause = err;
} catch {
// nothing to do
}
throw error;
}

// TODO (v8): we can remove this guard once transport.send's type signature doesn't include void anymore
if (!response) {
resolve(response);
return;
}
// TODO (v8): we can remove this guard once transport.send's type signature doesn't include void anymore
if (!response) {
return;
}

// Require valid status codes, otherwise can assume feedback was not sent successfully
if (typeof response.statusCode === 'number' && (response.statusCode < 200 || response.statusCode >= 300)) {
reject(new Error('Unable to send Feedback'));
}
// Require valid status codes, otherwise can assume feedback was not sent successfully
if (typeof response.statusCode === 'number' && (response.statusCode < 200 || response.statusCode >= 300)) {
throw new Error('Unable to send Feedback');
}

resolve(response);
});
return response;
});
}

Expand Down
2 changes: 1 addition & 1 deletion packages/types/src/hub.ts
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ export interface Hub {
*
* @param callback that will be enclosed into push/popScope.
*/
withScope(callback: (scope: Scope) => void): void;
withScope<T>(callback: (scope: Scope) => T): T;

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