fix(macos): stop the take when the writer dies mid-capture, and tell the user

electron/ipc/handlers.ts

@@ -93,6 +93,7 @@ import {
 import { patchWebmDurationOnDisk } from "../recording/webm-duration";
 import { reindexRecordingOnDisk } from "../recording/webm-seek-index";
 import { registerNativeBridgeHandlers } from "./nativeBridge";
+import { createNativeMacMidCaptureErrorWatch } from "./nativeMacMidCaptureErrorWatch";
 import { registerRecordingPrefsHandlers } from "./recordingPrefs";
 import { RecordingStreamRegistry, registerRecordingStreamHandlers } from "./recordingStream";
 
@@ -1485,8 +1486,17 @@ function inspectNativeMacCaptureOutput() {
 	}
 }
 
-function attachNativeMacCaptureOutputDrain(proc: ChildProcessWithoutNullStreams) {
+function attachNativeMacCaptureOutputDrain(
+	proc: ChildProcessWithoutNullStreams,
+	onErrorDuringCapture: () => void,
+) {
 	let lineBuffer = "";
+	// Hooked here rather than on `nativeMacCaptureEvents`, which the stop wait
+	// replays from the buffer: the drain sees each line once, live.
+	const watchForMidCaptureError = createNativeMacMidCaptureErrorWatch(
+		() => nativeMacCaptureProcess === proc,
+		onErrorDuringCapture,
+	);
 	const drain = (chunk: Buffer) => {
 		const text = chunk.toString();
 		nativeMacCaptureOutput += text;
@@ -1497,6 +1507,7 @@ function attachNativeMacCaptureOutputDrain(proc: ChildProcessWithoutNullStreams)
 			const event = tryParseNativeHelperEvent(line.trim());
 			if (event) {
 				dispatchNativeMacHelperEvent(event);
+				watchForMidCaptureError(event);
 			}
 		}
 	};
@@ -2825,7 +2836,14 @@ export function registerIpcHandlers(
 				stdio: ["pipe", "pipe", "pipe"],
 			});
 			nativeMacCaptureProcess = proc;
-			attachNativeMacCaptureOutputDrain(proc);
+			// Drives the renderer's own stop, the same one the tray's Stop Recording
+			// sends: it clears the HUD and surfaces the helper's error as the result.
+			attachNativeMacCaptureOutputDrain(proc, () => {
+				const hudWindow = getMainWindow();
+				if (hudWindow && !hudWindow.isDestroyed()) {
+					hudWindow.webContents.send("stop-recording-from-tray");
+				}
+			});
 
 			await waitForNativeMacCaptureStart(proc);
 			const captureStartedAtMs = Date.now();

electron/ipc/nativeMacMidCaptureErrorWatch.test.ts

@@ -0,0 +1,49 @@
+import { describe, expect, it, vi } from "vitest";
+import { createNativeMacMidCaptureErrorWatch } from "./nativeMacMidCaptureErrorWatch";
+
+describe("createNativeMacMidCaptureErrorWatch", () => {
+	const error = { event: "error", code: "writer-failed-during-capture" };
+
+	it("fires on an error raised after recording started", () => {
+		const onError = vi.fn();
+		const watch = createNativeMacMidCaptureErrorWatch(() => true, onError);
+
+		watch({ event: "ready" });
+		watch({ event: "recording-started" });
+		watch(error);
+
+		expect(onError).toHaveBeenCalledTimes(1);
+	});
+
+	it("leaves an error raised before recording started to the start wait", () => {
+		const onError = vi.fn();
+		const watch = createNativeMacMidCaptureErrorWatch(() => true, onError);
+
+		watch(error);
+
+		expect(onError).not.toHaveBeenCalled();
+	});
+
+	it("ignores events that are not errors", () => {
+		const onError = vi.fn();
+		const watch = createNativeMacMidCaptureErrorWatch(() => true, onError);
+
+		watch({ event: "recording-started" });
+		watch({ event: "warning", code: "stop-capture-failed" });
+		watch({ event: "recording-stopped" });
+
+		expect(onError).not.toHaveBeenCalled();
+	});
+
+	it("ignores a helper that is no longer the current process", () => {
+		const onError = vi.fn();
+		let current = true;
+		const watch = createNativeMacMidCaptureErrorWatch(() => current, onError);
+
+		watch({ event: "recording-started" });
+		current = false;
+		watch(error);
+
+		expect(onError).not.toHaveBeenCalled();
+	});
+});

electron/ipc/nativeMacMidCaptureErrorWatch.ts

@@ -0,0 +1,24 @@
+/**
+ * Decides, one live helper event at a time, when the macOS helper raised an
+ * error mid-take.
+ *
+ * The start and stop waits only listen while they are pending, so an error the
+ * helper raises between them (a dead writer, a stream that stopped) used to sit
+ * in the buffer until the user pressed stop, while the HUD counted on for
+ * minutes (issue #621). Before `recording-started` the start wait owns the
+ * error, and a helper that is no longer the current process has nobody to stop.
+ */
+export function createNativeMacMidCaptureErrorWatch(
+	isCurrentProcess: () => boolean,
+	onError: () => void,
+) {
+	let recordingStarted = false;
+	return (event: Record<string, unknown>) => {
+		if (event.event === "recording-started") {
+			recordingStarted = true;
+		}
+		if (event.event === "error" && recordingStarted && isCurrentProcess()) {
+			onError();
+		}
+	};
+}

electron/native/screencapturekit/Sources/OpenScreenScreenCaptureKitHelper/ScreenCaptureRecorder.swift

@@ -130,7 +130,11 @@ final class ScreenCaptureRecorder: NSObject, SCStreamOutput, SCStreamDelegate {
 	private var didStartWriting = false
 	private var didEmitRecordingStarted = false
 	private var didReportWriterFailure = false
-	private var isStopping = false
+	/// The one shutdown, however many callers ask for it. A writer failure starts it on
+	/// its own task, and the `stop` Electron sends right after has to wait for that run:
+	/// the command loop exits the process as soon as `stop()` returns, which would cut
+	/// `finishWriter()` off before its terminal event.
+	private var shutdownTask: Task<Void, Never>?
 	private var isPaused = false
 	private var pauseStartedAt: CMTime?
 	private var totalPausedDuration = CMTime.zero
@@ -182,17 +186,20 @@ final class ScreenCaptureRecorder: NSObject, SCStreamOutput, SCStreamDelegate {
 	}
 
 	func stop() async {
-		let shouldStop = stateQueue.sync {
-			if isStopping {
-				return false
+		let task = stateQueue.sync { () -> Task<Void, Never> in
+			if let shutdownTask {
+				return shutdownTask
 			}
-			isStopping = true
-			return true
-		}
-		if !shouldStop {
-			return
+			let task = Task {
+				await self.performStop()
+			}
+			shutdownTask = task
+			return task
 		}
+		await task.value
+	}
 
+	private func performStop() async {
 		do {
 			try await stream?.stopCapture()
 		} catch {
@@ -208,7 +215,7 @@ final class ScreenCaptureRecorder: NSObject, SCStreamOutput, SCStreamDelegate {
 
 	func pause() {
 		let didPause = stateQueue.sync {
-			if isStopping || isPaused {
+			if shutdownTask != nil || isPaused {
 				return false
 			}
 
@@ -227,7 +234,7 @@ final class ScreenCaptureRecorder: NSObject, SCStreamOutput, SCStreamDelegate {
 
 	func resume() {
 		let didResume = stateQueue.sync {
-			if isStopping || !isPaused {
+			if shutdownTask != nil || !isPaused {
 				return false
 			}
 
@@ -297,6 +304,13 @@ final class ScreenCaptureRecorder: NSObject, SCStreamOutput, SCStreamDelegate {
 			audioMixer?.beginTimeline(at: presentationTime)
 			startAudioTicker()
 		}
+		// Checked before the readiness gate, not only at a false append: a failed
+		// writer is not guaranteed to call itself ready, and an append that is never
+		// attempted can never report the failure (issue #621).
+		if writer.status == .failed {
+			reportWriterFailure("writer status")
+			return
+		}
 
 		if videoInput.isReadyForMoreMediaData {
 			let appended = videoInput.append(sampleBuffer)
@@ -330,17 +344,27 @@ final class ScreenCaptureRecorder: NSObject, SCStreamOutput, SCStreamDelegate {
 	/// settles and every failure becomes the "Saving..." hang instead of an error.
 	/// This event answers "when did the writer die"; that one answers "did stopping
 	/// work". Two questions, two codes.
+	///
+	/// It also ends the capture, the way `didStopWithError` does. A failed writer
+	/// never recovers, so every frame after it is dropped; issue #621 is a take
+	/// whose writer died at 75 s while capture ran on for 22 more minutes. The
+	/// process stays up and still answers `stop`, so the Electron stop path works
+	/// unchanged and reads this event back as the reason.
 	private func reportWriterFailure(_ stage: String) {
 		guard !didReportWriterFailure, let writer else {
 			return
 		}
 		didReportWriterFailure = true
 		emitError(
 			code: "writer-failed-during-capture",
-			message: "\(stage): "
+			message: "Recording stopped: the video file could not be written (\(stage): "
 				+ (writer.error.map { "\($0)" }
-					?? "AVAssetWriter status \(writer.status.rawValue)"),
+					?? "AVAssetWriter status \(writer.status.rawValue)")
+				+ ").",
 		)
+		Task {
+			await stop()
+		}
 	}
 
 	private func ensureRequestedPermissions() throws {
@@ -604,7 +628,16 @@ final class ScreenCaptureRecorder: NSObject, SCStreamOutput, SCStreamDelegate {
 			leeway: .milliseconds(5)
 		)
 		timer.setEventHandler { [weak self] in
-			self?.audioMixer?.tick()
+			guard let self else {
+				return
+			}
+			// A still screen delivers no complete frames, so the frame path alone
+			// could sit on a dead writer for as long as nothing on screen moves.
+			if self.writer?.status == .failed {
+				self.reportWriterFailure("writer status")
+				return
+			}
+			self.audioMixer?.tick()
 		}
 		audioTicker = timer
 		timer.resume()