feat: Explicit Scope for captureException and captureMessage (#2627)

* feat: Explicit Scope for captureException and captureMessage

Author: kamilogorek

CHANGELOG.md

@@ -3,6 +3,7 @@
 ## Unreleased
 
 - "You miss 100 percent of the chances you don't take. — Wayne Gretzky" — Michael Scott
+- [minimal/core] feat: Allow for explicit scope through 2nd argument to `captureException/captureMessage` (#2627)
 
 ## 5.16.0
 
@@ -12,15 +13,15 @@
 - [browser] fix: Call wrapped `RequestAnimationFrame` with correct context (#2570)
 - [node] fix: Prevent reading the same source file multiple times (#2569)
 - [integrations] feat: Vue performance monitoring (#2571)
-- [apm] fix: Use proper type name for op #2584
-- [core] fix: sent_at for envelope headers to use same clock #2597
-- [apm] fix: Improve bundle size by moving span status to @sentry/apm #2589
-- [apm] feat: No longer discard transactions instead mark them deadline exceeded #2588
-- [apm] feat: Introduce `Sentry.startTransaction` and `Transaction.startChild` #2600
-- [apm] feat: Transactions no longer go through `beforeSend` #2600
+- [apm] fix: Use proper type name for op (#2584)
+- [core] fix: sent_at for envelope headers to use same clock (#2597)
+- [apm] fix: Improve bundle size by moving span status to @sentry/apm (#2589)
+- [apm] feat: No longer discard transactions instead mark them deadline exceeded (#2588)
+- [apm] feat: Introduce `Sentry.startTransaction` and `Transaction.startChild` (#2600)
+- [apm] feat: Transactions no longer go through `beforeSend` (#2600)
 - [browser] fix: Emit Sentry Request breadcrumbs from inside the client (#2615)
-- [apm] fix: No longer debounce IdleTransaction #2618
-- [apm] feat: Add pageload transaction option + fixes #2623
+- [apm] fix: No longer debounce IdleTransaction (#2618)
+- [apm] feat: Add pageload transaction option + fixes (#2623)
 
 ## 5.15.5
 

packages/browser/test/package/test-code.js

@@ -50,7 +50,19 @@ Sentry.addBreadcrumb({
 
 // Capture methods
 Sentry.captureException(new Error('foo'));
+Sentry.captureException(new Error('foo'), {
+  tags: {
+    foo: 1,
+  },
+});
+Sentry.captureException(new Error('foo'), scope => scope);
 Sentry.captureMessage('bar');
+Sentry.captureMessage('bar', {
+  tags: {
+    foo: 1,
+  },
+});
+Sentry.captureMessage('bar', scope => scope);
 
 // Scope behavior
 Sentry.withScope(scope => {

packages/core/src/baseclient.ts

@@ -1,5 +1,5 @@
 import { Scope } from '@sentry/hub';
-import { Client, Event, EventHint, Integration, IntegrationClass, Options, SdkInfo, Severity } from '@sentry/types';
+import { Client, Event, EventHint, Integration, IntegrationClass, Options, Severity } from '@sentry/types';
 import {
   Dsn,
   isPrimitive,
@@ -248,54 +248,31 @@ export abstract class BaseClient<B extends Backend, O extends Options> implement
    * @returns A new event with more information.
    */
   protected _prepareEvent(event: Event, scope?: Scope, hint?: EventHint): PromiseLike<Event | null> {
-    const { environment, release, dist, maxValueLength = 250, normalizeDepth = 3 } = this.getOptions();
-
-    const prepared: Event = { ...event };
-
-    if (!prepared.timestamp) {
-      prepared.timestamp = timestampWithMs();
-    }
-
-    if (prepared.environment === undefined && environment !== undefined) {
-      prepared.environment = environment;
-    }
-
-    if (prepared.release === undefined && release !== undefined) {
-      prepared.release = release;
-    }
-
-    if (prepared.dist === undefined && dist !== undefined) {
-      prepared.dist = dist;
-    }
-
-    if (prepared.message) {
-      prepared.message = truncate(prepared.message, maxValueLength);
-    }
-
-    const exception = prepared.exception && prepared.exception.values && prepared.exception.values[0];
-    if (exception && exception.value) {
-      exception.value = truncate(exception.value, maxValueLength);
-    }
+    const { normalizeDepth = 3 } = this.getOptions();
+    const prepared: Event = {
+      ...event,
+      event_id: event.event_id || (hint && hint.event_id ? hint.event_id : uuid4()),
+      timestamp: event.timestamp || timestampWithMs(),
+    };
 
-    const request = prepared.request;
-    if (request && request.url) {
-      request.url = truncate(request.url, maxValueLength);
-    }
+    this._applyClientOptions(prepared);
+    this._applyIntegrationsMetadata(prepared);
 
-    if (prepared.event_id === undefined) {
-      prepared.event_id = hint && hint.event_id ? hint.event_id : uuid4();
+    // If we have scope given to us, use it as the base for further modifications.
+    // This allows us to prevent unnecessary copying of data if `captureContext` is not provided.
+    let finalScope = scope;
+    if (hint && hint.captureContext) {
+      finalScope = Scope.clone(finalScope).update(hint.captureContext);
     }
 
-    this._addIntegrations(prepared.sdk);
-
     // We prepare the result here with a resolved Event.
     let result = SyncPromise.resolve<Event | null>(prepared);
 
     // This should be the last thing called, since we want that
     // {@link Hub.addEventProcessor} gets the finished prepared event.
-    if (scope) {
+    if (finalScope) {
       // In case we have a hub we reassign it.
-      result = scope.applyToEvent(prepared, hint);
+      result = finalScope.applyToEvent(prepared, hint);
     }
 
     return result.then(evt => {
@@ -345,11 +322,48 @@ export abstract class BaseClient<B extends Backend, O extends Options> implement
     };
   }
 
+  /**
+   *  Enhances event using the client configuration.
+   *  It takes care of all "static" values like environment, release and `dist`,
+   *  as well as truncating overly long values.
+   * @param event event instance to be enhanced
+   */
+  protected _applyClientOptions(event: Event): void {
+    const { environment, release, dist, maxValueLength = 250 } = this.getOptions();
+
+    if (event.environment === undefined && environment !== undefined) {
+      event.environment = environment;
+    }
+
+    if (event.release === undefined && release !== undefined) {
+      event.release = release;
+    }
+
+    if (event.dist === undefined && dist !== undefined) {
+      event.dist = dist;
+    }
+
+    if (event.message) {
+      event.message = truncate(event.message, maxValueLength);
+    }
+
+    const exception = event.exception && event.exception.values && event.exception.values[0];
+    if (exception && exception.value) {
+      exception.value = truncate(exception.value, maxValueLength);
+    }
+
+    const request = event.request;
+    if (request && request.url) {
+      request.url = truncate(request.url, maxValueLength);
+    }
+  }
+
   /**
    * This function adds all used integrations to the SDK info in the event.
    * @param sdkInfo The sdkInfo of the event that will be filled with all integrations.
    */
-  protected _addIntegrations(sdkInfo?: SdkInfo): void {
+  protected _applyIntegrationsMetadata(event: Event): void {
+    const sdkInfo = event.sdk;
     const integrationsArray = Object.keys(this._integrations);
     if (sdkInfo && integrationsArray.length > 0) {
       sdkInfo.integrations = integrationsArray;

packages/core/test/lib/base.test.ts

@@ -1,5 +1,5 @@
 import { Hub, Scope } from '@sentry/hub';
-import { Event } from '@sentry/types';
+import { Event, Severity } from '@sentry/types';
 import { SentryError } from '@sentry/utils';
 
 import { TestBackend } from '../mocks/backend';
@@ -163,9 +163,8 @@ describe('BaseClient', () => {
     });
   });
 
-  describe('captures', () => {
+  describe('captureException', () => {
     test('captures and sends exceptions', () => {
-      expect.assertions(1);
       const client = new TestClient({ dsn: PUBLIC_DSN });
       client.captureException(new Error('test exception'));
       expect(TestBackend.instance!.event).toEqual({
@@ -182,19 +181,69 @@ describe('BaseClient', () => {
       });
     });
 
+    test('allows for providing explicit scope', () => {
+      const client = new TestClient({ dsn: PUBLIC_DSN });
+      const scope = new Scope();
+      scope.setExtra('foo', 'wat');
+      client.captureException(
+        new Error('test exception'),
+        {
+          captureContext: {
+            extra: {
+              bar: 'wat',
+            },
+          },
+        },
+        scope,
+      );
+      expect(TestBackend.instance!.event).toEqual(
+        expect.objectContaining({
+          extra: {
+            bar: 'wat',
+            foo: 'wat',
+          },
+        }),
+      );
+    });
+
+    test('allows for clearing data from existing scope if explicit one does so in a callback function', () => {
+      const client = new TestClient({ dsn: PUBLIC_DSN });
+      const scope = new Scope();
+      scope.setExtra('foo', 'wat');
+      client.captureException(
+        new Error('test exception'),
+        {
+          captureContext: s => {
+            s.clear();
+            s.setExtra('bar', 'wat');
+            return s;
+          },
+        },
+        scope,
+      );
+      expect(TestBackend.instance!.event).toEqual(
+        expect.objectContaining({
+          extra: {
+            bar: 'wat',
+          },
+        }),
+      );
+    });
+  });
+
+  describe('captureMessage', () => {
     test('captures and sends messages', () => {
-      expect.assertions(1);
       const client = new TestClient({ dsn: PUBLIC_DSN });
       client.captureMessage('test message');
       expect(TestBackend.instance!.event).toEqual({
         event_id: '42',
+        level: 'info',
         message: 'test message',
         timestamp: 2020,
       });
     });
 
     test('should call eventFromException if input to captureMessage is not a primitive', () => {
-      expect.assertions(2);
       const client = new TestClient({ dsn: PUBLIC_DSN });
       const spy = jest.spyOn(TestBackend.instance!, 'eventFromException');
 
@@ -209,6 +258,33 @@ describe('BaseClient', () => {
       client.captureMessage([] as any);
       expect(spy.mock.calls.length).toEqual(2);
     });
+
+    test('allows for providing explicit scope', () => {
+      const client = new TestClient({ dsn: PUBLIC_DSN });
+      const scope = new Scope();
+      scope.setExtra('foo', 'wat');
+      client.captureMessage(
+        'test message',
+        Severity.Warning,
+        {
+          captureContext: {
+            extra: {
+              bar: 'wat',
+            },
+          },
+        },
+        scope,
+      );
+      expect(TestBackend.instance!.event).toEqual(
+        expect.objectContaining({
+          extra: {
+            bar: 'wat',
+            foo: 'wat',
+          },
+          level: 'warning',
+        }),
+      );
+    });
   });
 
   describe('captureEvent() / prepareEvent()', () => {

packages/core/test/mocks/backend.ts

@@ -1,4 +1,4 @@
-import { Event, Options, Transport } from '@sentry/types';
+import { Event, Options, Severity, Transport } from '@sentry/types';
 import { SyncPromise } from '@sentry/utils';
 
 import { BaseBackend } from '../../src/basebackend';
@@ -50,8 +50,8 @@ export class TestBackend extends BaseBackend<TestOptions> {
     });
   }
 
-  public eventFromMessage(message: string): PromiseLike<Event> {
-    return SyncPromise.resolve({ message });
+  public eventFromMessage(message: string, level: Severity = Severity.Info): PromiseLike<Event> {
+    return SyncPromise.resolve({ message, level });
   }
 
   public sendEvent(event: Event): void {