Skip to content

Commit

Permalink
feat: withScope returns from wrapped function
Browse files Browse the repository at this point in the history
This is part of a larger work towards better scope management.

This change makes Sentry.withScope and Hub.withScope easier to use with
existing functions that return a non-void type.

The motivation is that withScope could be used to wrap existing
functions and run them in a new scope without requiring a closure or
mutating state outside of the wrapped function (since it had to be
void).

While this does change the withScope signature, we're considering it
relatively low risk of breaking downstream code as we're making it more
general.
  • Loading branch information
rhcarvalho committed Jun 25, 2021
1 parent 61eda62 commit 52a83e3
Show file tree
Hide file tree
Showing 5 changed files with 25 additions and 10 deletions.
4 changes: 2 additions & 2 deletions packages/hub/src/hub.ts
Original file line number Diff line number Diff line change
Expand Up @@ -148,10 +148,10 @@ export class Hub implements HubInterface {
/**
* @inheritDoc
*/
public withScope(callback: (scope: Scope) => void): void {
public withScope<T>(fn: (scope: Scope) => T): T {
const scope = this.pushScope();
try {
callback(scope);
return fn(scope);
} finally {
this.popScope();
}
Expand Down
13 changes: 13 additions & 0 deletions packages/hub/test/hub.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,19 @@ describe('Hub', () => {
});
expect(hub.getStack()).toHaveLength(1);
});

test('returns return value from wrapped function', () => {
// someFn represents an existing function
const someFn = () => {
const hub = getCurrentHub();
hub.setTag('key', 'value');
hub.captureMessage('test');
return 'ok';
};
const hub = new Hub();
const value = hub.withScope(someFn); // wraps someFn to run it in a new scope
expect(value).toBe('ok');
});
});

test('getCurrentClient', () => {
Expand Down
8 changes: 4 additions & 4 deletions packages/minimal/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -170,13 +170,13 @@ export function setUser(user: User | null): void {
* This is essentially a convenience function for:
*
* pushScope();
* callback();
* fn();
* popScope();
*
* @param callback that will be enclosed into push/popScope.
* @param fn wrapped function.
*/
export function withScope(callback: (scope: Scope) => void): void {
callOnHub<void>('withScope', callback);
export function withScope<T>(fn: (scope: Scope) => T): T {
return callOnHub<T>('withScope', fn);
}

/**
Expand Down
4 changes: 3 additions & 1 deletion packages/minimal/test/lib/minimal.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -244,7 +244,7 @@ describe('Minimal', () => {
});

test('withScope', () => {
withScope(scope => {
const value = withScope(scope => {
scope.setLevel(Severity.Warning);
scope.setFingerprint(['1']);
withScope(scope2 => {
Expand All @@ -261,8 +261,10 @@ describe('Minimal', () => {
expect(global.__SENTRY__.hub._stack).toHaveLength(3);
});
expect(global.__SENTRY__.hub._stack).toHaveLength(2);
return 'ok';
});
expect(global.__SENTRY__.hub._stack).toHaveLength(1);
expect(value).toBe('ok');
});

test('setExtras', () => {
Expand Down
6 changes: 3 additions & 3 deletions packages/types/src/hub.ts
Original file line number Diff line number Diff line change
Expand Up @@ -61,12 +61,12 @@ export interface Hub {
* This is essentially a convenience function for:
*
* pushScope();
* callback();
* fn();
* popScope();
*
* @param callback that will be enclosed into push/popScope.
* @param fn wrapped function.
*/
withScope(callback: (scope: Scope) => void): void;
withScope<T>(fn: (scope: Scope) => T): T;

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

0 comments on commit 52a83e3

Please sign in to comment.