[v7] Unify BrowserClient/NodeClient and extract majority of functionalities to BaseClient

Author: kamilogorek

packages/browser/src/client.ts

@@ -1,12 +1,11 @@
-import { BaseClient, Scope, SDK_VERSION } from '@sentry/core';
+import { BaseClient, SDK_VERSION } from '@sentry/core';
 import { Event, EventHint, Options, Severity } from '@sentry/types';
 import { getGlobalObject, logger, supportsFetch } from '@sentry/utils';
-import { NoopTransport, ReportDialogOptions, Transport, TransportOptions } from '@sentry/transport-base';
+import { ReportDialogOptions } from '@sentry/transport-base';
 import { FetchTransport } from '@sentry/transport-fetch';
 import { XHRTransport } from '@sentry/transport-xhr';
 
 import { injectReportDialog } from './helpers';
-import { Breadcrumbs } from './integrations';
 import { eventFromException, eventFromMessage } from './eventbuilder';
 
 /**
@@ -28,12 +27,6 @@ export interface BrowserOptions extends Options {
    */
   denyUrls?: Array<string | RegExp>;
 
-  /** @deprecated use {@link Options.allowUrls} instead. */
-  whitelistUrls?: Array<string | RegExp>;
-
-  /** @deprecated use {@link Options.denyUrls} instead. */
-  blacklistUrls?: Array<string | RegExp>;
-
   /**
    * A flag enabling Sessions Tracking feature.
    * By default Sessions Tracking is disabled.
@@ -65,23 +58,11 @@ export class BrowserClient extends BaseClient<BrowserOptions> {
       ],
       version: SDK_VERSION,
     };
+    options.transport = options.transport ?? (supportsFetch() ? FetchTransport : XHRTransport);
 
     super(options);
   }
 
-  /**
-   * @inheritDoc
-   */
-  public eventFromException(exception: unknown, hint?: EventHint): PromiseLike<Event> {
-    return eventFromException(this._options, exception, hint);
-  }
-  /**
-   * @inheritDoc
-   */
-  public eventFromMessage(message: string, level: Severity = Severity.Info, hint?: EventHint): PromiseLike<Event> {
-    return eventFromMessage(this._options, message, level, hint);
-  }
-
   /**
    * Show a report dialog to the user to send feedback to a specific event.
    *
@@ -108,41 +89,22 @@ export class BrowserClient extends BaseClient<BrowserOptions> {
   /**
    * @inheritDoc
    */
-  protected _prepareEvent(event: Event, scope?: Scope, hint?: EventHint): PromiseLike<Event | null> {
-    event.platform = event.platform || 'javascript';
-    return super._prepareEvent(event, scope, hint);
+  protected _eventFromException(exception: unknown, hint?: EventHint): PromiseLike<Event> {
+    return eventFromException(this._options, exception, hint);
   }
-
   /**
    * @inheritDoc
    */
-  protected _sendEvent(event: Event): void {
-    const integration = this.getIntegration(Breadcrumbs);
-    if (integration) {
-      integration.addSentryBreadcrumb(event);
-    }
-    super._sendEvent(event);
+  protected _eventFromMessage(message: string, level: Severity = Severity.Info, hint?: EventHint): PromiseLike<Event> {
+    return eventFromMessage(this._options, message, level, hint);
   }
 
-  protected _setupTransport(): Transport {
-    // TODO: This whole function should be unnecessary and moved to client construction
-    if (!this._options.dsn) {
-      // We return the noop transport here in case there is no Dsn.
-      return new NoopTransport();
-    }
-
-    const transportOptions: TransportOptions = {
-      ...this._options.transportOptions,
-      dsn: this._options.transportOptions?.dsn ?? this._options.dsn,
-    };
-
-    if (this._options.transport) {
-      return new this._options.transport(transportOptions);
-    }
-
-    if (supportsFetch()) {
-      return new FetchTransport(transportOptions);
-    }
-    return new XHRTransport(transportOptions);
-  }
+  // TODO: Restore this functionality somewhere else, it definitely shouldn't be here.
+  // protected _sendEvent(event: Event): void {
+  //   const integration = this.getIntegration(Breadcrumbs);
+  //   if (integration) {
+  //     integration.addSentryBreadcrumb(event);
+  //   }
+  //   super._sendEvent(event);
+  // }
 }

packages/browser/src/eventbuilder.ts

@@ -1,4 +1,4 @@
-import { Event, EventHint, Options, Severity } from '@sentry/types';
+import { Event, EventHint, Severity } from '@sentry/types';
 import {
   addExceptionMechanism,
   addExceptionTypeValue,
@@ -11,14 +11,15 @@ import {
   SyncPromise,
 } from '@sentry/utils';
 
+import { BrowserOptions } from './client';
 import { eventFromPlainObject, eventFromStacktrace, prepareFramesForEvent } from './parsers';
 import { computeStackTrace } from './tracekit';
 
 /**
  * Builds and Event from a Exception
  * @hidden
  */
-export function eventFromException(options: Options, exception: unknown, hint?: EventHint): PromiseLike<Event> {
+export function eventFromException(options: BrowserOptions, exception: unknown, hint?: EventHint): PromiseLike<Event> {
   const syntheticException = (hint && hint.syntheticException) || undefined;
   const event = eventFromUnknownInput(exception, syntheticException, {
     attachStacktrace: options.attachStacktrace,
@@ -27,10 +28,11 @@ export function eventFromException(options: Options, exception: unknown, hint?:
     handled: true,
     type: 'generic',
   });
-  event.level = Severity.Error;
   if (hint && hint.event_id) {
     event.event_id = hint.event_id;
   }
+  event.level = Severity.Error;
+  event.platform = 'javascript';
   return SyncPromise.resolve(event);
 }
 
@@ -39,7 +41,7 @@ export function eventFromException(options: Options, exception: unknown, hint?:
  * @hidden
  */
 export function eventFromMessage(
-  options: Options,
+  options: BrowserOptions,
   message: string,
   level: Severity = Severity.Info,
   hint?: EventHint,
@@ -48,10 +50,11 @@ export function eventFromMessage(
   const event = eventFromString(message, syntheticException, {
     attachStacktrace: options.attachStacktrace,
   });
-  event.level = level;
   if (hint && hint.event_id) {
     event.event_id = hint.event_id;
   }
+  event.level = level;
+  event.platform = 'javascript';
   return SyncPromise.resolve(event);
 }
 

packages/browser/src/exports.ts

@@ -39,7 +39,6 @@ export {
 } from '@sentry/core';
 
 export { BrowserClient, BrowserOptions } from './client';
-export { injectReportDialog } from './helpers';
 export { eventFromException, eventFromMessage } from './eventbuilder';
 export { defaultIntegrations, forceLoad, init, lastEventId, onLoad, showReportDialog, flush, close, wrap } from './sdk';
 export { SDK_NAME } from './version';

packages/core/src/baseclient.ts

@@ -95,28 +95,6 @@ export interface ClientLike<O extends Options = Options> {
 
   /** Returns an array of installed integrations on the client. */
   getIntegration<T extends Integration>(integration: IntegrationClass<T>): T | null;
-
-  /** This is an internal function to setup all integrations that should run on the client */
-  setupIntegrations(): void;
-
-  // TODO: Anything below has been moved from backend to make it compile only. Rework.
-
-  /** Creates a {@link Event} from an exception. */
-  // eslint-disable-next-line @typescript-eslint/no-explicit-any
-  eventFromException(exception: any, hint?: EventHint): PromiseLike<Event>;
-
-  /** Creates a {@link Event} from a plain message. */
-  eventFromMessage(message: string, level?: Severity, hint?: EventHint): PromiseLike<Event>;
-
-  sendRequest<T>(request: TransportRequest<T>): void;
-
-  /**
-   * Returns the transport that is used by the backend.
-   * Please note that the transport gets lazy initialized so it will only be there once the first event has been sent.
-   *
-   * @returns The transport.
-   */
-  getTransport(): Transport;
 }
 
 /**
@@ -181,31 +159,6 @@ export abstract class BaseClient<O extends Options> implements ClientLike<O> {
     this._transport = this._setupTransport();
   }
 
-  public eventFromException(_exception: any, _hint?: EventHint): PromiseLike<Event> {
-    return Promise.resolve({});
-  }
-
-  public eventFromMessage(_message: string, _level: Severity = Severity.Info, _hint?: EventHint): PromiseLike<Event> {
-    return Promise.resolve({});
-  }
-
-  /**
-   * @inheritDoc
-   */
-  // TODO: Do we need generic here?
-  public sendRequest<T>(request: TransportRequest<T>): void {
-    this._transport.sendRequest(request).then(null, reason => {
-      logger.error(`Failed sending request: ${reason}`);
-    });
-  }
-
-  /**
-   * @inheritDoc
-   */
-  public getTransport(): Transport {
-    return this._transport;
-  }
-
   /**
    * @inheritDoc
    */
@@ -214,7 +167,7 @@ export abstract class BaseClient<O extends Options> implements ClientLike<O> {
     let eventId: string | undefined = hint && hint.event_id;
 
     this._process(
-      this.eventFromException(exception, hint)
+      this._eventFromException(exception, hint)
         .then(event => this._captureEvent(event, hint, scope))
         .then(result => {
           eventId = result;
@@ -227,12 +180,17 @@ export abstract class BaseClient<O extends Options> implements ClientLike<O> {
   /**
    * @inheritDoc
    */
-  public captureMessage(message: string, level?: Severity, hint?: EventHint, scope?: Scope): string | undefined {
+  public captureMessage(
+    message: string,
+    level: Severity = Severity.Info,
+    hint?: EventHint,
+    scope?: Scope,
+  ): string | undefined {
     let eventId: string | undefined = hint && hint.event_id;
 
     const promisedEvent = isPrimitive(message)
-      ? this.eventFromMessage(String(message), level, hint)
-      : this.eventFromException(message, hint);
+      ? this._eventFromMessage(String(message), level, hint)
+      : this._eventFromException(message, hint);
 
     this._process(
       promisedEvent
@@ -292,9 +250,7 @@ export abstract class BaseClient<O extends Options> implements ClientLike<O> {
    */
   public flush(timeout?: number): PromiseLike<boolean> {
     return this._isClientProcessing(timeout).then(ready => {
-      return this.getTransport()
-        .flush(timeout ?? 0)
-        .then(transportFlushed => ready && transportFlushed);
+      return this._transport.flush(timeout ?? 0).then(transportFlushed => ready && transportFlushed);
     });
   }
 
@@ -308,15 +264,6 @@ export abstract class BaseClient<O extends Options> implements ClientLike<O> {
     });
   }
 
-  /**
-   * Sets up the integrations
-   */
-  public setupIntegrations(): void {
-    if (this._isEnabled()) {
-      this._integrations = setupIntegrations(this._options);
-    }
-  }
-
   /**
    * @inheritDoc
    */
@@ -329,9 +276,38 @@ export abstract class BaseClient<O extends Options> implements ClientLike<O> {
     }
   }
 
+  /**
+   * Sets up the integrations
+   */
+  protected _setupIntegrations(): void {
+    if (this._isEnabled()) {
+      this._integrations = setupIntegrations(this._options);
+    }
+  }
+
   protected _setupTransport(): Transport {
-    // We return the noop transport here in case there is no Dsn.
-    return new NoopTransport();
+    // TODO: This whole function should be unnecessary and moved to client construction
+    if (!this._options.dsn || !this._options.transport) {
+      return new NoopTransport();
+    }
+
+    return new this._options.transport({
+      dsn: this._options.dsn,
+      ...this._options.transportOptions,
+      // TODO: Deprecate these options and move to `transportOptions`
+      // ...(this._options.httpProxy && { httpProxy: this._options.httpProxy }),
+      // ...(this._options.httpsProxy && { httpsProxy: this._options.httpsProxy }),
+      // ...(this._options.caCerts && { caCerts: this._options.caCerts }),
+    });
+  }
+  /**
+   * @inheritDoc
+   */
+  // TODO: Do we need generic here?
+  protected _sendRequest<T>(request: TransportRequest<T>): void {
+    this._transport.sendRequest(request).then(null, reason => {
+      logger.error(`Failed sending request: ${reason}`);
+    });
   }
 
   /** Updates existing session based on the provided event */
@@ -375,7 +351,7 @@ export abstract class BaseClient<O extends Options> implements ClientLike<O> {
 
   /** Deliver captured session to Sentry */
   protected _sendSession(session: Session): void {
-    this.sendRequest(sessionToTransportRequest(session));
+    this._sendRequest(sessionToTransportRequest(session));
   }
 
   /** Waits for the client to be done with processing. */
@@ -557,7 +533,7 @@ export abstract class BaseClient<O extends Options> implements ClientLike<O> {
    * @param event The Sentry event to send
    */
   protected _sendEvent(event: Event): void {
-    this.sendRequest(eventToTransportRequest(event));
+    this._sendRequest(eventToTransportRequest(event));
   }
 
   /**

packages/core/src/integrations/inboundfilters.ts

@@ -11,11 +11,6 @@ interface InboundFiltersOptions {
   denyUrls: Array<string | RegExp>;
   ignoreErrors: Array<string | RegExp>;
   ignoreInternal: boolean;
-
-  /** @deprecated use {@link InboundFiltersOptions.allowUrls} instead. */
-  whitelistUrls: Array<string | RegExp>;
-  /** @deprecated use {@link InboundFiltersOptions.denyUrls} instead. */
-  blacklistUrls: Array<string | RegExp>;
 }
 
 /** Inbound filters configurable by the user */
@@ -134,22 +129,8 @@ export class InboundFilters implements Integration {
 
   private _mergeOptions(clientOptions: Partial<InboundFiltersOptions> = {}): Partial<InboundFiltersOptions> {
     return {
-      allowUrls: [
-        // eslint-disable-next-line deprecation/deprecation
-        ...(this._options.whitelistUrls || []),
-        ...(this._options.allowUrls || []),
-        // eslint-disable-next-line deprecation/deprecation
-        ...(clientOptions.whitelistUrls || []),
-        ...(clientOptions.allowUrls || []),
-      ],
-      denyUrls: [
-        // eslint-disable-next-line deprecation/deprecation
-        ...(this._options.blacklistUrls || []),
-        ...(this._options.denyUrls || []),
-        // eslint-disable-next-line deprecation/deprecation
-        ...(clientOptions.blacklistUrls || []),
-        ...(clientOptions.denyUrls || []),
-      ],
+      allowUrls: [...(this._options.allowUrls || []), ...(clientOptions.allowUrls || [])],
+      denyUrls: [...(this._options.denyUrls || []), ...(clientOptions.denyUrls || [])],
       ignoreErrors: [
         ...(this._options.ignoreErrors || []),
         ...(clientOptions.ignoreErrors || []),

packages/hub/src/hub.ts

@@ -85,7 +85,9 @@ export class Hub implements HubInterface {
   public bindClient(client?: Client): void {
     const top = this.getStackTop();
     top.client = client;
+    // @ts-ignore TODO: integrations wont be instantiated by the hub anymore, left here so we compile
     if (client && client.setupIntegrations) {
+      // @ts-ignore TODO: integrations wont be instantiated by the hub anymore, left here so we compile
       client.setupIntegrations();
     }
   }