refactor api usage in client

Author: AbhiPrasad

packages/browser/src/transports/new-fetch.ts

@@ -1,6 +1,5 @@
 import { createTransport } from '@sentry/core';
 import { BaseTransportOptions, NewTransport, TransportMakeRequestResponse, TransportRequest } from '@sentry/types';
-import { resolvedSyncPromise } from '@sentry/utils';
 
 import { FetchImpl, getNativeFetchImplementation } from './utils';
 
@@ -16,10 +15,6 @@ export function makeNewFetchTransport(
   nativeFetch: FetchImpl = getNativeFetchImplementation(),
 ): NewTransport {
   function makeRequest(request: TransportRequest): PromiseLike<TransportMakeRequestResponse> {
-    if (!options.url) {
-      return resolvedSyncPromise({ statusCode: 400 });
-    }
-
     const requestOptions: RequestInit = {
       body: request.body,
       method: 'POST',

packages/browser/src/transports/new-xhr.ts

@@ -1,6 +1,6 @@
 import { createTransport } from '@sentry/core';
 import { BaseTransportOptions, NewTransport, TransportMakeRequestResponse, TransportRequest } from '@sentry/types';
-import { resolvedSyncPromise, SyncPromise } from '@sentry/utils';
+import { SyncPromise } from '@sentry/utils';
 
 /**
  * The DONE ready state for XmlHttpRequest
@@ -21,10 +21,6 @@ export interface XHRTransportOptions extends BaseTransportOptions {
  */
 export function makeNewXHRTransport(options: XHRTransportOptions): NewTransport {
   function makeRequest(request: TransportRequest): PromiseLike<TransportMakeRequestResponse> {
-    if (!options.url) {
-      return resolvedSyncPromise({ statusCode: 400 });
-    }
-
     return new SyncPromise<TransportMakeRequestResponse>((resolve, _reject) => {
       const xhr = new XMLHttpRequest();
 

packages/browser/test/unit/sdk.test.ts

@@ -1,4 +1,4 @@
-import { NoopTransport, Scope } from '@sentry/core';
+import { Scope } from '@sentry/core';
 import { MockIntegration } from '@sentry/core/test/lib/sdk.test';
 import { Client, Integration } from '@sentry/types';
 
@@ -12,7 +12,6 @@ const PUBLIC_DSN = 'https://username@domain/123';
 function getDefaultBrowserOptions(options: Partial<BrowserOptions> = {}): BrowserOptions {
   return {
     integrations: [],
-    transport: NoopTransport,
     stackParser: () => [],
     ...options,
   };

packages/core/src/baseclient.ts

@@ -4,6 +4,7 @@ import {
   Client,
   ClientOptions,
   DsnComponents,
+  Envelope,
   Event,
   EventHint,
   Integration,
@@ -30,7 +31,7 @@ import {
   uuid4,
 } from '@sentry/utils';
 
-import { getEnvelopeEndpointWithUrlEncodedAuth, initAPIDetails } from './api';
+import { getEnvelopeEndpointWithUrlEncodedAuth } from './api';
 import { IS_DEBUG_BUILD } from './flags';
 import { IntegrationIndex, setupIntegrations } from './integration';
 import { createEventEnvelope, createSessionEnvelope } from './request';
@@ -75,18 +76,14 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
   /** The client Dsn, if specified in options. Without this Dsn, the SDK will be disabled. */
   protected readonly _dsn?: DsnComponents;
 
+  protected readonly _transport?: NewTransport | undefined;
+
   /** Array of used integrations. */
   protected _integrations: IntegrationIndex = {};
 
   /** Number of calls being processed */
   protected _numProcessing: number = 0;
 
-  /** Cached transport used internally. */
-  protected _transport: NewTransport;
-
-  /** New v7 Transport that is initialized alongside the old one */
-  protected _newTransport?: NewTransport;
-
   /**
    * Initializes this client instance.
    *
@@ -96,18 +93,13 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
    */
   protected constructor(options: O) {
     this._options = options;
-
-    let url;
     if (options.dsn) {
-      this._dsn = makeDsn(options.dsn);
-      // TODO(v7): Figure out what to do with transport when dsn is not defined
-      const api = initAPIDetails(this._dsn, options._metadata, options.tunnel);
-      url = getEnvelopeEndpointWithUrlEncodedAuth(api.dsn, api.tunnel);
+      const dsn = makeDsn(options.dsn);
+      const url = getEnvelopeEndpointWithUrlEncodedAuth(dsn, options.tunnel);
+      this._transport = options.transport({ ...options.transportOptions, url });
     } else {
       IS_DEBUG_BUILD && logger.warn('No DSN provided, client will not do anything.');
     }
-
-    this._transport = options.transport({ ...options.transportOptions, url });
   }
 
   /**
@@ -217,19 +209,22 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
   /**
    * @inheritDoc
    */
-  public getTransport(): Transport {
+  public getTransport(): NewTransport | undefined {
     return this._transport;
   }
 
   /**
    * @inheritDoc
    */
   public flush(timeout?: number): PromiseLike<boolean> {
-    return this._isClientDoneProcessing(timeout).then(clientFinished => {
-      return this.getTransport()
-        .close(timeout)
-        .then(transportFlushed => clientFinished && transportFlushed);
-    });
+    const transport = this._transport;
+    if (transport) {
+      return this._isClientDoneProcessing(timeout).then(clientFinished => {
+        return transport.flush(timeout).then(transportFlushed => clientFinished && transportFlushed);
+      });
+    } else {
+      return resolvedSyncPromise(true);
+    }
   }
 
   /**
@@ -267,50 +262,32 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
    * @inheritDoc
    */
   public sendEvent(event: Event): void {
-    // TODO(v7): Remove the if-else
-    if (
-      this._newTransport &&
-      this._options.dsn &&
-      this._options._experiments &&
-      this._options._experiments.newTransport
-    ) {
-      const api = initAPIDetails(this._options.dsn, this._options._metadata, this._options.tunnel);
-      const env = createEventEnvelope(event, api);
-      void this._newTransport.send(env).then(null, reason => {
-        IS_DEBUG_BUILD && logger.error('Error while sending event:', reason);
-      });
-    } else {
-      void this._transport.sendEvent(event).then(null, reason => {
-        IS_DEBUG_BUILD && logger.error('Error while sending event:', reason);
-      });
+    if (this._dsn) {
+      const env = createEventEnvelope(event, this._dsn, this._options._metadata, this._options.tunnel);
+      this.sendEnvelope(env);
     }
   }
 
   /**
    * @inheritDoc
    */
   public sendSession(session: Session): void {
-    if (!this._transport.sendSession) {
-      IS_DEBUG_BUILD && logger.warn("Dropping session because custom transport doesn't implement sendSession");
-      return;
+    if (this._dsn) {
+      const [env] = createSessionEnvelope(session, this._dsn, this._options._metadata, this._options.tunnel);
+      this.sendEnvelope(env);
     }
+  }
 
-    // TODO(v7): Remove the if-else
-    if (
-      this._newTransport &&
-      this._options.dsn &&
-      this._options._experiments &&
-      this._options._experiments.newTransport
-    ) {
-      const api = initAPIDetails(this._options.dsn, this._options._metadata, this._options.tunnel);
-      const [env] = createSessionEnvelope(session, api);
-      void this._newTransport.send(env).then(null, reason => {
-        IS_DEBUG_BUILD && logger.error('Error while sending session:', reason);
+  /**
+   * @inheritdoc
+   */
+  public sendEnvelope(env: Envelope): void {
+    if (this._transport && this._dsn) {
+      this._transport.send(env).then(null, reason => {
+        IS_DEBUG_BUILD && logger.error('Error while sending event:', reason);
       });
     } else {
-      void this._transport.sendSession(session).then(null, reason => {
-        IS_DEBUG_BUILD && logger.error('Error while sending session:', reason);
-      });
+      IS_DEBUG_BUILD && logger.error('Transport disabled');
     }
   }
 
@@ -590,15 +567,15 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
   protected _processEvent(event: Event, hint?: EventHint, scope?: Scope): PromiseLike<Event> {
     // eslint-disable-next-line @typescript-eslint/unbound-method
     const { beforeSend, sampleRate } = this.getOptions();
-    const transport = this.getTransport();
 
     type RecordLostEvent = NonNullable<Transport['recordLostEvent']>;
     type RecordLostEventParams = Parameters<RecordLostEvent>;
 
-    function recordLostEvent(outcome: RecordLostEventParams[0], category: RecordLostEventParams[1]): void {
-      if (transport.recordLostEvent) {
-        transport.recordLostEvent(outcome, category);
-      }
+    function recordLostEvent(_outcome: RecordLostEventParams[0], _category: RecordLostEventParams[1]): void {
+      // no-op as new transports don't have client outcomes
+      // if (transport.recordLostEvent) {
+      //   transport.recordLostEvent(outcome, category);
+      // }
     }
 
     if (!this._isEnabled()) {

packages/core/src/request.ts

@@ -1,8 +1,10 @@
 import {
+  DsnComponents,
   Event,
   EventEnvelope,
   EventItem,
   SdkInfo,
+  SdkMetadata,
   SentryRequest,
   SentryRequestType,
   Session,
@@ -15,11 +17,11 @@ import { createEnvelope, dsnToString, normalize, serializeEnvelope } from '@sent
 import { APIDetails, getEnvelopeEndpointWithUrlEncodedAuth, getStoreEndpointWithUrlEncodedAuth } from './api';
 
 /** Extract sdk info from from the API metadata */
-function getSdkMetadataForEnvelopeHeader(api: APIDetails): SdkInfo | undefined {
-  if (!api.metadata || !api.metadata.sdk) {
+function getSdkMetadataForEnvelopeHeader(metadata?: SdkMetadata): SdkInfo | undefined {
+  if (!metadata || !metadata.sdk) {
     return;
   }
-  const { name, version } = api.metadata.sdk;
+  const { name, version } = metadata.sdk;
   return { name, version };
 }
 
@@ -42,13 +44,15 @@ function enhanceEventWithSdkInfo(event: Event, sdkInfo?: SdkInfo): Event {
 /** Creates an envelope from a Session */
 export function createSessionEnvelope(
   session: Session | SessionAggregates,
-  api: APIDetails,
+  dsn: DsnComponents,
+  metadata?: SdkMetadata,
+  tunnel?: string,
 ): [SessionEnvelope, SentryRequestType] {
-  const sdkInfo = getSdkMetadataForEnvelopeHeader(api);
+  const sdkInfo = getSdkMetadataForEnvelopeHeader(metadata);
   const envelopeHeaders = {
     sent_at: new Date().toISOString(),
     ...(sdkInfo && { sdk: sdkInfo }),
-    ...(!!api.tunnel && { dsn: dsnToString(api.dsn) }),
+    ...(!!tunnel && { dsn: dsnToString(dsn) }),
   };
 
   // I know this is hacky but we don't want to add `sessions` to request type since it's never rate limited
@@ -63,7 +67,7 @@ export function createSessionEnvelope(
 
 /** Creates a SentryRequest from a Session. */
 export function sessionToSentryRequest(session: Session | SessionAggregates, api: APIDetails): SentryRequest {
-  const [envelope, type] = createSessionEnvelope(session, api);
+  const [envelope, type] = createSessionEnvelope(session, api.dsn, api.metadata, api.tunnel);
   return {
     body: serializeEnvelope(envelope),
     type,
@@ -75,8 +79,13 @@ export function sessionToSentryRequest(session: Session | SessionAggregates, api
  * Create an Envelope from an event. Note that this is duplicated from below,
  * but on purpose as this will be refactored in v7.
  */
-export function createEventEnvelope(event: Event, api: APIDetails): EventEnvelope {
-  const sdkInfo = getSdkMetadataForEnvelopeHeader(api);
+export function createEventEnvelope(
+  event: Event,
+  dsn: DsnComponents,
+  metadata?: SdkMetadata,
+  tunnel?: string,
+): EventEnvelope {
+  const sdkInfo = getSdkMetadataForEnvelopeHeader(metadata);
   const eventType = event.type || 'event';
 
   const { transactionSampling } = event.sdkProcessingMetadata || {};
@@ -96,7 +105,7 @@ export function createEventEnvelope(event: Event, api: APIDetails): EventEnvelop
   // 2. Restore the original version of the request body, which is commented out
   // 3. Search for either of the PR URLs above and pull out the companion hacks in the browser playwright tests and the
   //    baseClient tests in this package
-  enhanceEventWithSdkInfo(event, api.metadata.sdk);
+  enhanceEventWithSdkInfo(event, metadata && metadata.sdk);
   event.tags = event.tags || {};
   event.extra = event.extra || {};
 
@@ -115,7 +124,7 @@ export function createEventEnvelope(event: Event, api: APIDetails): EventEnvelop
     event_id: event.event_id as string,
     sent_at: new Date().toISOString(),
     ...(sdkInfo && { sdk: sdkInfo }),
-    ...(!!api.tunnel && { dsn: dsnToString(api.dsn) }),
+    ...(!!tunnel && { dsn: dsnToString(dsn) }),
   };
   const eventItem: EventItem = [
     {
@@ -129,7 +138,7 @@ export function createEventEnvelope(event: Event, api: APIDetails): EventEnvelop
 
 /** Creates a SentryRequest from an event. */
 export function eventToSentryRequest(event: Event, api: APIDetails): SentryRequest {
-  const sdkInfo = getSdkMetadataForEnvelopeHeader(api);
+  const sdkInfo = getSdkMetadataForEnvelopeHeader(api.metadata);
   const eventType = event.type || 'event';
   const useEnvelope = eventType === 'transaction' || !!api.tunnel;
 

packages/node/src/transports/new.ts

@@ -35,12 +35,6 @@ export interface NodeTransportOptions extends BaseTransportOptions {
  * Creates a Transport that uses native the native 'http' and 'https' modules to send events to Sentry.
  */
 export function makeNodeTransport(options: NodeTransportOptions): NewTransport {
-  if (!options.url) {
-    return {
-      send: () => new Promise(() => undefined),
-      flush: () => new Promise(() => true),
-    };
-  }
   const urlSegments = new URL(options.url);
   const isHttps = urlSegments.protocol === 'https:';
 
@@ -60,8 +54,7 @@ export function makeNodeTransport(options: NodeTransportOptions): NewTransport {
     : new nativeHttpModule.Agent({ keepAlive: false, maxSockets: 30, timeout: 2000 });
 
   const requestExecutor = createRequestExecutor(
-    // have to explicitly cast because we check url above
-    options as NodeTransportOptions & { url: string },
+    options as NodeTransportOptions,
     options.httpModule ?? nativeHttpModule,
     agent,
   );
@@ -97,7 +90,7 @@ function applyNoProxyOption(transportUrlSegments: URL, proxy: string | undefined
  * Creates a RequestExecutor to be used with `createTransport`.
  */
 function createRequestExecutor(
-  options: NodeTransportOptions & { url: string },
+  options: NodeTransportOptions,
   httpModule: HTTPModule,
   agent: http.Agent,
 ): TransportRequestExecutor {

packages/types/src/client.ts

@@ -5,7 +5,7 @@ import { ClientOptions } from './options';
 import { Scope } from './scope';
 import { Session } from './session';
 import { Severity, SeverityLevel } from './severity';
-import { Transport } from './transport';
+import { Transport, NewTransport } from './transport';
 
 /**
  * User-Facing Sentry SDK Client.
@@ -72,7 +72,7 @@ export interface Client<O extends ClientOptions = ClientOptions> {
    *
    * @returns The transport.
    */
-  getTransport?(): Transport;
+  getTransport(): NewTransport | undefined;
 
   /**
    * Flush the event queue and set the client to `enabled = false`. See {@link Client.flush}.

packages/types/src/transport.ts

@@ -46,7 +46,7 @@ export interface BaseTransportOptions extends InternalBaseTransportOptions {
   // transport does not care about dsn specific - client should take care of
   // parsing and figuring that out
   // If no DSN is configured, pass in an undefined url.
-  url?: string;
+  url: string;
 }
 
 export interface NewTransport {