refactor: remove browser extension recording (CDP-only)#1159
refactor: remove browser extension recording (CDP-only)#1159
Conversation
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 4 potential issues.
Bugbot Autofix prepared fixes for all 4 issues found in the latest run.
- ✅ Fixed: CDP error shown as "Failed to load extension"
- Replaced 'extension-load' error reason with 'cdp-connection' and updated the user-facing error message to accurately describe CDP debugger connection failures.
- ✅ Fixed: Duplicate EventEmitter classes at two paths
- Removed duplicate EventEmitter class from src/recording/utils/events.ts and updated the import in src/recording/messaging/index.ts to use the canonical version from src/recording/events.ts.
- ✅ Fixed: Unused
settingsparameter andRecorderSettingsimport- Removed unused settings parameter from LaunchBrowserArgs type and launchBrowser function, along with the RecorderSettings import and the settings argument passed by the caller.
- ✅ Fixed: Duplicate global
Window.__K6_STUDIO_TAB_ID__declarations- Removed redundant src/recording/globals.d.ts file since the same global type augmentation already exists in src/recording/utils.ts where it is actually used.
Or push these changes by commenting:
@cursor push f137614d2f
Preview (f137614d2f)
diff --git a/src/handlers/browser/index.ts b/src/handlers/browser/index.ts
--- a/src/handlers/browser/index.ts
+++ b/src/handlers/browser/index.ts
@@ -18,10 +18,7 @@
const browserWindow = browserWindowFromEvent(event)
try {
- k6StudioState.currentRecordingSession = await launchBrowser({
- ...options,
- settings: k6StudioState.appSettings.recorder,
- })
+ k6StudioState.currentRecordingSession = await launchBrowser(options)
k6StudioState.currentRecordingSession.on('record', (event) => {
browserWindow.webContents.send(
diff --git a/src/handlers/browser/launch.ts b/src/handlers/browser/launch.ts
--- a/src/handlers/browser/launch.ts
+++ b/src/handlers/browser/launch.ts
@@ -1,19 +1,15 @@
-import { RecorderSettings } from '@/types/settings'
-
import { launchBrowserWithDevToolsProtocol } from './recorders/cdp'
import { launchBrowserWithHttpOnly } from './recorders/http'
import { RecordingSession } from './recorders/types'
import { LaunchBrowserOptions } from './types'
-type LaunchBrowserArgs = LaunchBrowserOptions & { settings: RecorderSettings }
-
/**
* Starts a browser instance for recording. Throws if the browser fails to start.
* Runtime errors during the recording session are emitted via events on the
* `RecordingSession` instance.
*/
export async function launchBrowser(
- args: LaunchBrowserArgs
+ args: LaunchBrowserOptions
): Promise<RecordingSession> {
if (args.capture.browser) {
return launchBrowserWithDevToolsProtocol('pipe', args.url)
diff --git a/src/handlers/browser/recorders/cdp/index.ts b/src/handlers/browser/recorders/cdp/index.ts
--- a/src/handlers/browser/recorders/cdp/index.ts
+++ b/src/handlers/browser/recorders/cdp/index.ts
@@ -215,7 +215,7 @@
.catch((error) => {
process.kill()
- reject(new BrowserLaunchError('extension-load', error))
+ reject(new BrowserLaunchError('cdp-connection', error))
})
})
diff --git a/src/handlers/browser/types.ts b/src/handlers/browser/types.ts
--- a/src/handlers/browser/types.ts
+++ b/src/handlers/browser/types.ts
@@ -16,7 +16,7 @@
export type LaunchBrowserErrorReason =
| 'websocket-server-error'
- | 'extension-load'
+ | 'cdp-connection'
| 'browser-launch'
| 'unknown'
diff --git a/src/recording/globals.d.ts b/src/recording/globals.d.ts
deleted file mode 100644
--- a/src/recording/globals.d.ts
+++ /dev/null
@@ -1,7 +1,0 @@
-declare global {
- interface Window {
- __K6_STUDIO_TAB_ID__?: string
- }
-}
-
-export {}
\ No newline at end of file
diff --git a/src/recording/messaging/index.ts b/src/recording/messaging/index.ts
--- a/src/recording/messaging/index.ts
+++ b/src/recording/messaging/index.ts
@@ -1,6 +1,6 @@
import { z } from 'zod'
-import { EventEmitter } from '../utils/events'
+import { EventEmitter } from '../events'
import { NullTransport } from './transports/null'
import { Sender, SenderSchema, Transport } from './transports/transport'
diff --git a/src/recording/utils/events.ts b/src/recording/utils/events.ts
deleted file mode 100644
--- a/src/recording/utils/events.ts
+++ /dev/null
@@ -1,79 +1,0 @@
-export type EventMap<Map> = {
- [Type in keyof Map]: unknown
-}
-
-type EventListeners<Events extends EventMap<Events>> = {
- [Type in keyof Events]?: Array<(event: Events[Type]) => void>
-}
-
-/**
- * Super-simple cross-platform event emitter.
- */
-export class EventEmitter<Events extends EventMap<Events>> {
- #listeners: EventListeners<Events> = {}
- #queue: Array<() => void> | null = null
-
- constructor() {
- this.#listeners = {}
- }
-
- on<Type extends keyof Events>(
- type: Type,
- listener: (ev: Events[Type]) => void
- ) {
- if (this.#listeners[type] === undefined) {
- this.#listeners[type] = []
- }
-
- this.#listeners[type].push(listener)
-
- return () => {
- this.off(type, listener)
- }
- }
-
- off<Type extends keyof Events>(
- type: Type,
- listener: (ev: Events[Type]) => void
- ) {
- this.#listeners[type] = this.#listeners[type]?.filter((l) => l !== listener)
- }
-
- emit<Type extends keyof Events>(type: Type, event: Events[Type]) {
- // The first to emit processes the queue. Emits while processing will be
- // deferred until after all listeners of the current event have been called.
- if (this.#queue !== null) {
- this.#queue.push(this.#defer(type, event))
-
- return
- }
-
- this.#queue = [this.#defer(type, event)]
-
- while (this.#queue.length > 0) {
- const fn = this.#queue.shift()
-
- if (fn) {
- fn()
- }
- }
-
- this.#queue = null
- }
-
- #defer<Type extends keyof Events>(type: Type, event: Events[Type]) {
- return () => {
- const listeners = this.#listeners[type]
-
- if (listeners) {
- for (const listener of listeners) {
- try {
- listener(event)
- } catch (error) {
- console.log('Error in event listener', error)
- }
- }
- }
- }
- }
-}
\ No newline at end of file
diff --git a/src/views/Recorder/Recorder.hooks.tsx b/src/views/Recorder/Recorder.hooks.tsx
--- a/src/views/Recorder/Recorder.hooks.tsx
+++ b/src/views/Recorder/Recorder.hooks.tsx
@@ -44,12 +44,12 @@
})
break
- case 'extension-load':
+ case 'cdp-connection':
showToast({
status: 'error',
- title: 'Failed to load extension',
+ title: 'Failed to connect to browser',
description:
- 'Loading the browser extension failed. Browser recording will not be available.',
+ 'Could not establish a connection to the browser debugger. Browser recording will not be available.',
action: (
<Button onClick={() => openSettingsDialog('logs')}>
Open log fileYou can send follow-ups to the cloud agent here.
Reviewed by Cursor Bugbot for commit 77e6f78. Configure here.
| import { WebSocketTransport } from '@/utils/cdp/transports/webSocket' | ||
| import { readResource } from '@/utils/resources' | ||
| import { exhaustive } from '@/utils/typescript' | ||
| import { EventEmitter } from 'extension/src/utils/events' |
There was a problem hiding this comment.
CDP error shown as "Failed to load extension"
High Severity
When the CDP debugger connection fails, BrowserLaunchError('extension-load', error) is thrown. The 'extension-load' reason triggers a user-facing toast saying "Failed to load extension" and "Loading the browser extension failed." Since extensions have been removed in this PR, the error message is now completely wrong and misleading for what is actually a CDP connection failure. The LaunchBrowserErrorReason type still includes 'extension-load' as well.
Reviewed by Cursor Bugbot for commit 77e6f78. Configure here.
| } | ||
| } | ||
| } | ||
| } |
There was a problem hiding this comment.
Duplicate EventEmitter classes at two paths
Medium Severity
There are two identical EventEmitter class implementations: src/recording/events.ts and src/recording/utils/events.ts. The messaging transport imports from @/recording/events while src/recording/messaging/index.ts imports from ../utils/events. Having two copies of the same class at different import paths creates confusion about which is canonical and risks them diverging over time.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 77e6f78. Configure here.
| import { exhaustive } from '@/utils/typescript' | ||
|
|
||
| import { launchBrowserWithDevToolsProtocol } from './recorders/cdp' | ||
| import { launchBrowserWithExtension } from './recorders/extension' |
There was a problem hiding this comment.
Unused settings parameter and RecorderSettings import
Low Severity
RecorderSettings is imported and included in the LaunchBrowserArgs type, but the settings field is never accessed in launchBrowser. The caller in src/handlers/browser/index.ts still passes settings: k6StudioState.appSettings.recorder, making this dead code left over from the refactoring.
Reviewed by Cursor Bugbot for commit 77e6f78. Configure here.
| } | ||
| } | ||
|
|
||
| export {} |
There was a problem hiding this comment.
Duplicate global Window.__K6_STUDIO_TAB_ID__ declarations
Low Severity
The Window.__K6_STUDIO_TAB_ID__ global augmentation is declared in both src/recording/globals.d.ts and src/recording/utils.ts. The dedicated .d.ts file becomes redundant since utils.ts already contains the same declare global block alongside the code that uses it.
Additional Locations (1)
Reviewed by Cursor Bugbot for commit 77e6f78. Configure here.
2 participants
What
extension/src/cdptosrc/recording.extension/src/frontend/viewtosrc/recording/ui.Why
The app previously supported two browser-event capture mechanisms (extension vs CDP). This change removes the extension path and keeps CDP-only.
Notes
vite.browser.config.mtsnow buildssrc/recording/index.tsintoresources/browser/index.js.BrowserServer/CDP recorder now depend on the moved runtime messaging/types undersrc/recording/*.Walkthrough
remove_extension_cdp_only_smoke_test.mp4
Smoke test: Settings no longer show extension/CDP toggle; Recorder retains “Capture browser events”; record start/stop navigates to previewer; previewer shows Browser events tab and Browser test disabled when no events.
Recorder settings without browser recording method toggle
Recording previewer with browser events tab
Create test menu with browser test disabled when no events
Testing
npm testnpm run typechecknpm run lint(warnings only: import order)To show artifacts inline, enable in settings.