From 4f52a57636d81afdb75aa46a04cfd65a8edc982e Mon Sep 17 00:00:00 2001 From: EtienneLescot Date: Mon, 14 Sep 2026 11:00:43 +0200 Subject: [PATCH 1/2] fix(macos): stop the take when the writer dies mid-capture, and tell the user --- electron/ipc/handlers.ts | 27 ++++++++++++++-- .../ScreenCaptureRecorder.swift | 32 +++++++++++++++++-- 2 files changed, 54 insertions(+), 5 deletions(-) diff --git a/electron/ipc/handlers.ts b/electron/ipc/handlers.ts index 7f837ae19..01d3eef44 100644 --- a/electron/ipc/handlers.ts +++ b/electron/ipc/handlers.ts @@ -1485,8 +1485,12 @@ function inspectNativeMacCaptureOutput() { } } -function attachNativeMacCaptureOutputDrain(proc: ChildProcessWithoutNullStreams) { +function attachNativeMacCaptureOutputDrain( + proc: ChildProcessWithoutNullStreams, + onErrorDuringCapture: () => void, +) { let lineBuffer = ""; + let recordingStarted = false; const drain = (chunk: Buffer) => { const text = chunk.toString(); nativeMacCaptureOutput += text; @@ -1497,6 +1501,18 @@ function attachNativeMacCaptureOutputDrain(proc: ChildProcessWithoutNullStreams) const event = tryParseNativeHelperEvent(line.trim()); if (event) { dispatchNativeMacHelperEvent(event); + if (event.event === "recording-started") { + recordingStarted = true; + } + // The start and stop waits only listen while they are pending, so an + // error the helper raises mid-take (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). Hooked here rather than + // on `nativeMacCaptureEvents`, which the stop wait replays from the + // buffer: this sees each line once, live. + if (event.event === "error" && recordingStarted && nativeMacCaptureProcess === proc) { + onErrorDuringCapture(); + } } } }; @@ -2825,7 +2841,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(); diff --git a/electron/native/screencapturekit/Sources/OpenScreenScreenCaptureKitHelper/ScreenCaptureRecorder.swift b/electron/native/screencapturekit/Sources/OpenScreenScreenCaptureKitHelper/ScreenCaptureRecorder.swift index 00706a9f9..8a6e102c6 100644 --- a/electron/native/screencapturekit/Sources/OpenScreenScreenCaptureKitHelper/ScreenCaptureRecorder.swift +++ b/electron/native/screencapturekit/Sources/OpenScreenScreenCaptureKitHelper/ScreenCaptureRecorder.swift @@ -297,6 +297,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,6 +337,12 @@ 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 @@ -337,10 +350,14 @@ final class ScreenCaptureRecorder: NSObject, SCStreamOutput, SCStreamDelegate { 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 +621,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() From 17c726d0662621da71f065f29b58cb94237e3d41 Mon Sep 17 00:00:00 2001 From: EtienneLescot Date: Mon, 14 Sep 2026 11:25:44 +0200 Subject: [PATCH 2/2] fix(macos): let a second stop wait for the shutdown already running A writer failure now starts stop() on its own task, and Electron sends `stop` right after. The second call returned at once, so the command loop could exit(0) before finishWriter() emitted its terminal event. Every caller now awaits the one shutdown task. Also moves the mid-take error rule out of the drain into its own module, with tests for the cases it must ignore (CodeRabbit on #655). --- electron/ipc/handlers.ts | 21 +++----- .../ipc/nativeMacMidCaptureErrorWatch.test.ts | 49 +++++++++++++++++++ electron/ipc/nativeMacMidCaptureErrorWatch.ts | 24 +++++++++ .../ScreenCaptureRecorder.swift | 29 ++++++----- 4 files changed, 99 insertions(+), 24 deletions(-) create mode 100644 electron/ipc/nativeMacMidCaptureErrorWatch.test.ts create mode 100644 electron/ipc/nativeMacMidCaptureErrorWatch.ts diff --git a/electron/ipc/handlers.ts b/electron/ipc/handlers.ts index 01d3eef44..4e9ba5429 100644 --- a/electron/ipc/handlers.ts +++ b/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"; @@ -1490,7 +1491,12 @@ function attachNativeMacCaptureOutputDrain( onErrorDuringCapture: () => void, ) { let lineBuffer = ""; - let recordingStarted = false; + // 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; @@ -1501,18 +1507,7 @@ function attachNativeMacCaptureOutputDrain( const event = tryParseNativeHelperEvent(line.trim()); if (event) { dispatchNativeMacHelperEvent(event); - if (event.event === "recording-started") { - recordingStarted = true; - } - // The start and stop waits only listen while they are pending, so an - // error the helper raises mid-take (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). Hooked here rather than - // on `nativeMacCaptureEvents`, which the stop wait replays from the - // buffer: this sees each line once, live. - if (event.event === "error" && recordingStarted && nativeMacCaptureProcess === proc) { - onErrorDuringCapture(); - } + watchForMidCaptureError(event); } } }; diff --git a/electron/ipc/nativeMacMidCaptureErrorWatch.test.ts b/electron/ipc/nativeMacMidCaptureErrorWatch.test.ts new file mode 100644 index 000000000..c22b9c0e9 --- /dev/null +++ b/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(); + }); +}); diff --git a/electron/ipc/nativeMacMidCaptureErrorWatch.ts b/electron/ipc/nativeMacMidCaptureErrorWatch.ts new file mode 100644 index 000000000..4c62f6b7e --- /dev/null +++ b/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) => { + if (event.event === "recording-started") { + recordingStarted = true; + } + if (event.event === "error" && recordingStarted && isCurrentProcess()) { + onError(); + } + }; +} diff --git a/electron/native/screencapturekit/Sources/OpenScreenScreenCaptureKitHelper/ScreenCaptureRecorder.swift b/electron/native/screencapturekit/Sources/OpenScreenScreenCaptureKitHelper/ScreenCaptureRecorder.swift index 8a6e102c6..9059bac92 100644 --- a/electron/native/screencapturekit/Sources/OpenScreenScreenCaptureKitHelper/ScreenCaptureRecorder.swift +++ b/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? 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 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 }