-
Notifications
You must be signed in to change notification settings - Fork 41.5k
simple browser: console logs to chat #277293
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: main
Are you sure you want to change the base?
Changes from all commits
368dae8
e94686d
b5f289e
d9ed743
647c523
4f6e6c7
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 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -24,6 +24,18 @@ interface NodeDataResponse { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| bounds: IRectangle; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| interface LogEntry { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| type: 'console' | 'exception' | 'log'; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| level?: string; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| message?: string; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| args?: any[]; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| exceptionDetails?: any; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| timestamp: number; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| rawData: string; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const allConsole = new Map<number, LogEntry[]>(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| export class NativeBrowserElementsMainService extends Disposable implements INativeBrowserElementsMainService { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| _serviceBrand: undefined; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -141,6 +153,75 @@ export class NativeBrowserElementsMainService extends Disposable implements INat | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| debuggers.attach(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let sessionId: string | undefined; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const onMessage = (event: any, method: string, params: any, sessionIdFromMessage?: string) => { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| const onMessage = (event: any, method: string, params: any, sessionIdFromMessage?: string) => { | |
| /** | |
| * Types for debugger protocol messages | |
| */ | |
| interface ConsoleAPICalledParams { | |
| args?: Array<{ | |
| type: string; | |
| value?: unknown; | |
| preview?: { | |
| description?: string; | |
| }; | |
| }>; | |
| type: string; | |
| } | |
| interface ExceptionThrownParams { | |
| exceptionDetails?: { | |
| text?: string; | |
| exception?: { | |
| description?: string; | |
| }; | |
| }; | |
| } | |
| interface LogEntryAddedParams { | |
| level: string; | |
| text: string; | |
| } | |
| type DebuggerParams = ConsoleAPICalledParams | ExceptionThrownParams | LogEntryAddedParams; | |
| const onMessage = (event: unknown, method: string, params: DebuggerParams, sessionIdFromMessage?: string) => { |
Copilot
AI
Nov 13, 2025
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.
Multiple any types used (arg: any, frame: any) reduce type safety. Consider defining proper interfaces for the Chrome DevTools Protocol message structure (e.g., RemoteObject, CallFrame) to improve code maintainability.
Copilot
AI
Nov 13, 2025
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.
The onMessage event handler captures windowId from the outer scope, but windowId could potentially be undefined (as the parameter type indicates). While it's checked at line 140, using the non-null assertion (windowId!) at lines 160, 226 could fail if the check at 140 doesn't return. Consider checking windowId is defined before setting up the message handler or using a local constant to store the validated windowId.
Copilot
AI
Nov 13, 2025
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.
This comment does not accurately describe what's happening. The code already attached to the target and enabled console logging above (lines 241-250). This section is for handling detachment on IPC messages, not "starts console logging".
Copilot
AI
Nov 13, 2025
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.
Two separate event listeners are registered for 'ipc-message' events that handle different channels (vscode:cancelCurrentSession${cancelAndDetachId} and vscode:changeElementSelection${cancelAndDetachId}). Both listeners call removeAllListeners('ipc-message'), which will remove both listeners even if only one should be removed. Consider using a single listener that handles both channels, or use named listener functions that can be removed individually.
| Original file line number | Diff line number | Diff line change | ||
|---|---|---|---|---|
|
|
@@ -145,6 +145,9 @@ class SimpleBrowserOverlayWidget { | |||
| const cancelButtonLabel = localize('cancelSelectionLabel', 'Cancel'); | ||||
| cancelButton.label = cancelButtonLabel; | ||||
|
|
||||
| const attachLogs = this._showStore.add(new Button(mainContent, { ...defaultButtonStyles, supportIcons: true, title: localize('chat.attachLogs', "Attach Logs") })); | ||||
| attachLogs.icon = Codicon.bug; | ||||
|
|
||||
| const configure = this._showStore.add(new Button(mainContent, { supportIcons: true, title: localize('chat.configureElements', "Configure Attachments Sent") })); | ||||
| configure.icon = Codicon.gear; | ||||
|
|
||||
|
|
@@ -228,6 +231,10 @@ class SimpleBrowserOverlayWidget { | |||
| this._showStore.add(addDisposableListener(configure.element, 'click', () => { | ||||
| this._preferencesService.openSettings({ jsonEditor: false, query: '@id:chat.sendElementsToChat.enabled,chat.sendElementsToChat.attachCSS,chat.sendElementsToChat.attachImages' }); | ||||
| })); | ||||
|
|
||||
| this._showStore.add(addDisposableListener(attachLogs.element, 'click', async () => { | ||||
| await this.addConsolesToChat(); | ||||
| })); | ||||
| } | ||||
|
|
||||
| setActiveBrowserType(type: BrowserType | undefined) { | ||||
|
|
@@ -248,6 +255,23 @@ class SimpleBrowserOverlayWidget { | |||
| element.classList.remove('hidden'); | ||||
| } | ||||
|
|
||||
| async addConsolesToChat() { | ||||
| const logs = await this._browserElementsService.getConsoleLogs(); | ||||
| const toAttach: IChatRequestVariableEntry[] = []; | ||||
|
|
||||
| toAttach.push({ | ||||
| id: 'element-' + Date.now(), | ||||
| name: localize('consoleLogs', 'Console Logs'), | ||||
| fullName: localize('consoleLogs', 'Console Logs'), | ||||
| value: logs ?? localize('noConsoleLogs', 'No console logs captured.'), | ||||
| kind: 'element', | ||||
| icon: ThemeIcon.fromId(Codicon.bug.id), | ||||
| }); | ||||
|
|
||||
| const widget = await showChatView(this._viewService, this._layoutService) ?? this._chatWidgetService.lastFocusedWidget; | ||||
| widget?.attachmentModel?.addContext(...toAttach); | ||||
| } | ||||
|
Comment on lines
+258
to
+273
|
||||
|
|
||||
| async addElementToChat(cts: CancellationTokenSource) { | ||||
| // eslint-disable-next-line no-restricted-syntax | ||||
| const editorContainer = this._container.querySelector('.editor-container') as HTMLDivElement; | ||||
|
|
@@ -378,6 +402,7 @@ class SimpleBrowserOverlayController { | |||
| if (activeBrowserType) { | ||||
| try { | ||||
| await this._browserElementsService.startDebugSession(cts.token, activeBrowserType); | ||||
|
|
||||
|
||||
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.
The global module-level
Map(allConsole) storing console logs has no cleanup mechanism. As users open/close windows, this Map will continue to grow indefinitely, leading to a memory leak. Consider adding cleanup logic when windows are closed (e.g., in a window close event handler) to remove entries from the Map.