fix(display): surface screen-recording write errors instead of crashing (#289)

recordScreenToFile and recordScreenAndAudioToFiles created their Annex-B
output with createWriteStream and never attached an 'error' listener, so a
stream error reached the process as an uncaught exception instead of a
rejected promise.

Callers cannot defend against that. The event bypasses the promise chain, so
wrapping the call in try/catch does not help and the process still dies:

    try { await recordScreenToFile(service, path); } catch { /* never runs */ }

The likely trigger is ordinary rather than exotic: an output path whose
parent directory does not exist. A full disk or an unplugged volume
mid-recording does the same. Passing end(callback) is not a substitute -- the
callback does receive the error, but 'error' is still emitted separately and
still terminates the process.

The audio half of the same feature already guards against this in
M4aFileWriter, which holds the first error and re-throws it from the next
call. The video half did not.

AnnexBFileWriter gives the video track that same guarantee, so a failed
recording surfaces as a rejection the caller can handle. Two details differ
from M4aFileWriter deliberately:

- It does not await 'open'. Doing so would reject while the capture is
  already streaming, stranding it with no handle to stop it. An open failure
  surfaces from the first write instead.
- close() reports the held error in preference to ending an already-failed
  stream, which answers with a generic ERR_STREAM_DESTROYED and buries the
  cause. That is the common path, since the recorders write nothing until the
  first keyframe arrives.

Behaviour is otherwise unchanged: the same backpressure, the same output
bytes, no public API added.

Author: harsha509

src/services/ios/display/recording/av-capture.ts

@@ -1,14 +1,11 @@
-import {once} from 'node:events';
-import {createWriteStream} from 'node:fs';
-
 import {getLogger} from '../../../../lib/logger.js';
 import {XPCUUID} from '../../../../lib/remote-xpc/xpc-uuid.js';
 import {type AacEldFormat, aacEldDurationMs} from '../audio/aac-eld.js';
 import {AudioStreamCapture, type AudioStreamStats} from '../audio/audio-stream-capture.js';
 import {M4aFileWriter} from '../audio/m4a-writer.js';
 import {type DisplayService, type StartVideoStreamOptions} from '../index.js';
 import {toAnnexB} from '../video/hevc.js';
-import {ScreenStreamCapture, type ScreenStreamStats} from '../video/screen-stream-capture.js';
+import {AnnexBFileWriter, ScreenStreamCapture, type ScreenStreamStats} from '../video/screen-stream-capture.js';
 import {type MuxCommand, ffmpegMuxCommandBuilder} from './mux-command.js';
 
 const log = getLogger('AvCapture');
@@ -127,7 +124,7 @@ export async function recordScreenAndAudioToFiles(
     throw error;
   }
 
-  const videoOut = createWriteStream(videoPath);
+  const videoOut = new AnnexBFileWriter(videoPath);
   const audioOut = await M4aFileWriter.create(audioPath);
   let framesWritten = 0;
   let videoBytes = 0;
@@ -147,9 +144,7 @@ export async function recordScreenAndAudioToFiles(
         sawKeyFrame = true;
       }
       const chunk = toAnnexB(unit.nals);
-      if (!videoOut.write(chunk)) {
-        await once(videoOut, 'drain');
-      }
+      await videoOut.write(chunk);
       framesWritten += 1;
       videoBytes += chunk.length;
     }
@@ -180,15 +175,7 @@ export async function recordScreenAndAudioToFiles(
     });
     await audioCapture.stop().catch((): void => undefined);
     audioWritten = await audioOut.close();
-    await new Promise<void>((resolve, reject) => {
-      videoOut.end((error?: Error | null): void => {
-        if (error) {
-          reject(error);
-          return;
-        }
-        resolve();
-      });
-    });
+    await videoOut.close();
   }
 
   const audioDurationMs = aacEldDurationMs(audioWritten.sampleCount);

src/services/ios/display/video/screen-stream-capture.ts

@@ -191,6 +191,69 @@ export class ScreenStreamCapture {
   }
 }
 
+/**
+ * The Annex-B output file, with the write stream's `'error'` event held instead
+ * of left to reach the process.
+ *
+ * A recording runs unattended for minutes and the stream never escapes the
+ * function that owns it, so a caller cannot attach a listener of its own.
+ * Without one here, a mistyped path or a write that fails after the fact (a full
+ * disk, an unplugged volume) arrives as an uncaught `'error'` event and takes
+ * the process down — `end(callback)` does not help, since the callback receives
+ * the error *and* the event is still emitted. Holding the first error and
+ * re-throwing it from the next `write` or `close` turns that into a rejection
+ * the caller can see, the guarantee {@link M4aFileWriter} already gives the
+ * audio track.
+ *
+ * Opening is deliberately not awaited, unlike `M4aFileWriter.create`: the
+ * capture is already streaming by the time the file is created, so a rejection
+ * before the caller holds the writer would strand it. An open failure surfaces
+ * from the first {@link write} instead.
+ */
+export class AnnexBFileWriter {
+  private readonly stream: ReturnType<typeof createWriteStream>;
+  /** First stream error seen, re-thrown from the next `write` or `close`. */
+  private streamError: Error | undefined;
+
+  /** @param path Destination file path; truncated if it exists. */
+  constructor(path: string) {
+    this.stream = createWriteStream(path);
+    this.stream.on('error', (error: Error) => {
+      this.streamError ??= error;
+    });
+  }
+
+  /**
+   * Appends one chunk, resolving once the stream has room for more so an
+   * `await` per chunk applies backpressure.
+   */
+  async write(chunk: Buffer): Promise<void> {
+    if (this.streamError) {
+      throw this.streamError;
+    }
+    if (!this.stream.write(chunk)) {
+      await once(this.stream, 'drain');
+    }
+    if (this.streamError) {
+      throw this.streamError;
+    }
+  }
+
+  /** Flushes and closes the file, re-throwing any error the stream saw. */
+  async close(): Promise<void> {
+    // The held error is reported in preference to ending an already-failed
+    // stream, which answers with a generic `ERR_STREAM_DESTROYED` and buries the
+    // cause. That is the common path for a bad output path: the open fails
+    // before the first keyframe arrives, so no `write` ever ran to surface it.
+    if (this.streamError) {
+      throw this.streamError;
+    }
+    await new Promise<void>((resolve, reject) => {
+      this.stream.end((error?: Error | null): void => (error ? reject(error) : resolve()));
+    });
+  }
+}
+
 /** Options for {@link recordScreenToFile}. */
 export interface RecordScreenOptions extends ScreenStreamCaptureOptions {
   /** How long to record, in milliseconds. Defaults to 5000. */
@@ -239,7 +302,7 @@ export async function recordScreenToFile(
   const {durationMs = 5000, maxFrames = Number.POSITIVE_INFINITY, ...captureOptions} = options;
 
   const capture = await ScreenStreamCapture.start(service, captureOptions);
-  const output = createWriteStream(outputPath);
+  const output = new AnnexBFileWriter(outputPath);
   let framesWritten = 0;
   let bytesWritten = 0;
   let sawKeyFrame = false;
@@ -262,9 +325,7 @@ export async function recordScreenToFile(
       }
 
       const chunk = toAnnexB(unit.nals);
-      if (!output.write(chunk)) {
-        await once(output, 'drain');
-      }
+      await output.write(chunk);
       framesWritten += 1;
       bytesWritten += chunk.length;
 
@@ -277,15 +338,7 @@ export async function recordScreenToFile(
     await capture.stop().catch((error: unknown) => {
       log.debug(`Failed to stop the media stream cleanly: ${error instanceof Error ? error.message : String(error)}`);
     });
-    await new Promise<void>((resolve, reject) => {
-      output.end((error?: Error | null): void => {
-        if (error) {
-          reject(error);
-          return;
-        }
-        resolve();
-      });
-    });
+    await output.close();
   }
 
   return {

test/unit/display/video/screen-stream-capture.spec.ts

@@ -1,8 +1,14 @@
 import assert from 'node:assert/strict';
-import {describe, it} from 'node:test';
+import {mkdtemp, readFile, rm} from 'node:fs/promises';
+import {tmpdir} from 'node:os';
+import {join} from 'node:path';
+import {after, before, describe, it} from 'node:test';
 
 import type {DisplayService, MediaStreamAnswer} from '../../../../src/services/ios/display/index.js';
-import {ScreenStreamCapture} from '../../../../src/services/ios/display/video/screen-stream-capture.js';
+import {
+  AnnexBFileWriter,
+  ScreenStreamCapture,
+} from '../../../../src/services/ios/display/video/screen-stream-capture.js';
 
 /**
  * Builds a capture wired to a stub service and a stub receiver, bypassing
@@ -107,3 +113,81 @@ describe('ScreenStreamCapture', function () {
     });
   });
 });
+
+describe('AnnexBFileWriter', function () {
+  let directory: string;
+  let counter = 0;
+
+  before(async function () {
+    directory = await mkdtemp(join(tmpdir(), 'annexb-writer-'));
+  });
+
+  after(async function () {
+    await rm(directory, {force: true, recursive: true});
+  });
+
+  const chunk = (byte: number, length: number): Buffer => Buffer.alloc(length, byte);
+  const missingPath = (): string => join(directory, 'no', 'such', 'dir', 'screen.h265');
+
+  it('writes every chunk through to the file', async function () {
+    const path = join(directory, `stream-${counter++}.h265`);
+    const chunks = [chunk(0x11, 40), chunk(0x22, 55), chunk(0x33, 48)];
+
+    const writer = new AnnexBFileWriter(path);
+    for (const part of chunks) {
+      await writer.write(part);
+    }
+    await writer.close();
+
+    assert.deepStrictEqual(await readFile(path), Buffer.concat(chunks));
+  });
+
+  it('writes a file larger than the stream buffer, so backpressure is exercised', async function () {
+    const path = join(directory, `stream-${counter++}.h265`);
+    const frame = chunk(0xab, 256 * 1024);
+    const frames = 40;
+
+    const writer = new AnnexBFileWriter(path);
+    for (let i = 0; i < frames; i++) {
+      await writer.write(frame);
+    }
+    await writer.close();
+
+    const file = await readFile(path);
+    assert.strictEqual(file.length, frame.length * frames);
+    assert.ok(file.every((byte) => byte === 0xab));
+  });
+
+  it('reports a bad output path as a rejection rather than an uncaught error', async function () {
+    // A stream 'error' with no listener is an uncaught exception, which during
+    // an unattended recording would take the whole process down. The open
+    // failure lands asynchronously, so it surfaces from whichever call runs
+    // once it has — a later write, or the close in the recorder's finally.
+    const writer = new AnnexBFileWriter(missingPath());
+
+    await assert.rejects(async () => {
+      await writer.write(chunk(0x11, 40));
+      await writer.close();
+    }, /ENOENT/);
+  });
+
+  it('reports a bad output path from close() when no write ever ran', async function () {
+    // The recorder writes nothing until the first keyframe arrives, so a
+    // capture that is torn down early reaches close() having never written.
+    // close() must still name the real cause, not a generic stream error.
+    const writer = new AnnexBFileWriter(missingPath());
+
+    await assert.rejects(() => writer.close(), /ENOENT/);
+  });
+
+  it('surfaces a write failure through the next call', async function () {
+    const writer = new AnnexBFileWriter(join(directory, `stream-${counter++}.h265`));
+    await writer.write(chunk(0x11, 40));
+
+    // Stand in for a disk filling up mid-recording.
+    (writer as unknown as {streamError: Error}).streamError = new Error('ENOSPC');
+
+    await assert.rejects(() => writer.write(chunk(0x22, 40)), /ENOSPC/);
+    await assert.rejects(() => writer.close(), /ENOSPC/);
+  });
+});