-
-
Notifications
You must be signed in to change notification settings - Fork 1.8k
fix(console): Re-patch console in AWS Lambda runtimes #20337
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
base: develop
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,8 +1,9 @@ | ||
| /* eslint-disable @typescript-eslint/no-explicit-any */ | ||
| /* eslint-disable @typescript-eslint/ban-types */ | ||
| import type { ConsoleLevel, HandlerDataConsole } from '../types-hoist/instrument'; | ||
| import type { WrappedFunction } from '../types-hoist/wrappedfunction'; | ||
| import { CONSOLE_LEVELS, originalConsoleMethods } from '../utils/debug-logger'; | ||
| import { fill } from '../utils/object'; | ||
| import { fill, markFunctionWrapped } from '../utils/object'; | ||
| import { GLOBAL_OBJ } from '../utils/worldwide'; | ||
| import { addHandler, maybeInstrument, triggerHandlers } from './handlers'; | ||
|
|
||
|
|
@@ -28,16 +29,70 @@ function instrumentConsole(): void { | |
| return; | ||
| } | ||
|
|
||
| fill(GLOBAL_OBJ.console, level, function (originalConsoleMethod: () => any): Function { | ||
| originalConsoleMethods[level] = originalConsoleMethod; | ||
| if (typeof process !== 'undefined' && !!process.env.LAMBDA_TASK_ROOT) { | ||
| // The AWS Lambda runtime replaces console methods AFTER our patch, which overwrites them. | ||
| patchWithDefineProperty(level); | ||
| } else { | ||
| patchWithFill(level); | ||
| } | ||
| }); | ||
| } | ||
|
|
||
| return function (...args: any[]): void { | ||
| const handlerData: HandlerDataConsole = { args, level }; | ||
| triggerHandlers('console', handlerData); | ||
| function patchWithFill(level: ConsoleLevel): void { | ||
| fill(GLOBAL_OBJ.console, level, function (originalConsoleMethod: () => any): Function { | ||
| originalConsoleMethods[level] = originalConsoleMethod; | ||
|
|
||
| const log = originalConsoleMethods[level]; | ||
| log?.apply(GLOBAL_OBJ.console, args); | ||
| }; | ||
| }); | ||
| return function (...args: any[]): void { | ||
| triggerHandlers('console', { args, level } as HandlerDataConsole); | ||
|
|
||
| const log = originalConsoleMethods[level]; | ||
| log?.apply(GLOBAL_OBJ.console, args); | ||
| }; | ||
| }); | ||
| } | ||
|
|
||
| function patchWithDefineProperty(level: ConsoleLevel): void { | ||
| const originalMethod = GLOBAL_OBJ.console[level] as (...args: unknown[]) => void; | ||
| originalConsoleMethods[level] = originalMethod; | ||
|
|
||
| let underlying: Function = originalMethod; | ||
|
|
||
| const wrapper = function (...args: any[]): void { | ||
| triggerHandlers('console', { args, level }); | ||
| underlying.apply(GLOBAL_OBJ.console, args); | ||
| }; | ||
| markFunctionWrapped(wrapper as unknown as WrappedFunction, originalMethod as unknown as WrappedFunction); | ||
|
|
||
| try { | ||
| let current: any = wrapper; | ||
|
|
||
| Object.defineProperty(GLOBAL_OBJ.console, level, { | ||
| configurable: true, | ||
| enumerable: true, | ||
| get() { | ||
| return current; | ||
| }, | ||
| // When `console[level]` is set to a new value, we want to check if it's something not done by us but by e.g. the Lambda runtime. | ||
| set(newValue) { | ||
| if ( | ||
| typeof newValue === 'function' && | ||
| // Ignore if it's set to the wrapper (e.g. by our own patch or consoleSandbox), which would cause an infinite loop. | ||
| newValue !== wrapper && | ||
| // Function is not one of our wrappers (which have __sentry_original__) and not the original (stored in originalConsoleMethods) | ||
| newValue !== originalConsoleMethods[level] && | ||
| !(newValue as WrappedFunction).__sentry_original__ | ||
| ) { | ||
| underlying = newValue; | ||
| originalConsoleMethods[level] = newValue; | ||
| current = wrapper; | ||
| } else { | ||
| // Accept as-is: consoleSandbox restores, other Sentry wrappers, or non-functions | ||
| current = newValue; | ||
| } | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Infinite recursion when third-party code wraps consoleHigh Severity The setter creates an infinite recursion when any third-party code wraps a console method using the common capture-and-call pattern (e.g. Additional Locations (1)Reviewed by Cursor Bugbot for commit 487e210. Configure here. |
||
| }, | ||
| }); | ||
| } catch { | ||
| // In case defineProperty fails (e.g. in older browsers), fall back to fill-style patching | ||
| patchWithFill(level); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,143 @@ | ||
| // Set LAMBDA_TASK_ROOT before any imports so instrumentConsole uses patchWithDefineProperty | ||
| process.env.LAMBDA_TASK_ROOT = '/var/task'; | ||
|
|
||
| import { afterAll, describe, expect, it, vi } from 'vitest'; | ||
| import { addConsoleInstrumentationHandler } from '../../../src/instrument/console'; | ||
| import type { WrappedFunction } from '../../../src/types-hoist/wrappedfunction'; | ||
| import { consoleSandbox, originalConsoleMethods } from '../../../src/utils/debug-logger'; | ||
| import { markFunctionWrapped } from '../../../src/utils/object'; | ||
| import { GLOBAL_OBJ } from '../../../src/utils/worldwide'; | ||
|
|
||
| afterAll(() => { | ||
| delete process.env.LAMBDA_TASK_ROOT; | ||
| }); | ||
|
|
||
| describe('addConsoleInstrumentationHandler in Lambda (patchWithDefineProperty)', () => { | ||
| it('calls registered handler when console.log is called', () => { | ||
| const handler = vi.fn(); | ||
| addConsoleInstrumentationHandler(handler); | ||
|
|
||
| GLOBAL_OBJ.console.log('test'); | ||
|
|
||
| expect(handler).toHaveBeenCalledWith(expect.objectContaining({ args: ['test'], level: 'log' })); | ||
| }); | ||
|
|
||
| describe('external replacement (e.g. Lambda runtime overwriting console)', () => { | ||
| it('keeps firing the handler after console.log is replaced externally', () => { | ||
| const handler = vi.fn(); | ||
| addConsoleInstrumentationHandler(handler); | ||
|
|
||
| GLOBAL_OBJ.console.log = vi.fn(); | ||
| handler.mockClear(); | ||
|
|
||
| GLOBAL_OBJ.console.log('after replacement'); | ||
|
|
||
| expect(handler).toHaveBeenCalledWith(expect.objectContaining({ args: ['after replacement'], level: 'log' })); | ||
| }); | ||
|
|
||
| it('calls the external replacement as the underlying method', () => { | ||
| addConsoleInstrumentationHandler(vi.fn()); | ||
|
|
||
| const lambdaLogger = vi.fn(); | ||
| GLOBAL_OBJ.console.log = lambdaLogger; | ||
|
|
||
| GLOBAL_OBJ.console.log('hello'); | ||
|
|
||
| expect(lambdaLogger).toHaveBeenCalledWith('hello'); | ||
| }); | ||
|
|
||
| it('always delegates to the latest replacement', () => { | ||
| addConsoleInstrumentationHandler(vi.fn()); | ||
|
|
||
| const first = vi.fn(); | ||
| const second = vi.fn(); | ||
|
|
||
| GLOBAL_OBJ.console.log = first; | ||
| GLOBAL_OBJ.console.log = second; | ||
|
|
||
| GLOBAL_OBJ.console.log('latest'); | ||
|
|
||
| expect(first).not.toHaveBeenCalled(); | ||
| expect(second).toHaveBeenCalledWith('latest'); | ||
| }); | ||
|
|
||
| it('updates originalConsoleMethods to point to the replacement', () => { | ||
| addConsoleInstrumentationHandler(vi.fn()); | ||
|
|
||
| const lambdaLogger = vi.fn(); | ||
| GLOBAL_OBJ.console.log = lambdaLogger; | ||
|
|
||
| expect(originalConsoleMethods.log).toBe(lambdaLogger); | ||
| }); | ||
| }); | ||
|
|
||
| describe('__sentry_original__ detection', () => { | ||
| it('accepts a function with __sentry_original__ without re-wrapping', () => { | ||
| const handler = vi.fn(); | ||
| addConsoleInstrumentationHandler(handler); | ||
|
|
||
| const otherWrapper = vi.fn(); | ||
| markFunctionWrapped(otherWrapper as unknown as WrappedFunction, vi.fn() as unknown as WrappedFunction); | ||
|
|
||
| GLOBAL_OBJ.console.log = otherWrapper; | ||
|
|
||
| expect(GLOBAL_OBJ.console.log).toBe(otherWrapper); | ||
| }); | ||
|
|
||
| it('does not fire our handler when a __sentry_original__ wrapper is installed', () => { | ||
| const handler = vi.fn(); | ||
| addConsoleInstrumentationHandler(handler); | ||
|
|
||
| const otherWrapper = vi.fn(); | ||
| markFunctionWrapped(otherWrapper as unknown as WrappedFunction, vi.fn() as unknown as WrappedFunction); | ||
|
|
||
| GLOBAL_OBJ.console.log = otherWrapper; | ||
| handler.mockClear(); | ||
|
|
||
| GLOBAL_OBJ.console.log('via other wrapper'); | ||
|
|
||
| expect(handler).not.toHaveBeenCalled(); | ||
| expect(otherWrapper).toHaveBeenCalledWith('via other wrapper'); | ||
| }); | ||
|
|
||
| it('re-wraps a plain function without __sentry_original__', () => { | ||
| const handler = vi.fn(); | ||
| addConsoleInstrumentationHandler(handler); | ||
|
|
||
| GLOBAL_OBJ.console.log = vi.fn(); | ||
| handler.mockClear(); | ||
|
|
||
| GLOBAL_OBJ.console.log('plain'); | ||
|
|
||
| expect(handler).toHaveBeenCalledWith(expect.objectContaining({ args: ['plain'], level: 'log' })); | ||
| }); | ||
| }); | ||
|
|
||
| describe('consoleSandbox interaction', () => { | ||
| it('does not fire the handler inside consoleSandbox', () => { | ||
| const handler = vi.fn(); | ||
| addConsoleInstrumentationHandler(handler); | ||
| handler.mockClear(); | ||
|
|
||
| consoleSandbox(() => { | ||
| GLOBAL_OBJ.console.log('sandbox message'); | ||
| }); | ||
|
|
||
| expect(handler).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it('resumes firing the handler after consoleSandbox returns', () => { | ||
| const handler = vi.fn(); | ||
| addConsoleInstrumentationHandler(handler); | ||
|
|
||
| consoleSandbox(() => { | ||
| GLOBAL_OBJ.console.log('inside sandbox'); | ||
| }); | ||
| handler.mockClear(); | ||
|
|
||
| GLOBAL_OBJ.console.log('after sandbox'); | ||
|
|
||
| expect(handler).toHaveBeenCalledWith(expect.objectContaining({ args: ['after sandbox'], level: 'log' })); | ||
| }); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,22 @@ | ||
| import { describe, expect, it, vi } from 'vitest'; | ||
| import { addConsoleInstrumentationHandler } from '../../../src/instrument/console'; | ||
| import { GLOBAL_OBJ } from '../../../src/utils/worldwide'; | ||
|
|
||
| describe('addConsoleInstrumentationHandler', () => { | ||
| it.each(['log', 'warn', 'error', 'debug', 'info'] as const)( | ||
| 'calls registered handler when console.%s is called', | ||
| level => { | ||
| const handler = vi.fn(); | ||
| addConsoleInstrumentationHandler(handler); | ||
|
|
||
| GLOBAL_OBJ.console[level]('test message'); | ||
|
|
||
| expect(handler).toHaveBeenCalledWith(expect.objectContaining({ args: ['test message'], level })); | ||
| }, | ||
| ); | ||
|
|
||
| it('calls through to the underlying console method without throwing', () => { | ||
| addConsoleInstrumentationHandler(vi.fn()); | ||
| expect(() => GLOBAL_OBJ.console.log('hello')).not.toThrow(); | ||
| }); | ||
| }); |


There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Lambda-only code increases browser bundle sizes
Low Severity
The
patchWithDefinePropertyfunction and its runtimeprocess.env.LAMBDA_TASK_ROOTcheck live in@sentry/core, so they're bundled into all browser packages despite being Lambda-specific dead code. The gzipped@sentry/browserlimit increased from 28KB to 30KB (~7%). Tree shaking can't eliminate the function because the branch is a runtime check. The PR author noted this might belong in@sentry/nodeinstead. Flagging per project rules on large browser bundle size increases.Additional Locations (1)
.size-limit.js#L198-L199Triggered by project rule: PR Review Guidelines for Cursor Bot
Reviewed by Cursor Bugbot for commit 487e210. Configure here.