Keep failing commands, slow tools and untrusted update links from breaking the session - #8
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.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 191e6e5551
ℹ️ 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".
| IsBusy = false; | ||
| SetProcessingPhase("Phase_Error"); | ||
| SetStatus("Error_UnexpectedFormat", exception.Message); |
There was a problem hiding this comment.
Reset active queue items when handling command errors
When StartProcessingAsync throws after MarkProcessingStarted or MarkValidationStarted, this handler only clears IsBusy and updates the global status, leaving the selected item in Processing or Validating. BatchQueueService considers only Ready items processable and only Failed/Cancelled items retryable, so the recovered session contains an item that cannot be processed or retried without removing and re-adding it; transition the active item to Failed (and attach the error) as part of command-error recovery.
Useful? React with 👍 / 👎.
Replaces #2, which GitHub closed automatically when its base branch was deleted along with the merge of #1. Same four commits, rebased onto the current
main.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.The
ProcessRunnerconflict with #1Both branches bounded the output drain. 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 cancellation#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
PRs #3 through #7 remain stacked on this branch and were rebased along with it.