Closes#203. Changes a message the user can see, per the decision on that ticket.
The divergence nothing could see
FFmpegEngine and ConcatEngine each carried their own copy of the same when, twelve lines apart in two files — and the copies had drifted:
engine
failure message
FFmpegEngine
prefers getFailStackTrace(), falls back to the log tail
ConcatEngine
only ever reads the log tail
Neither was tested, because both live inside a callback handed to FFmpegKit, which does not run on the JVM. So the disagreement was invisible.
sessionOutcome() now holds the rule; each engine maps Success/Cancelled/Failed onto its continuation. JVM-safe, verified: javap over the committed AAR shows ReturnCode(int) as a plain public constructor with pure static isSuccess/isCancel and a <clinit> that loads no native library.
The decision, applied
Unified on the stack trace, so a join failure now carries the diagnostics a conversion failure always did.
The prefix stays per-engine. Unifying the strategy must not unify the sentence — a join reporting "FFmpeg failed" would be a worse message than the one it replaces — and there is a test for exactly that.
Why the message sources are lambdas
getFailStackTrace and getAllLogsAsString are calls onto a native session, and only the failure arm needs either. Taking them by value would put both on the happy path of every successful conversion, which the shape this replaces did not — it read them inside the else branch.
Same reasoning as capabilitiesFrom taking a Sequence in #194: a seam should not change what runs when. There is a test that counts the reads, and the eager mutation reddens it.
A null return code is a real input rather than a defensive one — getReturnCode() is nullable and a session killed before reporting has none — so it fails, with null where the number would be.
Nothing asserted the old join text
Re-run immediately before committing, as the ticket asked:
The join legs are worth running locally (tools/local-emulator/run-e2e.sh) since this touches a real failure path only the device exercises end to end — noting that I have not done so.
Closes #203. **Changes a message the user can see**, per the decision on that ticket.
## The divergence nothing could see
`FFmpegEngine` and `ConcatEngine` each carried their own copy of the same `when`, twelve lines apart in two files — and the copies had drifted:
| engine | failure message |
|---|---|
| `FFmpegEngine` | prefers `getFailStackTrace()`, falls back to the log tail |
| `ConcatEngine` | only ever reads the log tail |
Neither was tested, because both live inside a callback handed to `FFmpegKit`, which does not run on the JVM. So the disagreement was invisible.
`sessionOutcome()` now holds the rule; each engine maps `Success`/`Cancelled`/`Failed` onto its continuation. **JVM-safe, verified**: `javap` over the committed AAR shows `ReturnCode(int)` as a plain public constructor with pure static `isSuccess`/`isCancel` and a `<clinit>` that loads no native library.
## The decision, applied
Unified on the stack trace, so a **join failure now carries the diagnostics a conversion failure always did**.
The *prefix* stays per-engine. Unifying the strategy must not unify the sentence — a join reporting "FFmpeg failed" would be a worse message than the one it replaces — and there is a test for exactly that.
## Why the message sources are lambdas
`getFailStackTrace` and `getAllLogsAsString` are calls onto a native session, and only the failure arm needs either. Taking them **by value** would put both on the happy path of every successful conversion, which the shape this replaces did not — it read them inside the `else` branch.
Same reasoning as `capabilitiesFrom` taking a `Sequence` in #194: **a seam should not change what runs when.** There is a test that counts the reads, and the eager mutation reddens it.
A null return code is a real input rather than a defensive one — `getReturnCode()` is nullable and a session killed before reporting has none — so it fails, with `null` where the number would be.
## Nothing asserted the old join text
Re-run immediately before committing, as the ticket asked:
```
$ grep -rn 'Joining failed\|FFmpeg failed' app/src/
```
returns only `main`, plus `ConcatWorker.GENERIC_FAILURE_MESSAGE` — a **different** constant this does not touch.
## Acceptance: mutations run and restored
| mutation | red |
|---|---|
| swap the `ifBlank` operands | 1 |
| treat cancellation as a failure | 2 |
| read both message sources eagerly | 1 |
| hardcode the prefix | 1 |
## Verification
`assembleDebug` + `testDebugUnitTest` (full suite) + `compileDebugAndroidTestKotlin` + `ktlintCheck` + `detekt` + `lintDebug` — green. One ktlint complaint fixed with `ktlintFormat`.
The join legs are worth running locally (`tools/local-emulator/run-e2e.sh`) since this touches a real failure path only the device exercises end to end — noting that I have not done so.
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Blocking a user prevents them from interacting with repositories, such as opening or commenting on pull requests or issues. Learn more about blocking a user.
Closes #203. Changes a message the user can see, per the decision on that ticket.
The divergence nothing could see
FFmpegEngineandConcatEngineeach carried their own copy of the samewhen, twelve lines apart in two files — and the copies had drifted:FFmpegEnginegetFailStackTrace(), falls back to the log tailConcatEngineNeither was tested, because both live inside a callback handed to
FFmpegKit, which does not run on the JVM. So the disagreement was invisible.sessionOutcome()now holds the rule; each engine mapsSuccess/Cancelled/Failedonto its continuation. JVM-safe, verified:javapover the committed AAR showsReturnCode(int)as a plain public constructor with pure staticisSuccess/isCanceland a<clinit>that loads no native library.The decision, applied
Unified on the stack trace, so a join failure now carries the diagnostics a conversion failure always did.
The prefix stays per-engine. Unifying the strategy must not unify the sentence — a join reporting "FFmpeg failed" would be a worse message than the one it replaces — and there is a test for exactly that.
Why the message sources are lambdas
getFailStackTraceandgetAllLogsAsStringare calls onto a native session, and only the failure arm needs either. Taking them by value would put both on the happy path of every successful conversion, which the shape this replaces did not — it read them inside theelsebranch.Same reasoning as
capabilitiesFromtaking aSequencein #194: a seam should not change what runs when. There is a test that counts the reads, and the eager mutation reddens it.A null return code is a real input rather than a defensive one —
getReturnCode()is nullable and a session killed before reporting has none — so it fails, withnullwhere the number would be.Nothing asserted the old join text
Re-run immediately before committing, as the ticket asked:
returns only
main, plusConcatWorker.GENERIC_FAILURE_MESSAGE— a different constant this does not touch.Acceptance: mutations run and restored
ifBlankoperandsVerification
assembleDebug+testDebugUnitTest(full suite) +compileDebugAndroidTestKotlin+ktlintCheck+detekt+lintDebug— green. One ktlint complaint fixed withktlintFormat.The join legs are worth running locally (
tools/local-emulator/run-e2e.sh) since this touches a real failure path only the device exercises end to end — noting that I have not done so.🤖 Generated with Claude Code