feat(various): Apply debug guard to logger everywhere (#4698)

This applies the `isDebugBuild()` guard (which allows logging code to be stripped when creating minified bundles) to a number of aspects of the logger:

- All calls to the logger now have the guard. While it's true that not all log statements appear in spots which currently have any chance of making it into a bundle, applying the guard universally reduces cognitive load for both folks reading and writing our code, and future-proofs (or, perhaps more accurately, new-bundling-situation-proofs) all of our logging.

- The `Sentry.init()` option `debug` now only enables the logger if we're in a debug build. This allows us to remove any reference to the logger in non-debug builds, making it able to be dropped from the code. A warning message has also been added to non-debug builds, because someone using `debug` is presumably doing so because they actually do want logging.

- The `logger` singleton is only attached to `global.__SENTRY__` if we're in a debug build. This makes `logger.ts` side-effect-free in non-debug builds, allowing it to be treeshaken freely.

Note that because the return value of `isDebugBuild()` is always `true` (absent intervention by a bundler), it's safe to use everywhere without changing any current behavior.

Finally, all preexisting `if`-block versions of the check are converted to short-circuit versions, for consistency and concision.

Author: lobsterkatie

packages/angular/src/tracing.ts

@@ -2,7 +2,7 @@ import { AfterViewInit, Directive, Injectable, Input, NgModule, OnDestroy, OnIni
 import { Event, NavigationEnd, NavigationStart, Router } from '@angular/router';
 import { getCurrentHub } from '@sentry/browser';
 import { Span, Transaction, TransactionContext } from '@sentry/types';
-import { getGlobalObject, logger, stripUrlQueryAndFragment, timestampWithMs } from '@sentry/utils';
+import { getGlobalObject, isDebugBuild, logger, stripUrlQueryAndFragment, timestampWithMs } from '@sentry/utils';
 import { Observable, Subscription } from 'rxjs';
 import { filter, tap } from 'rxjs/operators';
 
@@ -63,7 +63,8 @@ export class TraceService implements OnDestroy {
     filter(event => event instanceof NavigationStart),
     tap(event => {
       if (!instrumentationInitialized) {
-        logger.error('Angular integration has tracing enabled, but Tracing integration is not configured');
+        isDebugBuild() &&
+          logger.error('Angular integration has tracing enabled, but Tracing integration is not configured');
         return;
       }
 

packages/browser/src/client.ts

@@ -1,6 +1,6 @@
 import { BaseClient, Scope, SDK_VERSION } from '@sentry/core';
 import { Event, EventHint } from '@sentry/types';
-import { getGlobalObject, logger } from '@sentry/utils';
+import { getGlobalObject, isDebugBuild, logger } from '@sentry/utils';
 
 import { BrowserBackend, BrowserOptions } from './backend';
 import { injectReportDialog, ReportDialogOptions } from './helpers';
@@ -47,7 +47,7 @@ export class BrowserClient extends BaseClient<BrowserBackend, BrowserOptions> {
     }
 
     if (!this._isEnabled()) {
-      logger.error('Trying to call showReportDialog with Sentry Client disabled');
+      isDebugBuild() && logger.error('Trying to call showReportDialog with Sentry Client disabled');
       return;
     }
 

packages/browser/src/helpers.ts

@@ -192,16 +192,12 @@ export function injectReportDialog(options: ReportDialogOptions = {}): void {
   }
 
   if (!options.eventId) {
-    if (isDebugBuild()) {
-      logger.error('Missing eventId option in showReportDialog call');
-    }
+    isDebugBuild() && logger.error('Missing eventId option in showReportDialog call');
     return;
   }
 
   if (!options.dsn) {
-    if (isDebugBuild()) {
-      logger.error('Missing dsn option in showReportDialog call');
-    }
+    isDebugBuild() && logger.error('Missing dsn option in showReportDialog call');
     return;
   }
 

packages/browser/src/integrations/dedupe.ts

@@ -1,5 +1,5 @@
 import { Event, EventProcessor, Exception, Hub, Integration, StackFrame } from '@sentry/types';
-import { logger } from '@sentry/utils';
+import { isDebugBuild, logger } from '@sentry/utils';
 
 /** Deduplication filter */
 export class Dedupe implements Integration {
@@ -28,7 +28,7 @@ export class Dedupe implements Integration {
         // Juuust in case something goes wrong
         try {
           if (_shouldDropEvent(currentEvent, self._previousEvent)) {
-            logger.warn('Event dropped due to being a duplicate of previously captured event.');
+            isDebugBuild() && logger.warn('Event dropped due to being a duplicate of previously captured event.');
             return null;
           }
         } catch (_oO) {

packages/browser/src/integrations/globalhandlers.ts

@@ -237,9 +237,7 @@ function _enhanceEventWithInitialFrame(event: Event, url: any, line: any, column
 }
 
 function globalHandlerLog(type: string): void {
-  if (isDebugBuild()) {
-    logger.log(`Global Handler attached: ${type}`);
-  }
+  isDebugBuild() && logger.log(`Global Handler attached: ${type}`);
 }
 
 function addMechanismAndCapture(hub: Hub, error: EventHint['originalException'], event: Event, type: string): void {

packages/browser/src/sdk.ts

@@ -162,9 +162,7 @@ export function flush(timeout?: number): PromiseLike<boolean> {
   if (client) {
     return client.flush(timeout);
   }
-  if (isDebugBuild()) {
-    logger.warn('Cannot flush events. No client defined.');
-  }
+  isDebugBuild() && logger.warn('Cannot flush events. No client defined.');
   return resolvedSyncPromise(false);
 }
 
@@ -181,9 +179,7 @@ export function close(timeout?: number): PromiseLike<boolean> {
   if (client) {
     return client.close(timeout);
   }
-  if (isDebugBuild()) {
-    logger.warn('Cannot flush events and disable SDK. No client defined.');
-  }
+  isDebugBuild() && logger.warn('Cannot flush events and disable SDK. No client defined.');
   return resolvedSyncPromise(false);
 }
 
@@ -212,9 +208,7 @@ function startSessionTracking(): void {
   const document = window.document;
 
   if (typeof document === 'undefined') {
-    if (isDebugBuild()) {
-      logger.warn('Session tracking in non-browser environment with @sentry/browser is not supported.');
-    }
+    isDebugBuild() && logger.warn('Session tracking in non-browser environment with @sentry/browser is not supported.');
     return;
   }
 

packages/browser/src/transports/base.ts

@@ -105,7 +105,7 @@ export abstract class BaseTransport implements Transport {
     // A correct type for map-based implementation if we want to go that route
     // would be `Partial<Record<SentryRequestType, Partial<Record<Outcome, number>>>>`
     const key = `${requestTypeToCategory(category)}:${reason}`;
-    logger.log(`Adding outcome: ${key}`);
+    isDebugBuild() && logger.log(`Adding outcome: ${key}`);
     this._outcomes[key] = (this._outcomes[key] ?? 0) + 1;
   }
 
@@ -122,11 +122,11 @@ export abstract class BaseTransport implements Transport {
 
     // Nothing to send
     if (!Object.keys(outcomes).length) {
-      logger.log('No outcomes to flush');
+      isDebugBuild() && logger.log('No outcomes to flush');
       return;
     }
 
-    logger.log(`Flushing outcomes:\n${JSON.stringify(outcomes, null, 2)}`);
+    isDebugBuild() && logger.log(`Flushing outcomes:\n${JSON.stringify(outcomes, null, 2)}`);
 
     const url = getEnvelopeEndpointWithUrlEncodedAuth(this._api.dsn, this._api.tunnel);
 
@@ -144,7 +144,7 @@ export abstract class BaseTransport implements Transport {
     try {
       sendReport(url, serializeEnvelope(envelope));
     } catch (e) {
-      logger.error(e);
+      isDebugBuild() && logger.error(e);
     }
   }
 
@@ -170,8 +170,9 @@ export abstract class BaseTransport implements Transport {
      * https://developer.mozilla.org/en-US/docs/Web/API/Headers/get
      */
     const limited = this._handleRateLimit(headers);
-    if (limited && isDebugBuild()) {
-      logger.warn(`Too many ${requestType} requests, backing off until: ${this._disabledUntil(requestType)}`);
+    if (limited) {
+      isDebugBuild() &&
+        logger.warn(`Too many ${requestType} requests, backing off until: ${this._disabledUntil(requestType)}`);
     }
 
     if (status === 'success') {

packages/browser/src/transports/utils.ts

@@ -69,9 +69,8 @@ export function getNativeFetchImplementation(): FetchImpl {
       }
       document.head.removeChild(sandbox);
     } catch (e) {
-      if (isDebugBuild()) {
+      isDebugBuild() &&
         logger.warn('Could not create sandbox iframe for pure fetch check, bailing to window.fetch: ', e);
-      }
     }
   }
 

packages/core/src/basebackend.ts

@@ -67,7 +67,7 @@ export abstract class BaseBackend<O extends Options> implements Backend {
   public constructor(options: O) {
     this._options = options;
     if (!this._options.dsn) {
-      logger.warn('No DSN provided, backend will not do anything.');
+      isDebugBuild() && logger.warn('No DSN provided, backend will not do anything.');
     }
     this._transport = this._setupTransport();
   }
@@ -92,9 +92,7 @@ export abstract class BaseBackend<O extends Options> implements Backend {
    */
   public sendEvent(event: Event): void {
     void this._transport.sendEvent(event).then(null, reason => {
-      if (isDebugBuild()) {
-        logger.error('Error while sending event:', reason);
-      }
+      isDebugBuild() && logger.error('Error while sending event:', reason);
     });
   }
 
@@ -103,16 +101,12 @@ export abstract class BaseBackend<O extends Options> implements Backend {
    */
   public sendSession(session: Session): void {
     if (!this._transport.sendSession) {
-      if (isDebugBuild()) {
-        logger.warn("Dropping session because custom transport doesn't implement sendSession");
-      }
+      isDebugBuild() && logger.warn("Dropping session because custom transport doesn't implement sendSession");
       return;
     }
 
     void this._transport.sendSession(session).then(null, reason => {
-      if (isDebugBuild()) {
-        logger.error('Error while sending session:', reason);
-      }
+      isDebugBuild() && logger.error('Error while sending session:', reason);
     });
   }
 

packages/core/src/baseclient.ts

@@ -108,7 +108,7 @@ export abstract class BaseClient<B extends Backend, O extends Options> implement
   public captureException(exception: any, hint?: EventHint, scope?: Scope): string | undefined {
     // ensure we haven't captured this very object before
     if (checkOrSetAlreadyCaught(exception)) {
-      logger.log(ALREADY_SEEN_ERROR);
+      isDebugBuild() && logger.log(ALREADY_SEEN_ERROR);
       return;
     }
 
@@ -153,7 +153,7 @@ export abstract class BaseClient<B extends Backend, O extends Options> implement
   public captureEvent(event: Event, hint?: EventHint, scope?: Scope): string | undefined {
     // ensure we haven't captured this very object before
     if (hint && hint.originalException && checkOrSetAlreadyCaught(hint.originalException)) {
-      logger.log(ALREADY_SEEN_ERROR);
+      isDebugBuild() && logger.log(ALREADY_SEEN_ERROR);
       return;
     }
 
@@ -173,16 +173,12 @@ export abstract class BaseClient<B extends Backend, O extends Options> implement
    */
   public captureSession(session: Session): void {
     if (!this._isEnabled()) {
-      if (isDebugBuild()) {
-        logger.warn('SDK not enabled, will not capture session.');
-      }
+      isDebugBuild() && logger.warn('SDK not enabled, will not capture session.');
       return;
     }
 
     if (!(typeof session.release === 'string')) {
-      if (isDebugBuild()) {
-        logger.warn('Discarded session because of missing or non-string release');
-      }
+      isDebugBuild() && logger.warn('Discarded session because of missing or non-string release');
     } else {
       this._sendSession(session);
       // After sending, we set init false to indicate it's not the first occurrence
@@ -248,7 +244,7 @@ export abstract class BaseClient<B extends Backend, O extends Options> implement
     try {
       return (this._integrations[integration.id] as T) || null;
     } catch (_oO) {
-      logger.warn(`Cannot retrieve integration ${integration.id} from the current Client`);
+      isDebugBuild() && logger.warn(`Cannot retrieve integration ${integration.id} from the current Client`);
       return null;
     }
   }
@@ -507,7 +503,7 @@ export abstract class BaseClient<B extends Backend, O extends Options> implement
         return finalEvent.event_id;
       },
       reason => {
-        logger.error(reason);
+        isDebugBuild() && logger.error(reason);
         return undefined;
       },
     );

packages/core/src/integration.ts

@@ -1,6 +1,6 @@
 import { addGlobalEventProcessor, getCurrentHub } from '@sentry/hub';
 import { Integration, Options } from '@sentry/types';
-import { addNonEnumerableProperty, logger } from '@sentry/utils';
+import { addNonEnumerableProperty, isDebugBuild, logger } from '@sentry/utils';
 
 export const installedIntegrations: string[] = [];
 
@@ -59,7 +59,7 @@ export function setupIntegration(integration: Integration): void {
   }
   integration.setupOnce(addGlobalEventProcessor, getCurrentHub);
   installedIntegrations.push(integration.name);
-  logger.log(`Integration installed: ${integration.name}`);
+  isDebugBuild() && logger.log(`Integration installed: ${integration.name}`);
 }
 
 /**

packages/core/src/integrations/inboundfilters.ts

@@ -64,37 +64,33 @@ export class InboundFilters implements Integration {
   /** JSDoc */
   private _shouldDropEvent(event: Event, options: Partial<InboundFiltersOptions>): boolean {
     if (this._isSentryError(event, options)) {
-      if (isDebugBuild()) {
+      isDebugBuild() &&
         logger.warn(`Event dropped due to being internal Sentry Error.\nEvent: ${getEventDescription(event)}`);
-      }
       return true;
     }
     if (this._isIgnoredError(event, options)) {
-      if (isDebugBuild()) {
+      isDebugBuild() &&
         logger.warn(
           `Event dropped due to being matched by \`ignoreErrors\` option.\nEvent: ${getEventDescription(event)}`,
         );
-      }
       return true;
     }
     if (this._isDeniedUrl(event, options)) {
-      if (isDebugBuild()) {
+      isDebugBuild() &&
         logger.warn(
           `Event dropped due to being matched by \`denyUrls\` option.\nEvent: ${getEventDescription(
             event,
           )}.\nUrl: ${this._getEventFilterUrl(event)}`,
         );
-      }
       return true;
     }
     if (!this._isAllowedUrl(event, options)) {
-      if (isDebugBuild()) {
+      isDebugBuild() &&
         logger.warn(
           `Event dropped due to not being matched by \`allowUrls\` option.\nEvent: ${getEventDescription(
             event,
           )}.\nUrl: ${this._getEventFilterUrl(event)}`,
         );
-      }
       return true;
     }
     return false;
@@ -187,9 +183,7 @@ export class InboundFilters implements Integration {
         const { type = '', value = '' } = (event.exception.values && event.exception.values[0]) || {};
         return [`${value}`, `${type}: ${value}`];
       } catch (oO) {
-        if (isDebugBuild()) {
-          logger.error(`Cannot extract message for event ${getEventDescription(event)}`);
-        }
+        isDebugBuild() && logger.error(`Cannot extract message for event ${getEventDescription(event)}`);
         return [];
       }
     }
@@ -224,9 +218,7 @@ export class InboundFilters implements Integration {
       }
       return frames ? this._getLastValidUrl(frames) : null;
     } catch (oO) {
-      if (isDebugBuild()) {
-        logger.error(`Cannot extract url for event ${getEventDescription(event)}`);
-      }
+      isDebugBuild() && logger.error(`Cannot extract url for event ${getEventDescription(event)}`);
       return null;
     }
   }

packages/core/src/sdk.ts

@@ -1,6 +1,6 @@
 import { getCurrentHub } from '@sentry/hub';
 import { Client, Options } from '@sentry/types';
-import { logger } from '@sentry/utils';
+import { isDebugBuild, logger } from '@sentry/utils';
 
 /** A class object that can instantiate Client objects. */
 export type ClientClass<F extends Client, O extends Options> = new (options: O) => F;
@@ -14,7 +14,13 @@ export type ClientClass<F extends Client, O extends Options> = new (options: O)
  */
 export function initAndBind<F extends Client, O extends Options>(clientClass: ClientClass<F, O>, options: O): void {
   if (options.debug === true) {
-    logger.enable();
+    if (isDebugBuild()) {
+      logger.enable();
+    } else {
+      // use `console.warn` rather than `logger.warn` since by non-debug bundles have all `logger.x` statements stripped
+      // eslint-disable-next-line no-console
+      console.warn('[Sentry] Cannot initialize SDK with `debug` option using a non-debug bundle.');
+    }
   }
   const hub = getCurrentHub();
   const scope = hub.getScope();