feat: withScope returns from wrapped function

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.

Author: rhcarvalho

packages/hub/src/hub.ts

@@ -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();
     }

packages/hub/test/hub.test.ts

@@ -121,16 +121,20 @@ describe('Hub', () => {
   });
 
   describe('withScope', () => {
+    let hub: Hub;
+
+    beforeEach(() => {
+      hub = new Hub();
+    });
+
     test('simple', () => {
-      const hub = new Hub();
       hub.withScope(() => {
         expect(hub.getStack()).toHaveLength(2);
       });
       expect(hub.getStack()).toHaveLength(1);
     });
 
     test('bindClient', () => {
-      const hub = new Hub();
       const testClient: any = { bla: 'a' };
       hub.withScope(() => {
         hub.bindClient(testClient);
@@ -139,6 +143,27 @@ describe('Hub', () => {
       });
       expect(hub.getStack()).toHaveLength(1);
     });
+
+    test('should bubble up exceptions', () => {
+      const error = new Error('test');
+      expect(() => {
+        hub.withScope(() => {
+          throw error;
+        });
+      }).toThrow(error);
+    });
+
+    test('should return 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 value = hub.withScope(someFn); // runs someFn in a new scope
+      expect(value).toBe('ok');
+    });
   });
 
   test('getCurrentClient', () => {

packages/minimal/src/index.ts

@@ -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);
 }
 
 /**

packages/minimal/test/lib/minimal.test.ts

@@ -244,7 +244,7 @@ describe('Minimal', () => {
   });
 
   test('withScope', () => {
-    withScope(scope => {
+    const value = withScope(scope => {
       scope.setLevel(Severity.Warning);
       scope.setFingerprint(['1']);
       withScope(scope2 => {
@@ -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', () => {

packages/types/src/hub.ts

@@ -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;