Skip to content

fix(utils): Prevent logger circular dependencies #4069

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
Oct 15, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 13 additions & 0 deletions packages/utils/src/browser.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { getGlobalObject } from './global';
import { isString } from './is';

/**
Expand Down Expand Up @@ -108,3 +109,15 @@ function _htmlElementAsString(el: unknown, keyAttrs?: string[]): string {
}
return out.join('');
}

/**
* A safe form of location.href
*/
export function getLocationHref(): string {
const global = getGlobalObject<Window>();
try {
return global.document.location.href;
} catch (oO) {
return '';
}
}
44 changes: 44 additions & 0 deletions packages/utils/src/global.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
/**
* NOTE: In order to avoid circular dependencies, if you add a function to this module and it needs to print something,
* you must either a) use `console.log` rather than the logger, or b) put your function elsewhere.
*/

/* eslint-disable @typescript-eslint/no-explicit-any */

import { Integration } from '@sentry/types';

import { isNodeEnv } from './node';

/** Internal */
interface SentryGlobal {
Sentry?: {
Integrations?: Integration[];
};
SENTRY_ENVIRONMENT?: string;
SENTRY_DSN?: string;
SENTRY_RELEASE?: {
id?: string;
};
__SENTRY__: {
globalEventProcessors: any;
hub: any;
logger: any;
};
}

const fallbackGlobalObject = {};

/**
* Safely get global scope object
*
* @returns Global scope object
*/
export function getGlobalObject<T>(): T & SentryGlobal {
return (isNodeEnv()
? global
: typeof window !== 'undefined' // eslint-disable-line no-restricted-globals
? window // eslint-disable-line no-restricted-globals
: typeof self !== 'undefined'
? self
: fallbackGlobalObject) as T & SentryGlobal;
}
1 change: 1 addition & 0 deletions packages/utils/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ export * from './async';
export * from './browser';
export * from './dsn';
export * from './error';
export * from './global';
export * from './instrument';
export * from './is';
export * from './logger';
Expand Down
2 changes: 1 addition & 1 deletion packages/utils/src/instrument.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@
/* eslint-disable @typescript-eslint/ban-types */
import { WrappedFunction } from '@sentry/types';

import { getGlobalObject } from './global';
import { isInstanceOf, isString } from './is';
import { logger } from './logger';
import { getGlobalObject } from './misc';
import { fill } from './object';
import { getFunctionName } from './stacktrace';
import { supportsHistory, supportsNativeFetch } from './supports';
Expand Down
48 changes: 47 additions & 1 deletion packages/utils/src/logger.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,58 @@
/* eslint-disable @typescript-eslint/no-explicit-any */
import { consoleSandbox, getGlobalObject } from './misc';
import { WrappedFunction } from '@sentry/types';

import { getGlobalObject } from './global';

// TODO: Implement different loggers for different environments
const global = getGlobalObject<Window | NodeJS.Global>();

/** Prefix for logging strings */
const PREFIX = 'Sentry Logger ';

/** JSDoc */
interface ExtensibleConsole extends Console {
[key: string]: any;
}

/**
* Temporarily unwrap `console.log` and friends in order to perform the given callback using the original methods.
* Restores wrapping after the callback completes.
*
* @param callback The function to run against the original `console` messages
* @returns The results of the callback
*/
export function consoleSandbox(callback: () => any): any {
const global = getGlobalObject<Window>();
const levels = ['debug', 'info', 'warn', 'error', 'log', 'assert'];

if (!('console' in global)) {
return callback();
}

// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
const originalConsole = (global as any).console as ExtensibleConsole;
const wrappedLevels: { [key: string]: any } = {};

// Restore all wrapped console methods
levels.forEach(level => {
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
if (level in (global as any).console && (originalConsole[level] as WrappedFunction).__sentry_original__) {
wrappedLevels[level] = originalConsole[level] as WrappedFunction;
originalConsole[level] = (originalConsole[level] as WrappedFunction).__sentry_original__;
}
});

// Perform callback manipulations
const result = callback();

// Revert restoration to wrapped state
Object.keys(wrappedLevels).forEach(level => {
originalConsole[level] = wrappedLevels[level];
});

return result;
}

/** JSDoc */
class Logger {
/** JSDoc */
Expand Down
88 changes: 2 additions & 86 deletions packages/utils/src/misc.ts
Original file line number Diff line number Diff line change
@@ -1,43 +1,9 @@
/* eslint-disable @typescript-eslint/no-explicit-any */
import { Event, Integration, StackFrame, WrappedFunction } from '@sentry/types';
import { Event, StackFrame } from '@sentry/types';

import { isNodeEnv } from './node';
import { getGlobalObject } from './global';
import { snipLine } from './string';

/** Internal */
interface SentryGlobal {
Sentry?: {
Integrations?: Integration[];
};
SENTRY_ENVIRONMENT?: string;
SENTRY_DSN?: string;
SENTRY_RELEASE?: {
id?: string;
};
__SENTRY__: {
globalEventProcessors: any;
hub: any;
logger: any;
};
}

const fallbackGlobalObject = {};

/**
* Safely get global scope object
*
* @returns Global scope object
*/
export function getGlobalObject<T>(): T & SentryGlobal {
return (isNodeEnv()
? global
: typeof window !== 'undefined' // eslint-disable-line no-restricted-globals
? window // eslint-disable-line no-restricted-globals
: typeof self !== 'undefined'
? self
: fallbackGlobalObject) as T & SentryGlobal;
}

/**
* Extended Window interface that allows for Crypto API usage in IE browsers
*/
Expand Down Expand Up @@ -143,44 +109,6 @@ export function getEventDescription(event: Event): string {
return event.event_id || '<unknown>';
}

/** JSDoc */
interface ExtensibleConsole extends Console {
[key: string]: any;
}

/** JSDoc */
export function consoleSandbox(callback: () => any): any {
const global = getGlobalObject<Window>();
const levels = ['debug', 'info', 'warn', 'error', 'log', 'assert'];

if (!('console' in global)) {
return callback();
}

// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
const originalConsole = (global as any).console as ExtensibleConsole;
const wrappedLevels: { [key: string]: any } = {};

// Restore all wrapped console methods
levels.forEach(level => {
// eslint-disable-next-line @typescript-eslint/no-unsafe-member-access
if (level in (global as any).console && (originalConsole[level] as WrappedFunction).__sentry_original__) {
wrappedLevels[level] = originalConsole[level] as WrappedFunction;
originalConsole[level] = (originalConsole[level] as WrappedFunction).__sentry_original__;
}
});

// Perform callback manipulations
const result = callback();

// Revert restoration to wrapped state
Object.keys(wrappedLevels).forEach(level => {
originalConsole[level] = wrappedLevels[level];
});

return result;
}

/**
* Adds exception values, type and value to an synthetic Exception.
* @param event The event to modify.
Expand Down Expand Up @@ -223,18 +151,6 @@ export function addExceptionMechanism(
}
}

/**
* A safe form of location.href
*/
export function getLocationHref(): string {
const global = getGlobalObject<Window>();
try {
return global.document.location.href;
} catch (oO) {
return '';
}
}

// https://semver.org/#is-there-a-suggested-regular-expression-regex-to-check-a-semver-string
const SEMVER_REGEXP = /^(0|[1-9]\d*)\.(0|[1-9]\d*)\.(0|[1-9]\d*)(?:-((?:0|[1-9]\d*|\d*[a-zA-Z-][0-9a-zA-Z-]*)(?:\.(?:0|[1-9]\d*|\d*[a-zA-Z-][0-9a-zA-Z-]*))*))?(?:\+([0-9a-zA-Z-]+(?:\.[0-9a-zA-Z-]+)*))?$/;

Expand Down
5 changes: 5 additions & 0 deletions packages/utils/src/node.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
/**
* NOTE: In order to avoid circular dependencies, if you add a function to this module and it needs to print something,
* you must either a) use `console.log` rather than the logger, or b) put your function elsewhere.
*/

/**
* Checks whether we're in the Node.js or Browser environment
*
Expand Down
2 changes: 1 addition & 1 deletion packages/utils/src/supports.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { getGlobalObject } from './global';
import { logger } from './logger';
import { getGlobalObject } from './misc';

/**
* Tells whether current environment supports ErrorEvent objects
Expand Down
2 changes: 1 addition & 1 deletion packages/utils/src/time.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { getGlobalObject } from './misc';
import { getGlobalObject } from './global';
import { dynamicRequire, isNodeEnv } from './node';

/**
Expand Down
12 changes: 12 additions & 0 deletions packages/utils/test/global.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import { getGlobalObject } from '../src/global';

describe('getGlobalObject()', () => {
test('should return the same object', () => {
const backup = global.process;
delete global.process;
const first = getGlobalObject();
const second = getGlobalObject();
expect(first).toEqual(second);
global.process = backup;
});
});
19 changes: 1 addition & 18 deletions packages/utils/test/misc.test.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,6 @@
import { StackFrame } from '@sentry/types';

import {
addContextToFrame,
getEventDescription,
getGlobalObject,
parseRetryAfterHeader,
stripUrlQueryAndFragment,
} from '../src/misc';
import { addContextToFrame, getEventDescription, parseRetryAfterHeader, stripUrlQueryAndFragment } from '../src/misc';

describe('getEventDescription()', () => {
test('message event', () => {
Expand Down Expand Up @@ -117,17 +111,6 @@ describe('getEventDescription()', () => {
});
});

describe('getGlobalObject()', () => {
test('should return the same object', () => {
const backup = global.process;
delete global.process;
const first = getGlobalObject();
const second = getGlobalObject();
expect(first).toEqual(second);
global.process = backup;
});
});

describe('parseRetryAfterHeader', () => {
test('no header', () => {
expect(parseRetryAfterHeader(Date.now())).toEqual(60 * 1000);
Expand Down