ref: Switch to magic string for logger statements (#5155)

Co-authored-by: Katie Byers <[email protected]>

Author: lforst

CONTRIBUTING.md

@@ -61,16 +61,12 @@ Pro tip: If any of your breakpoints are in code run by multiple tests, and you r
 
 ## Debug Build Flags
 
-Throughout the codebase, you will find debug flags like `IS_DEBUG_BUILD` guarding various code sections.
-These flags serve two purposes:
+Throughout the codebase, you will find the `__DEBUG_BUILD__` flag guarding various code sections. This flag serves two purposes:
 
-1. They enable us to remove debug code for our production browser bundles.
-2. Enable users to tree-shake Sentry debug code for their production builds.
+1. It enables us to remove debug code from our minified CDN bundles during build, by replacing the flag with `false` before tree-shaking occurs.
+2. It enables users to remove Sentry debug code from their production bundles during their own build. When we build our npm packages, we replace the flag with `(typeof __SENTRY_DEBUG__ === 'undefined' || __SENTRY_DEBUG__)`. If the user does nothing, this evaluates to `true` and logging is included. But if the user runs their own replacement during build (again replacing the flag with `false`), the build will tree-shake the logging away, just as our bundle builds do.
 
-These debug flags need to be declared in each package individually and must not be imported across package boundaries, because some build tools have trouble tree-shaking imported guards.
-As a convention, we define debug flags in a `flags.ts` file in the root of a package's `src` folder.
-The `flags.ts` file will contain "magic strings" like `__SENTRY_DEBUG__` that may get replaced with actual values during our, or the user's build process.
-Take care when introducing new flags - they must not throw if they are not replaced.
+Note that the replacement flag, `__SENTRY_DEBUG__`, is different from the original flag . This is necessary because the replacement plugin runs twice, at two different stages of the build, and we don't want to run a replacement on our replacement (as would happen if we reused `__DEBUG_BUILD__`).
 
 ## Linting
 

jest/jest.config.js

@@ -12,6 +12,7 @@ module.exports = {
     'ts-jest': {
       tsconfig: '<rootDir>/tsconfig.test.json',
     },
+    __DEBUG_BUILD__: true,
   },
   testPathIgnorePatterns: ['<rootDir>/build/', '<rootDir>/node_modules/'],
 };

packages/angular/src/flags.ts

@@ -2,14 +2,9 @@
  * This file defines flags and constants that can be modified during compile time in order to facilitate tree shaking
  * for users.
  *
- * Debug flags need to be declared in each package individually and must not be imported across package boundaries,
- * because some build tools have trouble tree-shaking imported guards.
- *
- * As a convention, we define debug flags in a `flags.ts` file in the root of a package's `src` folder.
- *
- * Debug flag files will contain "magic strings" like `__SENTRY_DEBUG__` that may get replaced with actual values during
- * our, or the user's build process. Take care when introducing new flags - they must not throw if they are not
- * replaced.
+ * We define "magic strings" like `__SENTRY_DEBUG__` that may get replaced with actual values during our, or the user's
+ * build process. Take care when introducing new flags - they must not throw if they are not replaced. See the Debug
+ * Build Flags section in CONTRIBUTING.md.
  */
 
 declare const __SENTRY_DEBUG__: boolean;

packages/browser/src/client.ts

@@ -10,7 +10,6 @@ import {
 } from '@sentry/utils';
 
 import { eventFromException, eventFromMessage } from './eventbuilder';
-import { IS_DEBUG_BUILD } from './flags';
 import { Breadcrumbs } from './integrations';
 import { BREADCRUMB_INTEGRATION_ID } from './integrations/breadcrumbs';
 import { BrowserTransportOptions } from './transports/types';
@@ -151,24 +150,24 @@ export class BrowserClient extends BaseClient<BrowserClientOptions> {
     const outcomes = this._clearOutcomes();
 
     if (outcomes.length === 0) {
-      IS_DEBUG_BUILD && logger.log('No outcomes to send');
+      __DEBUG_BUILD__ && logger.log('No outcomes to send');
       return;
     }
 
     if (!this._dsn) {
-      IS_DEBUG_BUILD && logger.log('No dsn provided, will not send outcomes');
+      __DEBUG_BUILD__ && logger.log('No dsn provided, will not send outcomes');
       return;
     }
 
-    IS_DEBUG_BUILD && logger.log('Sending outcomes:', outcomes);
+    __DEBUG_BUILD__ && logger.log('Sending outcomes:', outcomes);
 
     const url = getEnvelopeEndpointWithUrlEncodedAuth(this._dsn, this._options.tunnel);
     const envelope = createClientReportEnvelope(outcomes, this._options.tunnel && dsnToString(this._dsn));
 
     try {
       sendReport(url, serializeEnvelope(envelope));
     } catch (e) {
-      IS_DEBUG_BUILD && logger.error(e);
+      __DEBUG_BUILD__ && logger.error(e);
     }
   }
 }

packages/browser/src/flags.ts

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

packages/browser/src/integrations/dedupe.ts

@@ -13,7 +13,6 @@ import {
 
 import { BrowserClient } from '../client';
 import { eventFromUnknownInput } from '../eventbuilder';
-import { IS_DEBUG_BUILD } from '../flags';
 import { shouldIgnoreOnError } from '../helpers';
 
 type GlobalHandlersIntegrationsOptionKeys = 'onerror' | 'onunhandledrejection';
@@ -238,7 +237,7 @@ function _enhanceEventWithInitialFrame(event: Event, url: any, line: any, column
 }
 
 function globalHandlerLog(type: string): void {
-  IS_DEBUG_BUILD && logger.log(`Global Handler attached: ${type}`);
+  __DEBUG_BUILD__ && logger.log(`Global Handler attached: ${type}`);
 }
 
 function addMechanismAndCapture(hub: Hub, error: EventHint['originalException'], event: Event, type: string): void {

packages/browser/src/integrations/globalhandlers.ts

@@ -16,7 +16,6 @@ import {
 } from '@sentry/utils';
 
 import { BrowserClient, BrowserClientOptions, BrowserOptions } from './client';
-import { IS_DEBUG_BUILD } from './flags';
 import { ReportDialogOptions, wrap as internalWrap } from './helpers';
 import { Breadcrumbs, Dedupe, GlobalHandlers, HttpContext, LinkedErrors, TryCatch } from './integrations';
 import { defaultStackParser } from './stack-parsers';
@@ -131,14 +130,14 @@ export function showReportDialog(options: ReportDialogOptions = {}, hub: Hub = g
   // doesn't work without a document (React Native)
   const global = getGlobalObject<Window>();
   if (!global.document) {
-    IS_DEBUG_BUILD && logger.error('Global document not defined in showReportDialog call');
+    __DEBUG_BUILD__ && logger.error('Global document not defined in showReportDialog call');
     return;
   }
 
   const { client, scope } = hub.getStackTop();
   const dsn = options.dsn || (client && client.getDsn());
   if (!dsn) {
-    IS_DEBUG_BUILD && logger.error('DSN not configured for showReportDialog call');
+    __DEBUG_BUILD__ && logger.error('DSN not configured for showReportDialog call');
     return;
   }
 
@@ -166,7 +165,7 @@ export function showReportDialog(options: ReportDialogOptions = {}, hub: Hub = g
   if (injectionPoint) {
     injectionPoint.appendChild(script);
   } else {
-    IS_DEBUG_BUILD && logger.error('Not injecting report dialog. No injection point found in HTML');
+    __DEBUG_BUILD__ && logger.error('Not injecting report dialog. No injection point found in HTML');
   }
 }
 
@@ -208,7 +207,7 @@ export function flush(timeout?: number): PromiseLike<boolean> {
   if (client) {
     return client.flush(timeout);
   }
-  IS_DEBUG_BUILD && logger.warn('Cannot flush events. No client defined.');
+  __DEBUG_BUILD__ && logger.warn('Cannot flush events. No client defined.');
   return resolvedSyncPromise(false);
 }
 
@@ -225,7 +224,7 @@ export function close(timeout?: number): PromiseLike<boolean> {
   if (client) {
     return client.close(timeout);
   }
-  IS_DEBUG_BUILD && logger.warn('Cannot flush events and disable SDK. No client defined.');
+  __DEBUG_BUILD__ && logger.warn('Cannot flush events and disable SDK. No client defined.');
   return resolvedSyncPromise(false);
 }
 
@@ -254,7 +253,8 @@ function startSessionTracking(): void {
   const document = window.document;
 
   if (typeof document === 'undefined') {
-    IS_DEBUG_BUILD && logger.warn('Session tracking in non-browser environment with @sentry/browser is not supported.');
+    __DEBUG_BUILD__ &&
+      logger.warn('Session tracking in non-browser environment with @sentry/browser is not supported.');
     return;
   }
 

packages/browser/src/sdk.ts

@@ -1,7 +1,5 @@
 import { getGlobalObject, isNativeFetch, logger, supportsFetch } from '@sentry/utils';
 
-import { IS_DEBUG_BUILD } from '../flags';
-
 const global = getGlobalObject<Window>();
 let cachedFetchImpl: FetchImpl;
 
@@ -71,7 +69,7 @@ export function getNativeFetchImplementation(): FetchImpl {
       }
       document.head.removeChild(sandbox);
     } catch (e) {
-      IS_DEBUG_BUILD &&
+      __DEBUG_BUILD__ &&
         logger.warn('Could not create sandbox iframe for pure fetch check, bailing to window.fetch: ', e);
     }
   }
@@ -101,6 +99,8 @@ export function sendReport(url: string, body: string | Uint8Array): void {
       method: 'POST',
       credentials: 'omit',
       keepalive: true,
-    }).then(null, error => logger.error(error));
+    }).then(null, error => {
+      __DEBUG_BUILD__ && logger.error(error);
+    });
   }
 }

packages/browser/src/transports/utils.ts

@@ -39,7 +39,6 @@ import {
 
 import { getEnvelopeEndpointWithUrlEncodedAuth } from './api';
 import { createEventEnvelope, createSessionEnvelope } from './envelope';
-import { IS_DEBUG_BUILD } from './flags';
 import { IntegrationIndex, setupIntegrations } from './integration';
 
 const ALREADY_SEEN_ERROR = "Not capturing exception because it's already been captured.";
@@ -112,7 +111,7 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
         url,
       });
     } else {
-      IS_DEBUG_BUILD && logger.warn('No DSN provided, client will not do anything.');
+      __DEBUG_BUILD__ && logger.warn('No DSN provided, client will not do anything.');
     }
   }
 
@@ -123,7 +122,7 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
   public captureException(exception: any, hint?: EventHint, scope?: Scope): string | undefined {
     // ensure we haven't captured this very object before
     if (checkOrSetAlreadyCaught(exception)) {
-      IS_DEBUG_BUILD && logger.log(ALREADY_SEEN_ERROR);
+      __DEBUG_BUILD__ && logger.log(ALREADY_SEEN_ERROR);
       return;
     }
 
@@ -173,7 +172,7 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
   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)) {
-      IS_DEBUG_BUILD && logger.log(ALREADY_SEEN_ERROR);
+      __DEBUG_BUILD__ && logger.log(ALREADY_SEEN_ERROR);
       return;
     }
 
@@ -193,12 +192,12 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
    */
   public captureSession(session: Session): void {
     if (!this._isEnabled()) {
-      IS_DEBUG_BUILD && logger.warn('SDK not enabled, will not capture session.');
+      __DEBUG_BUILD__ && logger.warn('SDK not enabled, will not capture session.');
       return;
     }
 
     if (!(typeof session.release === 'string')) {
-      IS_DEBUG_BUILD && logger.warn('Discarded session because of missing or non-string release');
+      __DEBUG_BUILD__ && 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
@@ -277,7 +276,7 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
     try {
       return (this._integrations[integration.id] as T) || null;
     } catch (_oO) {
-      IS_DEBUG_BUILD && logger.warn(`Cannot retrieve integration ${integration.id} from the current Client`);
+      __DEBUG_BUILD__ && logger.warn(`Cannot retrieve integration ${integration.id} from the current Client`);
       return null;
     }
   }
@@ -322,7 +321,7 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
       // would be `Partial<Record<SentryRequestType, Partial<Record<Outcome, number>>>>`
       // With typescript 4.1 we could even use template literal types
       const key = `${reason}:${category}`;
-      IS_DEBUG_BUILD && logger.log(`Adding outcome: "${key}"`);
+      __DEBUG_BUILD__ && logger.log(`Adding outcome: "${key}"`);
 
       // The following works because undefined + 1 === NaN and NaN is falsy
       this._outcomes[key] = this._outcomes[key] + 1 || 1;
@@ -575,7 +574,7 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
         return finalEvent.event_id;
       },
       reason => {
-        IS_DEBUG_BUILD && logger.warn(reason);
+        __DEBUG_BUILD__ && logger.warn(reason);
         return undefined;
       },
     );
@@ -683,10 +682,10 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
   protected _sendEnvelope(envelope: Envelope): void {
     if (this._transport && this._dsn) {
       this._transport.send(envelope).then(null, reason => {
-        IS_DEBUG_BUILD && logger.error('Error while sending event:', reason);
+        __DEBUG_BUILD__ && logger.error('Error while sending event:', reason);
       });
     } else {
-      IS_DEBUG_BUILD && logger.error('Transport disabled');
+      __DEBUG_BUILD__ && logger.error('Transport disabled');
     }
   }
 

packages/core/src/baseclient.ts

@@ -2,8 +2,6 @@ import { addGlobalEventProcessor, getCurrentHub } from '@sentry/hub';
 import { Integration, Options } from '@sentry/types';
 import { logger } from '@sentry/utils';
 
-import { IS_DEBUG_BUILD } from './flags';
-
 export const installedIntegrations: string[] = [];
 
 /** Map of integrations assigned to a client */
@@ -69,7 +67,7 @@ export function setupIntegrations(integrations: Integration[]): IntegrationIndex
     if (installedIntegrations.indexOf(integration.name) === -1) {
       integration.setupOnce(addGlobalEventProcessor, getCurrentHub);
       installedIntegrations.push(integration.name);
-      IS_DEBUG_BUILD && logger.log(`Integration installed: ${integration.name}`);
+      __DEBUG_BUILD__ && logger.log(`Integration installed: ${integration.name}`);
     }
   });