feat(core): Introduce seperate client options (#4927)

This PR works toward differentiating the options that are passed into `Sentry.init` and the options passed into clients. We want to make this differentiation to minimize internal state in the client, and instead rely on the max number of items being passed in to the client constructor. We do this by explicitly differentiating between the options that are configured in sdk init (`Options`) and the options that are passed into the client constructor (`ClientOptions`).

Author: AbhiPrasad

packages/browser/src/client.ts

@@ -1,17 +1,13 @@
 import { BaseClient, NewTransport, Scope, SDK_VERSION } from '@sentry/core';
-import { Event, EventHint, Options, Severity, SeverityLevel, Transport } from '@sentry/types';
+import { ClientOptions, Event, EventHint, Options, Severity, SeverityLevel, Transport } from '@sentry/types';
 import { getGlobalObject, logger, stackParserFromOptions } from '@sentry/utils';
 
 import { eventFromException, eventFromMessage } from './eventbuilder';
 import { IS_DEBUG_BUILD } from './flags';
 import { injectReportDialog, ReportDialogOptions } from './helpers';
 import { Breadcrumbs } from './integrations';
 
-/**
- * Configuration options for the Sentry Browser SDK.
- * @see BrowserClient for more information.
- */
-export interface BrowserOptions extends Options {
+export interface BaseBrowserOptions {
   /**
    * A pattern for error URLs which should exclusively be sent to Sentry.
    * This is the opposite of {@link Options.denyUrls}.
@@ -27,19 +23,31 @@ export interface BrowserOptions extends Options {
   denyUrls?: Array<string | RegExp>;
 }
 
+/**
+ * Configuration options for the Sentry Browser SDK.
+ * @see @sentry/types Options for more information.
+ */
+export interface BrowserOptions extends Options, BaseBrowserOptions {}
+
+/**
+ * Configuration options for the Sentry Browser SDK Client class
+ * @see BrowserClient for more information.
+ */
+export interface BrowserClientOptions extends ClientOptions, BaseBrowserOptions {}
+
 /**
  * The Sentry Browser SDK Client.
  *
  * @see BrowserOptions for documentation on configuration options.
  * @see SentryClient for usage documentation.
  */
-export class BrowserClient extends BaseClient<BrowserOptions> {
+export class BrowserClient extends BaseClient<BrowserClientOptions> {
   /**
    * Creates a new Browser SDK instance.
    *
    * @param options Configuration options for this SDK.
    */
-  public constructor(options: BrowserOptions = {}, transport: Transport, newTransport?: NewTransport) {
+  public constructor(options: BrowserClientOptions, transport: Transport, newTransport?: NewTransport) {
     options._metadata = options._metadata || {};
     options._metadata.sdk = options._metadata.sdk || {
       name: 'sentry.javascript.browser',
@@ -51,7 +59,6 @@ export class BrowserClient extends BaseClient<BrowserOptions> {
       ],
       version: SDK_VERSION,
     };
-
     super(options, transport, newTransport);
   }
 

packages/browser/src/sdk.ts

@@ -1,12 +1,20 @@
-import { getCurrentHub, initAndBind, Integrations as CoreIntegrations } from '@sentry/core';
+import { getCurrentHub, getIntegrationsToSetup, initAndBind, Integrations as CoreIntegrations } from '@sentry/core';
 import { Hub } from '@sentry/types';
-import { addInstrumentationHandler, getGlobalObject, logger, resolvedSyncPromise } from '@sentry/utils';
+import {
+  addInstrumentationHandler,
+  getGlobalObject,
+  logger,
+  resolvedSyncPromise,
+  stackParserFromOptions,
+  supportsFetch,
+} from '@sentry/utils';
 
-import { BrowserClient, BrowserOptions } from './client';
+import { BrowserClient, BrowserClientOptions, BrowserOptions } from './client';
 import { IS_DEBUG_BUILD } from './flags';
 import { ReportDialogOptions, wrap as internalWrap } from './helpers';
 import { Breadcrumbs, Dedupe, GlobalHandlers, LinkedErrors, TryCatch, UserAgent } from './integrations';
 import { defaultStackParsers } from './stack-parsers';
+import { FetchTransport, XHRTransport } from './transports';
 import { setupBrowserTransport } from './transports/setup';
 
 export const defaultIntegrations = [
@@ -97,9 +105,17 @@ export function init(options: BrowserOptions = {}): void {
   if (options.stackParser === undefined) {
     options.stackParser = defaultStackParsers;
   }
-
   const { transport, newTransport } = setupBrowserTransport(options);
-  initAndBind(BrowserClient, options, transport, newTransport);
+
+  const clientOptions: BrowserClientOptions = {
+    ...options,
+    stackParser: stackParserFromOptions(options),
+    integrations: getIntegrationsToSetup(options),
+    // TODO(v7): get rid of transport being passed down below
+    transport: options.transport || (supportsFetch() ? FetchTransport : XHRTransport),
+  };
+
+  initAndBind(BrowserClient, clientOptions, transport, newTransport);
 
   if (options.autoSessionTracking) {
     startSessionTracking();

packages/browser/src/transports/setup.ts

@@ -31,7 +31,10 @@ export interface BrowserTransportOptions extends BaseTransportOptions {
  * this function will return a ready to use `NewTransport`.
  */
 // TODO(v7): Adjust return value when NewTransport is the default
-export function setupBrowserTransport(options: BrowserOptions): { transport: Transport; newTransport?: NewTransport } {
+export function setupBrowserTransport(options: BrowserOptions): {
+  transport: Transport;
+  newTransport?: NewTransport;
+} {
   if (!options.dsn) {
     // We return the noop transport here in case there is no Dsn.
     return { transport: new NoopTransport() };

packages/browser/test/unit/helper/browser-client-options.ts

@@ -0,0 +1,12 @@
+import { NoopTransport } from '@sentry/core';
+
+import { BrowserClientOptions } from '../../../src/client';
+
+export function getDefaultBrowserClientOptions(options: Partial<BrowserClientOptions> = {}): BrowserClientOptions {
+  return {
+    integrations: [],
+    transport: NoopTransport,
+    stackParser: () => [],
+    ...options,
+  };
+}

packages/browser/test/unit/index.test.ts

@@ -16,6 +16,7 @@ import {
   showReportDialog,
   wrap,
 } from '../../src';
+import { getDefaultBrowserClientOptions } from './helper/browser-client-options';
 import { SimpleTransport } from './mocks/simpletransport';
 
 const dsn = 'https://[email protected]/4291';
@@ -75,7 +76,8 @@ describe('SentryBrowser', () => {
   describe('showReportDialog', () => {
     describe('user', () => {
       const EX_USER = { email: '[email protected]' };
-      const client = new BrowserClient({ dsn }, new SimpleTransport({ dsn }));
+      const options = getDefaultBrowserClientOptions({ dsn });
+      const client = new BrowserClient(options, new SimpleTransport({ dsn }));
       const reportDialogSpy = jest.spyOn(client, 'showReportDialog');
 
       beforeEach(() => {
@@ -139,53 +141,41 @@ describe('SentryBrowser', () => {
     });
 
     it('should capture a message', done => {
-      getCurrentHub().bindClient(
-        new BrowserClient(
-          {
-            beforeSend: (event: Event): Event | null => {
-              expect(event.message).toBe('test');
-              expect(event.exception).toBeUndefined();
-              done();
-              return event;
-            },
-            dsn,
-          },
-          new SimpleTransport({ dsn }),
-        ),
-      );
+      const options = getDefaultBrowserClientOptions({
+        beforeSend: (event: Event): Event | null => {
+          expect(event.message).toBe('test');
+          expect(event.exception).toBeUndefined();
+          done();
+          return event;
+        },
+        dsn,
+      });
+      getCurrentHub().bindClient(new BrowserClient(options, new SimpleTransport({ dsn })));
       captureMessage('test');
     });
 
     it('should capture an event', done => {
-      getCurrentHub().bindClient(
-        new BrowserClient(
-          {
-            beforeSend: (event: Event): Event | null => {
-              expect(event.message).toBe('event');
-              expect(event.exception).toBeUndefined();
-              done();
-              return event;
-            },
-            dsn,
-          },
-          new SimpleTransport({ dsn }),
-        ),
-      );
+      const options = getDefaultBrowserClientOptions({
+        beforeSend: (event: Event): Event | null => {
+          expect(event.message).toBe('event');
+          expect(event.exception).toBeUndefined();
+          done();
+          return event;
+        },
+        dsn,
+      });
+      getCurrentHub().bindClient(new BrowserClient(options, new SimpleTransport({ dsn })));
       captureEvent({ message: 'event' });
     });
 
     it('should not dedupe an event on bound client', async () => {
       const localBeforeSend = jest.fn();
-      getCurrentHub().bindClient(
-        new BrowserClient(
-          {
-            beforeSend: localBeforeSend,
-            dsn,
-            integrations: [],
-          },
-          new SimpleTransport({ dsn }),
-        ),
-      );
+      const options = getDefaultBrowserClientOptions({
+        beforeSend: localBeforeSend,
+        dsn,
+        integrations: [],
+      });
+      getCurrentHub().bindClient(new BrowserClient(options, new SimpleTransport({ dsn })));
 
       captureMessage('event222');
       captureMessage('event222');
@@ -197,16 +187,12 @@ describe('SentryBrowser', () => {
 
     it('should use inboundfilter rules of bound client', async () => {
       const localBeforeSend = jest.fn();
-      getCurrentHub().bindClient(
-        new BrowserClient(
-          {
-            beforeSend: localBeforeSend,
-            dsn,
-            integrations: [new Integrations.InboundFilters({ ignoreErrors: ['capture'] })],
-          },
-          new SimpleTransport({ dsn }),
-        ),
-      );
+      const options = getDefaultBrowserClientOptions({
+        beforeSend: localBeforeSend,
+        dsn,
+        integrations: [new Integrations.InboundFilters({ ignoreErrors: ['capture'] })],
+      });
+      getCurrentHub().bindClient(new BrowserClient(options, new SimpleTransport({ dsn })));
 
       captureMessage('capture');
 
@@ -267,7 +253,8 @@ describe('SentryBrowser initialization', () => {
     });
 
     it('should set SDK data when instantiating a client directly', () => {
-      const client = new BrowserClient({ dsn }, new SimpleTransport({ dsn }));
+      const options = getDefaultBrowserClientOptions({ dsn });
+      const client = new BrowserClient(options, new SimpleTransport({ dsn }));
 
       const sdkData = (client.getTransport() as any)._api.metadata?.sdk;
 
@@ -309,20 +296,16 @@ describe('SentryBrowser initialization', () => {
 
 describe('wrap()', () => {
   it('should wrap and call function while capturing error', done => {
-    getCurrentHub().bindClient(
-      new BrowserClient(
-        {
-          beforeSend: (event: Event): Event | null => {
-            expect(event.exception!.values![0].type).toBe('TypeError');
-            expect(event.exception!.values![0].value).toBe('mkey');
-            done();
-            return null;
-          },
-          dsn,
-        },
-        new SimpleTransport({ dsn }),
-      ),
-    );
+    const options = getDefaultBrowserClientOptions({
+      beforeSend: (event: Event): Event | null => {
+        expect(event.exception!.values![0].type).toBe('TypeError');
+        expect(event.exception!.values![0].value).toBe('mkey');
+        done();
+        return null;
+      },
+      dsn,
+    });
+    getCurrentHub().bindClient(new BrowserClient(options, new SimpleTransport({ dsn })));
 
     try {
       wrap(() => {

packages/browser/test/unit/integrations/linkederrors.test.ts

@@ -5,6 +5,7 @@ import { BrowserClient } from '../../../src/client';
 import * as LinkedErrorsModule from '../../../src/integrations/linkederrors';
 import { defaultStackParsers } from '../../../src/stack-parsers';
 import { setupBrowserTransport } from '../../../src/transports';
+import { getDefaultBrowserClientOptions } from '../helper/browser-client-options';
 
 const parser = createStackParser(...defaultStackParsers);
 
@@ -45,7 +46,7 @@ describe('LinkedErrors', () => {
       one.cause = two;
 
       const originalException = one;
-      const options = { stackParser: parser };
+      const options = getDefaultBrowserClientOptions({ stackParser: parser });
       const client = new BrowserClient(options, setupBrowserTransport(options).transport);
       return client.eventFromException(originalException).then(event => {
         const result = LinkedErrorsModule._handler(parser, 'cause', 5, event, {
@@ -76,7 +77,7 @@ describe('LinkedErrors', () => {
       one.reason = two;
 
       const originalException = one;
-      const options = { stackParser: parser };
+      const options = getDefaultBrowserClientOptions({ stackParser: parser });
       const client = new BrowserClient(options, setupBrowserTransport(options).transport);
       return client.eventFromException(originalException).then(event => {
         const result = LinkedErrorsModule._handler(parser, 'reason', 5, event, {
@@ -103,7 +104,7 @@ describe('LinkedErrors', () => {
       one.cause = two;
       two.cause = three;
 
-      const options = { stackParser: parser };
+      const options = getDefaultBrowserClientOptions({ stackParser: parser });
       const client = new BrowserClient(options, setupBrowserTransport(options).transport);
       const originalException = one;
       return client.eventFromException(originalException).then(event => {