the rest of the owl

AbhiPrasad · AbhiPrasad · commit f282c6c12bf9 · 2022-04-18T22:40:57.000-04:00
diff --git a/MIGRATION.md b/MIGRATION.md
@@ -115,12 +115,6 @@ For our efforts to reduce bundle size of the SDK we had to remove and refactor p
   [#4919](https://github.com/getsentry/sentry-javascript/pull/4919)). `Backend` was an unnecessary abstraction which is
   not present in other Sentry SDKs. For the sake of reducing complexity, increasing consistency with other Sentry SDKs and
   decreasing bundle-size, `Backend` was removed.
-  <!-- TODO(v7): Add more info and PR link for passing transports in options once this is done -->
-  <!-- TODO(v7): This needs refinement once NewTransport is the default (maybe this should get its own section with an expamp) -->
-- Inject transport into client instead of initializing it in the client in `setupTransport` (see
-  [#4921](https://github.com/getsentry/sentry-javascript/pull/4921/)). If you are creating your own `Client` or
-  calling `initAndBind`, you will have to supply your desired transport. Either provide a custom one or call
-  `setupBrowserTransport` or `setupNodeTransport` for default transports, depending on your requirements.
 - Remove support for Opera browser pre v15
 
 # Upgrading from 6.17.x to 6.18.0
diff --git a/packages/browser/src/client.ts b/packages/browser/src/client.ts
@@ -1,5 +1,5 @@
-import { BaseClient, NewTransport, Scope, SDK_VERSION } from '@sentry/core';
-import { ClientOptions, Event, EventHint, Options, Severity, SeverityLevel, Transport } from '@sentry/types';
+import { BaseClient, Scope, SDK_VERSION } from '@sentry/core';
+import { ClientOptions, Event, EventHint, Options, Severity, SeverityLevel } from '@sentry/types';
 import { getGlobalObject, logger, stackParserFromOptions } from '@sentry/utils';
 
 import { eventFromException, eventFromMessage } from './eventbuilder';
@@ -47,7 +47,7 @@ export class BrowserClient extends BaseClient<BrowserClientOptions> {
    *
    * @param options Configuration options for this SDK.
    */
-  public constructor(options: BrowserClientOptions, transport: Transport, newTransport?: NewTransport) {
+  public constructor(options: BrowserClientOptions) {
     options._metadata = options._metadata || {};
     options._metadata.sdk = options._metadata.sdk || {
       name: 'sentry.javascript.browser',
@@ -59,7 +59,7 @@ export class BrowserClient extends BaseClient<BrowserClientOptions> {
       ],
       version: SDK_VERSION,
     };
-    super(options, transport, newTransport);
+    super(options);
   }
 
   /**
diff --git a/packages/browser/src/sdk.ts b/packages/browser/src/sdk.ts
@@ -14,8 +14,7 @@ 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';
+import { makeNewFetchTransport, makeNewXHRTransport } from './transports';
 
 export const defaultIntegrations = [
   new CoreIntegrations.InboundFilters(),
@@ -105,17 +104,15 @@ export function init(options: BrowserOptions = {}): void {
   if (options.stackParser === undefined) {
     options.stackParser = defaultStackParsers;
   }
-  const { transport, newTransport } = setupBrowserTransport(options);
 
   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),
+    transport: options.transport || (supportsFetch() ? makeNewFetchTransport : makeNewXHRTransport),
   };
 
-  initAndBind(BrowserClient, clientOptions, transport, newTransport);
+  initAndBind(BrowserClient, clientOptions);
 
   if (options.autoSessionTracking) {
     startSessionTracking();
diff --git a/packages/browser/src/transports/index.ts b/packages/browser/src/transports/index.ts
@@ -4,5 +4,3 @@ export { XHRTransport } from './xhr';
 
 export { makeNewFetchTransport } from './new-fetch';
 export { makeNewXHRTransport } from './new-xhr';
-
-export { setupBrowserTransport } from './setup';
diff --git a/packages/browser/src/transports/new-fetch.ts b/packages/browser/src/transports/new-fetch.ts
@@ -1,10 +1,6 @@
-import {
-  BaseTransportOptions,
-  createTransport,
-  NewTransport,
-  TransportMakeRequestResponse,
-  TransportRequest,
-} from '@sentry/core';
+import { createTransport } from '@sentry/core';
+import { BaseTransportOptions, NewTransport, TransportMakeRequestResponse, TransportRequest } from '@sentry/types';
+import { resolvedSyncPromise } from '@sentry/utils';
 
 import { FetchImpl, getNativeFetchImplementation } from './utils';
 
@@ -20,6 +16,10 @@ 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',
diff --git a/packages/browser/src/transports/new-xhr.ts b/packages/browser/src/transports/new-xhr.ts
@@ -1,11 +1,6 @@
-import {
-  BaseTransportOptions,
-  createTransport,
-  NewTransport,
-  TransportMakeRequestResponse,
-  TransportRequest,
-} from '@sentry/core';
-import { SyncPromise } from '@sentry/utils';
+import { createTransport } from '@sentry/core';
+import { BaseTransportOptions, NewTransport, TransportMakeRequestResponse, TransportRequest } from '@sentry/types';
+import { resolvedSyncPromise, SyncPromise } from '@sentry/utils';
 
 /**
  * The DONE ready state for XmlHttpRequest
@@ -26,6 +21,10 @@ 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();
 
diff --git a/packages/browser/src/transports/setup.ts b/packages/browser/src/transports/setup.ts
diff --git a/packages/core/src/baseclient.ts b/packages/core/src/baseclient.ts
@@ -11,6 +11,7 @@ import {
   Severity,
   SeverityLevel,
   Transport,
+  NewTransport,
 } from '@sentry/types';
 import {
   checkOrSetAlreadyCaught,
@@ -29,7 +30,7 @@ import {
   uuid4,
 } from '@sentry/utils';
 
-import { APIDetails, initAPIDetails } from './api';
+import { APIDetails, initAPIDetails, getEnvelopeEndpointWithUrlEncodedAuth } from './api';
 import { IS_DEBUG_BUILD } from './flags';
 import { IntegrationIndex, setupIntegrations } from './integration';
 import { createEventEnvelope, createSessionEnvelope } from './request';
@@ -82,7 +83,7 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
   protected _numProcessing: number = 0;
 
   /** Cached transport used internally. */
-  protected _transport: Transport;
+  protected _transport: NewTransport;
 
   /** New v7 Transport that is initialized alongside the old one */
   protected _newTransport?: NewTransport;
@@ -94,25 +95,20 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
    * @param transport The (old) Transport instance for the client to use (TODO(v7): remove)
    * @param newTransport The NewTransport instance for the client to use
    */
-  protected constructor(options: O, transport: Transport, newTransport?: NewTransport) {
+  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);
     } else {
       IS_DEBUG_BUILD && logger.warn('No DSN provided, client will not do anything.');
     }
 
-    // TODO(v7): remove old transport
-    this._transport = transport;
-    this._newTransport = newTransport;
-
-    // TODO(v7): refactor this to keep metadata/api outside of transport. This hack is used to
-    //           satisfy tests until we move to NewTransport where we have to revisit this.
-    (this._transport as unknown as { _api: Partial<APIDetails> })._api = {
-      ...((this._transport as unknown as { _api: Partial<APIDetails> })._api || {}),
-      metadata: options._metadata || {},
-    };
+    this._transport = options.transport({ ...options.transportOptions, url: url || '' });
   }
 
   /**
diff --git a/packages/core/src/index.ts b/packages/core/src/index.ts
@@ -1,12 +1,5 @@
 export type { APIDetails } from './api';
 export type { ClientClass } from './sdk';
-export type {
-  BaseTransportOptions,
-  NewTransport,
-  TransportMakeRequestResponse,
-  TransportRequest,
-  TransportRequestExecutor,
-} from './transports/base';
 
 export {
   addBreadcrumb,
diff --git a/packages/core/src/sdk.ts b/packages/core/src/sdk.ts
@@ -1,16 +1,11 @@
 import { getCurrentHub } from '@sentry/hub';
-import { Client, ClientOptions, Transport } from '@sentry/types';
+import { Client, ClientOptions, NewTransport, Transport } from '@sentry/types';
 import { logger } from '@sentry/utils';
 
 import { IS_DEBUG_BUILD } from './flags';
-import { NewTransport } from './transports/base';
 
 /** A class object that can instantiate Client objects. */
-export type ClientClass<F extends Client, O extends ClientOptions> = new (
-  options: O,
-  transport: Transport,
-  newTransport?: NewTransport,
-) => F;
+export type ClientClass<F extends Client, O extends ClientOptions> = new (options: O) => F;
 
 /**
  * Internal function to create a new SDK client instance. The client is
@@ -22,8 +17,6 @@ export type ClientClass<F extends Client, O extends ClientOptions> = new (
 export function initAndBind<F extends Client, O extends ClientOptions>(
   clientClass: ClientClass<F, O>,
   options: O,
-  transport: Transport,
-  newTransport?: NewTransport,
 ): void {
   if (options.debug === true) {
     if (IS_DEBUG_BUILD) {
@@ -40,6 +33,6 @@ export function initAndBind<F extends Client, O extends ClientOptions>(
     scope.update(options.initialScope);
   }
 
-  const client = new clientClass(options, transport, newTransport);
+  const client = new clientClass(options);
   hub.bindClient(client);
 }
diff --git a/packages/node/src/client.ts b/packages/node/src/client.ts
@@ -1,6 +1,6 @@
-import { BaseClient, NewTransport, Scope, SDK_VERSION } from '@sentry/core';
+import { BaseClient, Scope, SDK_VERSION } from '@sentry/core';
 import { SessionFlusher } from '@sentry/hub';
-import { Event, EventHint, Severity, SeverityLevel, Transport } from '@sentry/types';
+import { Event, EventHint, Severity, SeverityLevel } from '@sentry/types';
 import { logger, resolvedSyncPromise, stackParserFromOptions } from '@sentry/utils';
 
 import { eventFromMessage, eventFromUnknownInput } from './eventbuilder';
@@ -20,7 +20,7 @@ export class NodeClient extends BaseClient<NodeClientOptions> {
    * Creates a new Node SDK instance.
    * @param options Configuration options for this SDK.
    */
-  public constructor(options: NodeClientOptions, transport: Transport, newTransport?: NewTransport) {
+  public constructor(options: NodeClientOptions) {
     options._metadata = options._metadata || {};
     options._metadata.sdk = options._metadata.sdk || {
       name: 'sentry.javascript.node',
@@ -33,7 +33,7 @@ export class NodeClient extends BaseClient<NodeClientOptions> {
       version: SDK_VERSION,
     };
 
-    super(options, transport, newTransport);
+    super(options);
   }
 
   /**
diff --git a/packages/node/src/sdk.ts b/packages/node/src/sdk.ts
@@ -8,7 +8,7 @@ import { NodeClient } from './client';
 import { IS_DEBUG_BUILD } from './flags';
 import { Console, ContextLines, Http, LinkedErrors, OnUncaughtException, OnUnhandledRejection } from './integrations';
 import { nodeStackParser } from './stack-parser';
-import { HTTPSTransport, HTTPTransport, setupNodeTransport } from './transports';
+import { makeNodeTransport } from './transports';
 import { NodeClientOptions, NodeOptions } from './types';
 
 export const defaultIntegrations = [
@@ -131,18 +131,15 @@ export function init(options: NodeOptions = {}): void {
     setHubOnCarrier(carrier, getCurrentHub());
   }
 
-  const { transport, newTransport } = setupNodeTransport(options);
-
   // TODO(v7): Refactor this to reduce the logic above
   const clientOptions: NodeClientOptions = {
     ...options,
     stackParser: stackParserFromOptions(options),
     integrations: getIntegrationsToSetup(options),
-    // TODO(v7): Fix me when we switch to new transports entirely.
-    transport: options.transport || (transport instanceof HTTPTransport ? HTTPTransport : HTTPSTransport),
+    transport: options.transport || makeNodeTransport,
   };
 
-  initAndBind(NodeClient, clientOptions, transport, newTransport);
+  initAndBind(NodeClient, clientOptions);
 
   if (options.autoSessionTracking) {
     startSessionTracking();
diff --git a/packages/node/src/transports/index.ts b/packages/node/src/transports/index.ts
@@ -4,4 +4,3 @@ export { BaseTransport } from './base';
 export { HTTPTransport } from './http';
 export { HTTPSTransport } from './https';
 export { makeNodeTransport } from './new';
-export { setupNodeTransport } from './setup';
diff --git a/packages/node/src/transports/new.ts b/packages/node/src/transports/new.ts
diff --git a/packages/node/src/transports/setup.ts b/packages/node/src/transports/setup.ts
diff --git a/packages/types/src/options.ts b/packages/types/src/options.ts
diff --git a/packages/types/src/transport.ts b/packages/types/src/transport.ts
