Keep failing commands, slow tools and untrusted update links from breaking the session - #2
Conversation
Result validation read the selected export format directly when deciding which sample rate to expect. The archive preset forces FLAC and a stream copy keeps the source format, so a correct export was reported with a sample-rate warning whenever the Premiere profile was still selected. Both cases now resolve the format the same way the processing does, and a stream copy no longer claims a lossy source was written to a lossless format.
Asynchronous commands run as async void, so any exception that escaped them reached the dispatcher handler and shut the app down. They now report through the view model instead: the error is logged and shown as status, and a cancelled command counts as a normal outcome. The most likely trigger was saving the run log after a finished batch, which is now caught separately - the exported files are done at that point, so a read-only or full output folder is only a warning. Writing the crash log is wrapped as well so a failure there cannot throw inside the crash handler.
The startup check ran "<tool> -version" without a time limit, so a binary that never answers (broken download, unreachable network path) kept the tool status pending forever. The probe now stops after 20 seconds, kills the process and reports the tool as unavailable; an outer cancellation kills it too instead of leaving it behind. The process runner waited without a limit for its output readers to close. A child process that inherited the pipes can keep them open, so the wait is bounded: generous on the normal path, where the last lines carry the loudness measurements, and short after a cancellation.
The release link from the API is opened with the shell when the user clicks the update notice, so it is now checked first: only an absolute https URL on github.com is used, anything else falls back to the known releases page. The update check also shares one HttpClient instead of creating a new connection pool per instance.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 3cf2be9432
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| catch (OperationCanceledException) | ||
| { | ||
| // Cancelling a running command is a normal outcome, not a failure. | ||
| } |
There was a problem hiding this comment.
When a user cancels a batch while an uncached FFmpeg availability probe is running, ToolDiscoveryService.GetStatusAsync rethrows OperationCanceledException, but this blanket catch suppresses it before StartProcessingAsync can mark the current item cancelled. Its finally only clears IsBusy, leaving the item in Processing; BatchQueueService then considers it neither processable nor retryable, so the user cannot resume it without removing and re-adding it. Cancellation needs to be translated into the command's normal cancelled state rather than discarded here.
Useful? React with 👍 / 👎.
1 participant
Stacked on #1 — the base retargets to
mainonce #1 is merged, so the diff here is only the four commits of this branch.Fixed
async void, so any exception that escaped one reached the dispatcher handler and ended the session. They now report through the view model: the error is logged and shown as status, and a cancelled command counts as a normal outcome.Changed
<tool> -versionwithout a time limit, so a binary that never answers (broken download, unreachable network path) kept the tool status pending forever. The probe now stops after 20 seconds, kills the process and reports the tool as unavailable.HttpClientinstead of creating a connection pool per instance.Conflict resolution against #1
Both branches bounded the output drain in
ProcessRunner. The merged version keeps the structure from #1 —TerminationWaitTimeout, the linked cancellation source that lets the watchdog end the exit wait, the cancelled/timed-out distinction, the locked buffer snapshot, andCancelOutputRead/CancelErrorReadas the fallback after an unsuccessful drain — and takes the split timeouts from this branch:ReaderDrainTimeout= 10 s on the normal pathCancelDrainTimeout= 2 s after a cancellationThe reason for the longer normal-path value: #1 used 2 s for both. If that drain times out, the last captured lines are dropped — and those carry the
loudnormmeasurements of the first pass. The parser swallows a missing measurement, so a two-pass run would quietly degrade to a single pass. After a cancellation the remaining output is discarded anyway, so 2 s stays right there.Verification