ref: Avoid using global singleton for logger (#8880)

This tries to simplify the logger to avoid the global lookup.
If you have more than one utils, having multiple loggers would be the
least of your problems.

Author: mydea

packages/browser-integration-tests/suites/public-api/debug/init.js

@@ -0,0 +1,8 @@
+import * as Sentry from '@sentry/browser';
+
+window.Sentry = Sentry;
+
+Sentry.init({
+  dsn: 'https://[email protected]/1337',
+  debug: true,
+});

packages/browser-integration-tests/suites/public-api/debug/test.ts

@@ -0,0 +1,41 @@
+/* eslint-disable no-console */
+import type { ConsoleMessage } from '@playwright/test';
+import { expect } from '@playwright/test';
+
+import { sentryTest } from '../../../utils/fixtures';
+
+sentryTest('logs debug messages correctly', async ({ getLocalTestUrl, page }) => {
+  const bundleKey = process.env.PW_BUNDLE || '';
+  const hasDebug = !bundleKey.includes('_min');
+
+  const url = await getLocalTestUrl({ testDir: __dirname });
+
+  const consoleMessages: string[] = [];
+
+  page.on('console', (msg: ConsoleMessage) => {
+    consoleMessages.push(msg.text());
+  });
+
+  await page.goto(url);
+
+  await page.evaluate(() => console.log('test log'));
+
+  expect(consoleMessages).toEqual(
+    hasDebug
+      ? [
+          'Sentry Logger [log]: Integration installed: InboundFilters',
+          'Sentry Logger [log]: Integration installed: FunctionToString',
+          'Sentry Logger [log]: Integration installed: TryCatch',
+          'Sentry Logger [log]: Integration installed: Breadcrumbs',
+          'Sentry Logger [log]: Global Handler attached: onerror',
+          'Sentry Logger [log]: Global Handler attached: onunhandledrejection',
+          'Sentry Logger [log]: Integration installed: GlobalHandlers',
+          'Sentry Logger [log]: Integration installed: LinkedErrors',
+          'Sentry Logger [log]: Integration installed: Dedupe',
+          'Sentry Logger [log]: Integration installed: HttpContext',
+          'Sentry Logger [warn]: Discarded session because of missing or non-string release',
+          'test log',
+        ]
+      : ['[Sentry] Cannot initialize SDK with `debug` option using a non-debug bundle.', 'test log'],
+  );
+});

packages/hub/test/scope.test.ts

@@ -9,6 +9,7 @@ describe('Scope', () => {
   afterEach(() => {
     jest.resetAllMocks();
     jest.useRealTimers();
+    GLOBAL_OBJ.__SENTRY__ = GLOBAL_OBJ.__SENTRY__ || {};
     GLOBAL_OBJ.__SENTRY__.globalEventProcessors = undefined;
   });
 

packages/integrations/test/captureconsole.test.ts

@@ -1,7 +1,13 @@
 /* eslint-disable @typescript-eslint/unbound-method */
 import type { Event, Hub, Integration } from '@sentry/types';
 import type { ConsoleLevel } from '@sentry/utils';
-import { addInstrumentationHandler, CONSOLE_LEVELS, GLOBAL_OBJ, originalConsoleMethods } from '@sentry/utils';
+import {
+  addInstrumentationHandler,
+  CONSOLE_LEVELS,
+  GLOBAL_OBJ,
+  originalConsoleMethods,
+  resetInstrumentationHandlers,
+} from '@sentry/utils';
 
 import { CaptureConsole } from '../src/captureconsole';
 
@@ -54,6 +60,8 @@ describe('CaptureConsole setup', () => {
     CONSOLE_LEVELS.forEach(key => {
       originalConsoleMethods[key] = _originalConsoleMethods[key];
     });
+
+    resetInstrumentationHandlers();
   });
 
   describe('monkeypatching', () => {

packages/replay/test/mocks/resetSdkMock.ts

@@ -1,3 +1,6 @@
+import type { EventProcessor } from '@sentry/types';
+import { getGlobalSingleton, resetInstrumentationHandlers } from '@sentry/utils';
+
 import type { Replay as ReplayIntegration } from '../../src';
 import type { ReplayContainer } from '../../src/replay';
 import type { RecordMock } from './../index';
@@ -17,9 +20,11 @@ export async function resetSdkMock({ replayOptions, sentryOptions, autoStart }:
   jest.setSystemTime(new Date(BASE_TIMESTAMP));
   jest.clearAllMocks();
   jest.resetModules();
-  // NOTE: The listeners added to `addInstrumentationHandler` are leaking
-  // @ts-ignore Don't know if there's a cleaner way to clean up old event processors
-  globalThis.__SENTRY__.globalEventProcessors = [];
+
+  // Clear all handlers that have been registered
+  resetInstrumentationHandlers();
+  getGlobalSingleton<EventProcessor[]>('globalEventProcessors', () => []).length = 0;
+
   const SentryUtils = await import('@sentry/utils');
   jest.spyOn(SentryUtils, 'addInstrumentationHandler').mockImplementation((type, handler: (args: any) => any) => {
     if (type === 'dom') {

packages/utils/src/instrument.ts

@@ -11,7 +11,7 @@ import type {
 
 import { isString } from './is';
 import type { ConsoleLevel } from './logger';
-import { CONSOLE_LEVELS, logger } from './logger';
+import { CONSOLE_LEVELS, logger, originalConsoleMethods } from './logger';
 import { fill } from './object';
 import { getFunctionName } from './stacktrace';
 import { supportsHistory, supportsNativeFetch } from './supports';
@@ -94,6 +94,16 @@ export function addInstrumentationHandler(type: InstrumentHandlerType, callback:
   instrument(type);
 }
 
+/**
+ * Reset all instrumentation handlers.
+ * This can be used by tests to ensure we have a clean slate of instrumentation handlers.
+ */
+export function resetInstrumentationHandlers(): void {
+  Object.keys(handlers).forEach(key => {
+    handlers[key as InstrumentHandlerType] = undefined;
+  });
+}
+
 /** JSDoc */
 function triggerHandlers(type: InstrumentHandlerType, data: any): void {
   if (!type || !handlers[type]) {
@@ -113,11 +123,6 @@ function triggerHandlers(type: InstrumentHandlerType, data: any): void {
   }
 }
 
-/** Only exported for testing & debugging. */
-export const originalConsoleMethods: {
-  [key in ConsoleLevel]?: (...args: any[]) => void;
-} = {};
-
 /** JSDoc */
 function instrumentConsole(): void {
   if (!('console' in GLOBAL_OBJ)) {

packages/utils/src/logger.ts

@@ -1,6 +1,4 @@
-import type { WrappedFunction } from '@sentry/types';
-
-import { getGlobalSingleton, GLOBAL_OBJ } from './worldwide';
+import { GLOBAL_OBJ } from './worldwide';
 
 /** Prefix for logging strings */
 const PREFIX = 'Sentry Logger ';
@@ -9,7 +7,13 @@ export const CONSOLE_LEVELS = ['debug', 'info', 'warn', 'error', 'log', 'assert'
 export type ConsoleLevel = (typeof CONSOLE_LEVELS)[number];
 
 type LoggerMethod = (...args: unknown[]) => void;
-type LoggerConsoleMethods = Record<(typeof CONSOLE_LEVELS)[number], LoggerMethod>;
+type LoggerConsoleMethods = Record<ConsoleLevel, LoggerMethod>;
+
+/** This may be mutated by the console instrumentation. */
+export const originalConsoleMethods: {
+  // eslint-disable-next-line @typescript-eslint/no-explicit-any
+  [key in ConsoleLevel]?: (...args: any[]) => void;
+} = {};
 
 /** JSDoc */
 interface Logger extends LoggerConsoleMethods {
@@ -28,26 +32,24 @@ export function consoleSandbox<T>(callback: () => T): T {
     return callback();
   }
 
-  const originalConsole = GLOBAL_OBJ.console as Console & Record<string, unknown>;
-  const wrappedLevels: Partial<LoggerConsoleMethods> = {};
+  const console = GLOBAL_OBJ.console as Console;
+  const wrappedFuncs: Partial<LoggerConsoleMethods> = {};
+
+  const wrappedLevels = Object.keys(originalConsoleMethods) as ConsoleLevel[];
 
   // Restore all wrapped console methods
-  CONSOLE_LEVELS.forEach(level => {
-    // TODO(v7): Remove this check as it's only needed for Node 6
-    const originalWrappedFunc =
-      originalConsole[level] && (originalConsole[level] as WrappedFunction).__sentry_original__;
-    if (level in originalConsole && originalWrappedFunc) {
-      wrappedLevels[level] = originalConsole[level] as LoggerConsoleMethods[typeof level];
-      originalConsole[level] = originalWrappedFunc as Console[typeof level];
-    }
+  wrappedLevels.forEach(level => {
+    const originalConsoleMethod = originalConsoleMethods[level] as LoggerMethod;
+    wrappedFuncs[level] = console[level] as LoggerMethod | undefined;
+    console[level] = originalConsoleMethod;
   });
 
   try {
     return callback();
   } finally {
     // Revert restoration to wrapped state
-    Object.keys(wrappedLevels).forEach(level => {
-      originalConsole[level] = wrappedLevels[level as (typeof CONSOLE_LEVELS)[number]];
+    wrappedLevels.forEach(level => {
+      console[level] = wrappedFuncs[level] as LoggerMethod;
     });
   }
 }
@@ -83,12 +85,4 @@ function makeLogger(): Logger {
   return logger as Logger;
 }
 
-// Ensure we only have a single logger instance, even if multiple versions of @sentry/utils are being used
-let logger: Logger;
-if (__DEBUG_BUILD__) {
-  logger = getGlobalSingleton('logger', makeLogger);
-} else {
-  logger = makeLogger();
-}
-
-export { logger };
+export const logger = makeLogger();