DRAFT: feat(analytics-core): add console observer helper #1478
Conversation
Add
|
|
bugbot run |
![macroscopeapp[bot]](https://avatars.githubusercontent.com/in/900172?s=60&v=4){kind=small}
| get(target: Console, prop: ConsoleLogLevel) { | ||
| return function (...args: any[]) { | ||
| if (handlers.has(prop)) { | ||
| const callbacks = handlers.get(prop); | ||
| if (callbacks) { | ||
| callbacks.forEach((callback) => callback(prop, args)); | ||
| } | ||
| } | ||
| /* eslint-disable-next-line @typescript-eslint/no-unsafe-argument */ | ||
| return (target[prop] as (...args: any[]) => void)(...args); | ||
| }; | ||
| }, | ||
| }; |
There was a problem hiding this comment.
The Proxy get trap wraps every console property. This breaks non-function props (e.g., console.memory). Consider only wrapping when target[prop] is a function; otherwise return the value as-is.
| get(target: Console, prop: ConsoleLogLevel) { | |
| return function (...args: any[]) { | |
| if (handlers.has(prop)) { | |
| const callbacks = handlers.get(prop); | |
| if (callbacks) { | |
| callbacks.forEach((callback) => callback(prop, args)); | |
| } | |
| } | |
| /* eslint-disable-next-line @typescript-eslint/no-unsafe-argument */ | |
| return (target[prop] as (...args: any[]) => void)(...args); | |
| }; | |
| }, | |
| }; | |
| get(target: Console, prop: ConsoleLogLevel) { | |
| const value = target[prop]; | |
| if (typeof value !== 'function') return value; | |
| return function (...args: any[]) { | |
| if (handlers.has(prop)) { | |
| const callbacks = handlers.get(prop); | |
| if (callbacks) { | |
| callbacks.forEach((callback) => callback(prop, args)); | |
| } | |
| } | |
| /* eslint-disable-next-line @typescript-eslint/no-unsafe-argument */ | |
| return (value as (...args: any[]) => void)(...args); | |
| }; | |
| }, |
🚀 Want me to fix this? Reply ex: "fix it for me".
There was a problem hiding this comment.
Pull request overview
This PR introduces a console observer utility that intercepts global console methods and allows callbacks to be registered for different log levels. The implementation provides a Proxy-based approach to observe console calls without breaking existing functionality.
Key Changes:
- New console observer utility with observe/disconnect/restore capabilities
- Unit tests for basic observation and disconnection behavior
- Demo HTML page for manual testing
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
packages/analytics-core/src/observers/console.ts |
Core implementation of console observer using Proxy API to intercept console methods |
packages/analytics-core/src/index.ts |
Exports the consoleObserver for public use |
packages/analytics-core/test/observers/console.test.ts |
Unit tests covering observe and disconnect functionality |
test-server/observers/console.html |
Demo page for manual verification of console observation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| type Callback = (logLevel: ConsoleLogLevel, args: any[]) => void; | ||
|
|
||
| const handlers = new Map<ConsoleLogLevel, Array<Callback>>(); | ||
|
|
||
| function overrideConsole() { | ||
| /* istanbul ignore if */ | ||
| if (isOverridden || !originalConsole || !globalScope) return; | ||
|
|
||
| // use Proxy to override the console method | ||
| const handler = { | ||
| // get args from console method call | ||
| get(target: Console, prop: ConsoleLogLevel) { | ||
| return function (...args: any[]) { | ||
| if (handlers.has(prop)) { | ||
| const callbacks = handlers.get(prop); | ||
| if (callbacks) { | ||
| callbacks.forEach((callback) => callback(prop, args)); | ||
| } | ||
| } | ||
| /* eslint-disable-next-line @typescript-eslint/no-unsafe-argument */ | ||
| return (target[prop] as (...args: any[]) => void)(...args); | ||
| }; | ||
| }, | ||
| }; | ||
| const proxy = new Proxy(originalConsole, handler); | ||
| globalScope.console = proxy; | ||
| isOverridden = true; | ||
| } | ||
|
|
||
| async function observe(level: ConsoleLogLevel, callback: Callback) { | ||
| if (handlers.has(level)) { | ||
| // using ! is safe because we know the key exists based on has() condition | ||
| handlers.get(level)!.push(callback); | ||
| } else { | ||
| handlers.set(level, [callback]); | ||
| } | ||
| overrideConsole(); | ||
| } | ||
|
|
||
| function disconnectHandler(callback: Callback) { | ||
| handlers.forEach((callbacks, level) => { | ||
| if (callbacks.includes(callback)) { | ||
| handlers.delete(level); | ||
| } | ||
| }); | ||
| } | ||
|
|
||
| // this should only be used for testing | ||
| // restoring console can break other console overrides | ||
| function _restoreConsole() { | ||
| if (globalScope) { | ||
| /* eslint-disable-next-line @typescript-eslint/no-unsafe-member-access */ | ||
| (globalScope as any).console = originalConsole; | ||
| } | ||
| isOverridden = false; | ||
| handlers.clear(); | ||
| } | ||
|
|
||
| const consoleObserver = { | ||
| observe, | ||
| disconnectHandler, | ||
| _restoreConsole, | ||
| }; |
There was a problem hiding this comment.
The Callback type and consoleObserver methods lack documentation. Public APIs should have JSDoc comments explaining parameters, return values, and behavior, especially for an exported utility that overrides global console behavior.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
| handlers.delete(level); | ||
| } | ||
| }); | ||
| } |
There was a problem hiding this comment.
disconnectHandler removes all callbacks for a log level
High Severity
The disconnectHandler function uses handlers.delete(level) which removes the entire level entry from the map, rather than just removing the specific callback from the array. When multiple callbacks are registered for the same log level (e.g., two callbacks for 'log'), calling disconnectHandler with one callback removes all callbacks for that level, not just the specified one. The fix would be to splice/filter the specific callback from the array instead of deleting the entire level.
🔬 Verification Test
Test code:
// Added to packages/analytics-core/test/observers/console.test.ts
it('should only disconnect the specified callback when multiple exist', async () => {
const callback1 = jest.fn();
const callback2 = jest.fn();
await consoleObserver.observe('log', callback1);
await consoleObserver.observe('log', callback2);
console.log('before disconnect');
expect(callback1).toHaveBeenCalledTimes(1);
expect(callback2).toHaveBeenCalledTimes(1);
// Disconnect only callback1
consoleObserver.disconnectHandler(callback1);
console.log('after disconnect');
// callback1 should not be called again
expect(callback1).toHaveBeenCalledTimes(1);
// callback2 should still be called
expect(callback2).toHaveBeenCalledTimes(2);
});Command run:
cd packages/analytics-core && npx jest test/observers/console.test.ts --testNamePattern="should only disconnect"
Output:
FAIL test/observers/console.test.ts
consoleObserver
disconnect
✕ should only disconnect the specified callback when multiple exist (11 ms)
● consoleObserver › disconnect › should only disconnect the specified callback when multiple exist
expect(received).toHaveBeenCalledTimes(expected)
Expected number of calls: 2
Received number of calls: 1
at Object.<anonymous> (test/observers/console.test.ts:XX:XX)
Why this proves the bug: The test expects callback2 to still receive console.log events after only callback1 is disconnected, but it doesn't. This proves disconnectHandler removes all callbacks for the level instead of just the specified one.
daniel-graham-amplitude
force-pushed
the
AMP-146148-console-observer-helper
branch
from
0a25305 to
21c96dd
Compare
daniel-graham-amplitude
force-pushed
the
AMP-146148-console-observer-helper
branch
from
21c96dd to
cdf10a6
Compare
Summary
Add a helper that overrides the MDN Console and add callbacks to the different log levels ('error', 'log', etc...) that get triggered when the function is called
Checklist
Note
Adds a lightweight observer to intercept console calls and invoke callbacks per log level.
consoleObserverinobservers/console.tsuses aProxyto wrapconsoleand dispatch callbacks viaobserve/disconnectHandler(with_restoreConsolefor tests)consoleObserverfromsrc/index.tsconsole.test.tstest-server/observers/console.htmlshowcasing usageWritten by Cursor Bugbot for commit 0a25305. This will update automatically on new commits. Configure here.