-
-
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
Changes from 13 commits
88c6f55
d1cfd60
487e210
dc97744
d4d89f6
e7709a1
8a1fc4d
249d85d
e3e3636
2821b31
a93810e
aef0ddd
6b3ee74
3268548
ef9eaa6
125c6db
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 |
|---|---|---|
| @@ -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(); | ||
| }); | ||
| }); |
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
| @@ -0,0 +1,119 @@ | ||||||
| /* eslint-disable @typescript-eslint/no-explicit-any */ | ||||||
| /* eslint-disable @typescript-eslint/ban-types */ | ||||||
| import type { ConsoleLevel, HandlerDataConsole, WrappedFunction } from '@sentry/core'; | ||||||
| import { | ||||||
| CONSOLE_LEVELS, | ||||||
| GLOBAL_OBJ, | ||||||
| consoleIntegration as coreConsoleIntegration, | ||||||
| defineIntegration, | ||||||
| fill, | ||||||
| markFunctionWrapped, | ||||||
| maybeInstrument, | ||||||
| originalConsoleMethods, | ||||||
| triggerHandlers, | ||||||
| } from '@sentry/core'; | ||||||
|
|
||||||
| interface ConsoleIntegrationOptions { | ||||||
| levels: ConsoleLevel[]; | ||||||
| } | ||||||
|
|
||||||
| /** | ||||||
| * Node-specific console integration that captures breadcrumbs and handles | ||||||
| * the AWS Lambda runtime replacing console methods after our patch. | ||||||
| * | ||||||
| * In Lambda, console methods are patched via `Object.defineProperty` so that | ||||||
| * external replacements (by the Lambda runtime) are absorbed as the delegate | ||||||
| * while our wrapper stays in place. Outside Lambda, this delegates entirely | ||||||
| * to the core `consoleIntegration` which uses the simpler `fill`-based patch. | ||||||
| */ | ||||||
| export const consoleIntegration = defineIntegration((options: Partial<ConsoleIntegrationOptions> = {}) => { | ||||||
| return { | ||||||
| name: 'Console', | ||||||
| setup(client) { | ||||||
| if (process.env.LAMBDA_TASK_ROOT) { | ||||||
| maybeInstrument('console', instrumentConsoleLambda); | ||||||
| } | ||||||
|
|
||||||
| // Delegate breadcrumb handling to the core console integration. | ||||||
| const core = coreConsoleIntegration(options); | ||||||
| core.setup?.(client); | ||||||
| }, | ||||||
| }; | ||||||
| }); | ||||||
|
|
||||||
| function instrumentConsoleLambda(): void { | ||||||
| if (!('console' in GLOBAL_OBJ)) { | ||||||
| return; | ||||||
| } | ||||||
|
|
||||||
| CONSOLE_LEVELS.forEach(function (level: ConsoleLevel): void { | ||||||
| if (!(level in GLOBAL_OBJ.console)) { | ||||||
| return; | ||||||
| } | ||||||
|
|
||||||
| patchWithDefineProperty(level); | ||||||
| }); | ||||||
| } | ||||||
|
|
||||||
| function patchWithDefineProperty(level: ConsoleLevel): void { | ||||||
| const nativeMethod = GLOBAL_OBJ.console[level] as (...args: unknown[]) => void; | ||||||
| originalConsoleMethods[level] = nativeMethod; | ||||||
|
|
||||||
| let consoleDelegate: Function = nativeMethod; | ||||||
| let isExecuting = false; | ||||||
|
|
||||||
| const wrapper = function (...args: any[]): void { | ||||||
| if (isExecuting) { | ||||||
| // Re-entrant call: a third party captured `wrapper` via the getter and calls it | ||||||
| // from inside their replacement (e.g. `const prev = console.log; console.log = (...a) => { prev(...a); }`). | ||||||
| // Calling `consoleDelegate` here would recurse, so fall back to the native method. | ||||||
| nativeMethod.apply(GLOBAL_OBJ.console, args); | ||||||
| return; | ||||||
| } | ||||||
|
isaacs marked this conversation as resolved.
|
||||||
| isExecuting = true; | ||||||
| try { | ||||||
| triggerHandlers('console', { args, level } as HandlerDataConsole); | ||||||
| consoleDelegate.apply(GLOBAL_OBJ.console, args); | ||||||
| } finally { | ||||||
| isExecuting = false; | ||||||
| } | ||||||
| }; | ||||||
| markFunctionWrapped(wrapper as unknown as WrappedFunction, nativeMethod as unknown as WrappedFunction); | ||||||
|
|
||||||
| try { | ||||||
| let current: any = wrapper; | ||||||
|
|
||||||
| Object.defineProperty(GLOBAL_OBJ.console, level, { | ||||||
| configurable: true, | ||||||
| enumerable: true, | ||||||
| get() { | ||||||
| return current; | ||||||
| }, | ||||||
| set(newValue) { | ||||||
| if ( | ||||||
| typeof newValue === 'function' && | ||||||
| newValue !== wrapper && | ||||||
| newValue !== originalConsoleMethods[level] && | ||||||
| !(newValue as WrappedFunction).__sentry_original__ | ||||||
|
Member
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. Clobbering the wrapper if it's a sentry wrapped function makes sense (since that's likely ourselves doing it), but I'm unclear why you're allowing it to be set back to the originalConsoleMethods[level]. That would mean: const original = console.log
// Sentry setup happens
console.log = someAwsThing;
// later...
console.log = original; // lose the Sentry instrumentation!It seems like setting it to the original should just set the consoleDelegate, no?
Member
Author
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. True, that's probably a bug. I'm gonna look into that. Currently, it makes sure that the
Member
Author
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. Updated and added a test for that - |
||||||
| ) { | ||||||
| consoleDelegate = newValue; | ||||||
| current = wrapper; | ||||||
| } else { | ||||||
| current = newValue; | ||||||
| } | ||||||
| }, | ||||||
| }); | ||||||
| } catch { | ||||||
| // Fall back to fill-based patching if defineProperty fails | ||||||
| fill(GLOBAL_OBJ.console, level, function (originalConsoleMethod: () => any): Function { | ||||||
| originalConsoleMethods[level] = originalConsoleMethod; | ||||||
|
|
||||||
| return function (...args: any[]): void { | ||||||
| triggerHandlers('console', { args, level } as HandlerDataConsole); | ||||||
|
|
||||||
| const log = originalConsoleMethods[level]; | ||||||
| log?.apply(GLOBAL_OBJ.console, args); | ||||||
|
Member
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.
Suggested change
|
||||||
| }; | ||||||
| }); | ||||||
| } | ||||||
| } | ||||||
Uh oh!
There was an error while loading. Please reload this page.