refactor: simplify screen recording code

- Fix vite output path (was broken lib/background.mjs, now background.js) - Import recording types from protocol.ts instead of duplicating in background.ts - Remove dead getSessionId() and unused sessionId params from screen-recording.ts - Replace 4 executor wrapper functions with single withRecordingDefaults helper - Fix binary chunk routing with lastRecordingMetadataTabId tracking - Add ExtensionStopRecordingResult type for proper extension/relay separation Saves ~56 lines while improving correctness of multi-tab recording.
2026-01-24 15:48:52 +01:00
parent 3b5427ebe8
commit 8449006e59
6 changed files with 64 additions and 116 deletions
@@ -3,7 +3,7 @@ declare const process: { env: { PLAYWRITER_PORT: string } }
 import { createStore } from 'zustand/vanilla'
 import type { ExtensionState, ConnectionState, TabState, TabInfo, RecordingInfo } from './types'
 import type { CDPEvent, Protocol } from 'playwriter/src/cdp-types'
-import type { ExtensionCommandMessage, ExtensionResponseMessage } from 'playwriter/src/protocol'
+import type { ExtensionCommandMessage, ExtensionResponseMessage, StartRecordingParams, StopRecordingParams, IsRecordingParams, CancelRecordingParams, StartRecordingResult, ExtensionStopRecordingResult, IsRecordingResult, CancelRecordingResult } from 'playwriter/src/protocol'

 const RELAY_PORT = process.env.PLAYWRITER_PORT
 const RELAY_URL = `ws://127.0.0.1:${RELAY_PORT}/extension`
@@ -1089,15 +1089,7 @@ function resolveTabIdFromSessionId(sessionId?: string): number | undefined {
  return found?.tabId
 }

-interface StartRecordingParams {
-  sessionId?: string
-  frameRate?: number
-  audio?: boolean
-  videoBitsPerSecond?: number
-  audioBitsPerSecond?: number
-}
-
-async function handleStartRecording(params: StartRecordingParams): Promise<{ success: true; tabId: number; startedAt: number } | { success: false; error: string }> {
+async function handleStartRecording(params: StartRecordingParams): Promise<StartRecordingResult> {
  const tabId = resolveTabIdFromSessionId(params.sessionId)
  if (!tabId) {
    return { success: false, error: 'No connected tab found for recording. Click the Playwriter extension icon on the tab you want to record.' }
@@ -1177,11 +1169,7 @@ async function handleStartRecording(params: StartRecordingParams): Promise<{ suc
  }
 }

-interface StopRecordingParams {
-  sessionId?: string
-}
-
-async function handleStopRecording(params: StopRecordingParams): Promise<{ success: true; tabId: number; duration: number } | { success: false; error: string }> {
+async function handleStopRecording(params: StopRecordingParams): Promise<ExtensionStopRecordingResult> {
  const tabId = resolveTabIdFromSessionId(params.sessionId)
  if (!tabId) {
    return { success: false, error: 'No connected tab found' }
@@ -1225,11 +1213,7 @@ async function handleStopRecording(params: StopRecordingParams): Promise<{ succe
  }
 }

-interface IsRecordingParams {
-  sessionId?: string
-}
-
-async function handleIsRecording(params: IsRecordingParams): Promise<{ isRecording: boolean; tabId?: number; startedAt?: number }> {
+async function handleIsRecording(params: IsRecordingParams): Promise<IsRecordingResult> {
  const tabId = resolveTabIdFromSessionId(params.sessionId)
  if (!tabId) {
    return { isRecording: false }
@@ -1257,11 +1241,7 @@ async function handleIsRecording(params: IsRecordingParams): Promise<{ isRecordi
  }
 }

-interface CancelRecordingParams {
-  sessionId?: string
-}
-
-async function handleCancelRecording(params: CancelRecordingParams): Promise<{ success: boolean; error?: string }> {
+async function handleCancelRecording(params: CancelRecordingParams): Promise<CancelRecordingResult> {
  const tabId = resolveTabIdFromSessionId(params.sessionId)
  if (!tabId) {
    return { success: false, error: 'No connected tab found' }
@@ -58,12 +58,7 @@ export default defineConfig({
        welcome: resolve(__dirname, 'src/welcome.html'),
      },
      output: {
-        entryFileNames: (chunkInfo) => {
-          if (chunkInfo.name === 'background') {
-            return 'lib/background.mjs';
-          }
-          return '[name].js';
-        },
+        entryFileNames: '[name].js',
        format: 'es',
      },
    },
@@ -100,6 +100,8 @@ export async function startPlayWriterCDPRelayServer({
    resolveStop?: (result: StopRecordingResult) => void
  }
  const activeRecordings = new Map<number, ActiveRecording>()
+  // Track which tabId just sent recordingData metadata - used to route the next binary chunk
+  let lastRecordingMetadataTabId: number | null = null

  const playwrightClients = new Map<string, PlaywrightClient>()
  let extensionWs: WSContext | null = null
@@ -839,12 +841,20 @@ export async function startPlayWriterCDPRelayServer({
        // Handle binary data (recording chunks)
        if (event.data instanceof ArrayBuffer || Buffer.isBuffer(event.data)) {
          const buffer = Buffer.isBuffer(event.data) ? event.data : Buffer.from(event.data)
-          // Find the recording that's waiting for data (the one we just received metadata for)
-          // The extension sends metadata first, then binary
-          for (const recording of activeRecordings.values()) {
-            recording.chunks.push(buffer)
-            logger?.log(pc.blue(`Received recording chunk for tab ${recording.tabId}: ${buffer.length} bytes (total chunks: ${recording.chunks.length})`))
-            break // Only add to the first active recording
+          // Route to the recording that just sent metadata
+          const tabId = lastRecordingMetadataTabId
+          lastRecordingMetadataTabId = null
+          
+          if (tabId !== null) {
+            const recording = activeRecordings.get(tabId)
+            if (recording) {
+              recording.chunks.push(buffer)
+              logger?.log(pc.blue(`Received recording chunk for tab ${tabId}: ${buffer.length} bytes (total chunks: ${recording.chunks.length})`))
+            } else {
+              logger?.log(pc.yellow(`Received recording chunk for unknown tab ${tabId}, ignoring`))
+            }
+          } else {
+            logger?.log(pc.yellow('Received recording chunk without preceding metadata, ignoring'))
          }
          return
        }
@@ -883,6 +893,11 @@ export async function startPlayWriterCDPRelayServer({
          const { tabId, final } = (message as any).params
          const recording = activeRecordings.get(tabId)
          
+          // Track which tab sent this metadata for routing the next binary chunk
+          if (!final) {
+            lastRecordingMetadataTabId = tabId
+          }
+          
          if (recording && final) {
            // This is the final marker - write all chunks to file
            try {
@@ -591,36 +591,14 @@ export class PlaywrightExecutor {
      }

      // Screen recording functions (via chrome.tabCapture in extension - survives navigation)
+      // Recording uses chrome.tabCapture which requires activeTab permission.
+      // This permission is granted when the user clicks the Playwriter extension icon on a tab.
      const relayPort = this.cdpConfig.port || 19988
-
-      const startRecordingFn = async (options: {
-        page?: Page
-        frameRate?: number
-        videoBitsPerSecond?: number
-        audioBitsPerSecond?: number
-        audio?: boolean
-        outputPath: string
-      }) => {
-        // Recording uses chrome.tabCapture which requires activeTab permission.
-        // This permission is granted when the user clicks the Playwriter extension icon on a tab.
-        // Recording will work on any tab where the user has clicked the icon.
-        return startRecording({
-          page: options.page || page,
-          relayPort,
-          ...options
-        })
-      }
-
-      const stopRecordingFn = async (options: { page?: Page } = {}) => {
-        return stopRecording({ page: options.page || page, relayPort })
-      }
-
-      const isRecordingFn = async (options: { page?: Page } = {}) => {
-        return isRecording({ page: options.page || page, relayPort })
-      }
-
-      const cancelRecordingFn = async (options: { page?: Page } = {}) => {
-        return cancelRecording({ page: options.page || page, relayPort })
+      // Recording will work on any tab where the user has clicked the icon.
+      const withRecordingDefaults = <T extends { page?: Page }, R>(
+        fn: (opts: T & { relayPort: number }) => Promise<R>
+      ) => {
+        return (options: T = {} as T) => fn({ page: options.page || page, relayPort, ...options })
      }
      const self = this

@@ -642,10 +620,10 @@ export class PlaywrightExecutor {
        formatStylesAsText,
        getReactSource: getReactSourceFn,
        screenshotWithAccessibilityLabels: screenshotWithAccessibilityLabelsFn,
-        startRecording: startRecordingFn,
-        stopRecording: stopRecordingFn,
-        isRecording: isRecordingFn,
-        cancelRecording: cancelRecordingFn,
+        startRecording: withRecordingDefaults(startRecording),
+        stopRecording: withRecordingDefaults(stopRecording),
+        isRecording: withRecordingDefaults(isRecording),
+        cancelRecording: withRecordingDefaults(cancelRecording),
        resetPlaywright: async () => {
          const { page: newPage, context: newContext } = await self.reset()
          vmContextObj.page = newPage
@@ -146,6 +146,17 @@ export type StartRecordingResult = {
  error: string
 }

+/** Result from extension - doesn't include path/size since relay writes the file */
+export type ExtensionStopRecordingResult = {
+  success: true
+  tabId: number
+  duration: number
+} | {
+  success: false
+  error: string
+}
+
+/** Final result from relay - includes path/size after file is written */
 export type StopRecordingResult = {
  success: true
  tabId: number
@@ -3,13 +3,13 @@
 * Recording happens in the extension context, so it survives page navigation.
 * 
 * This module communicates with the relay server which forwards commands to the extension.
+ * 
+ * Note: Recording uses the first connected tab. Multi-tab recording support would require
+ * proper sessionId mapping between Playwright pages and extension tabs.
 */

 import type { Page } from 'playwright-core'
 import type {
-  StartRecordingBody,
-  StopRecordingParams,
-  CancelRecordingParams,
  StartRecordingResult,
  StopRecordingResult,
  IsRecordingResult,
@@ -17,7 +17,7 @@ import type {
 } from './protocol.js'

 export interface StartRecordingOptions {
-  /** Target page to record */
+  /** Target page to record (currently unused - records first connected tab) */
  page: Page
  /** Frame rate (default: 30) */
  frameRate?: number
@@ -34,7 +34,7 @@ export interface StartRecordingOptions {
 }

 export interface StopRecordingOptions {
-  /** Target page that is being recorded */
+  /** Target page that is being recorded (currently unused) */
  page: Page
  /** Relay server port (default: 19988) */
  relayPort?: number
@@ -46,20 +46,12 @@ export interface RecordingState {
  tabId?: number
 }

-function getSessionId(page: Page): string | undefined {
-  // The page's _guid is Playwright-internal and doesn't match the extension's sessionId (pw-tab-X).
-  // For now, we don't pass sessionId and let the extension use the first connected tab.
-  // TODO: Add proper mapping between page and extension tab sessionIds
-  return undefined
-}
-
 /**
 * Start recording the page.
 * The recording is handled by the extension, so it survives page navigation.
 */
 export async function startRecording(options: StartRecordingOptions): Promise<RecordingState> {
  const {
-    page,
    frameRate = 30,
    videoBitsPerSecond = 2500000,
    audioBitsPerSecond = 128000,
@@ -67,20 +59,11 @@ export async function startRecording(options: StartRecordingOptions): Promise<Re
    outputPath,
    relayPort = 19988,
  } = options
-
-  const sessionId = getSessionId(page)
  
  const response = await fetch(`http://127.0.0.1:${relayPort}/recording/start`, {
    method: 'POST',
    headers: { 'Content-Type': 'application/json' },
-    body: JSON.stringify({
-      sessionId,
-      frameRate,
-      videoBitsPerSecond,
-      audioBitsPerSecond,
-      audio,
-      outputPath,
-    } satisfies StartRecordingBody),
+    body: JSON.stringify({ frameRate, videoBitsPerSecond, audioBitsPerSecond, audio, outputPath }),
  })

  const result = await response.json() as StartRecordingResult
@@ -101,14 +84,12 @@ export async function startRecording(options: StartRecordingOptions): Promise<Re
 * Returns the path to the saved video file.
 */
 export async function stopRecording(options: StopRecordingOptions): Promise<{ path: string; duration: number; size: number }> {
-  const { page, relayPort = 19988 } = options
-
-  const sessionId = getSessionId(page)
+  const { relayPort = 19988 } = options

  const response = await fetch(`http://127.0.0.1:${relayPort}/recording/stop`, {
    method: 'POST',
    headers: { 'Content-Type': 'application/json' },
-    body: JSON.stringify({ sessionId } satisfies StopRecordingParams),
+    body: JSON.stringify({}),
  })

  const result = await response.json() as StopRecordingResult
@@ -117,43 +98,31 @@ export async function stopRecording(options: StopRecordingOptions): Promise<{ pa
    throw new Error(`Failed to stop recording: ${result.error}`)
  }

-  return {
-    path: result.path,
-    duration: result.duration,
-    size: result.size,
-  }
+  return { path: result.path, duration: result.duration, size: result.size }
 }

 /**
- * Check if recording is currently active on a page.
+ * Check if recording is currently active.
 */
 export async function isRecording(options: { page: Page; relayPort?: number }): Promise<RecordingState> {
-  const { page, relayPort = 19988 } = options
+  const { relayPort = 19988 } = options

-  const sessionId = getSessionId(page)
-
-  const response = await fetch(`http://127.0.0.1:${relayPort}/recording/status?sessionId=${sessionId || ''}`)
+  const response = await fetch(`http://127.0.0.1:${relayPort}/recording/status`)
  const result = await response.json() as IsRecordingResult

-  return {
-    isRecording: result.isRecording,
-    startedAt: result.startedAt,
-    tabId: result.tabId,
-  }
+  return { isRecording: result.isRecording, startedAt: result.startedAt, tabId: result.tabId }
 }

 /**
 * Cancel recording without saving.
 */
 export async function cancelRecording(options: { page: Page; relayPort?: number }): Promise<void> {
-  const { page, relayPort = 19988 } = options
-
-  const sessionId = getSessionId(page)
+  const { relayPort = 19988 } = options

  const response = await fetch(`http://127.0.0.1:${relayPort}/recording/cancel`, {
    method: 'POST',
    headers: { 'Content-Type': 'application/json' },
-    body: JSON.stringify({ sessionId } satisfies CancelRecordingParams),
+    body: JSON.stringify({}),
  })

  const result = await response.json() as CancelRecordingResult