D14: picking a file died instead of reporting an unreadable one when
FFmpegKit's native library could not load. `probeWithFFprobe` guarded its
call with `catch (e: Exception)`, and `ConversionViewModel.onInputPicked`
guarded nothing, so the failure escaped a `viewModelScope.launch` -- which
has no handler, and on a device ends the process.
All three of the obvious narrow guards catch nothing, which is why this
needed reading the AAR rather than guessing. `NativeLoader.loadLibrary`
catches the `UnsatisfiedLinkError` that `System.loadLibrary` raises and
rethrows a *bare* `java.lang.Error` wrapping it, so `UnsatisfiedLinkError`
never escapes and the escaping type carries no information at all. Every
touch of the class after the first is a different type again --
`NoClassDefFoundError` -- so a guard written for the first shape lets the
second pick onwards crash, which is the harder half to notice. Both are in
the test output verbatim.
`catch (Throwable)` was the wrong answer for the reason the audit gave: it
would swallow a genuine `OutOfMemoryError` in a method that spawns a native
process, turning "this device is out of memory" into "this file looks
unreadable" and letting the app act on it. So the line is drawn by a named
predicate, `isNativeLoadFailure`, rather than by the catch clause -- every
class-loading shape is a `LinkageError`, and nothing that means the JVM is
failing is one. That disjointness is what makes the guard narrow.
This is consistent with the position `config/detekt/detekt.yml` already
takes for `TooGenericExceptionCaught`: the boundary's failure types are
undocumented, so guessing crashes the app on a file it could have reported.
One level up the opposite mistake is available too, and the predicate is
what lets both be avoided at once.
`TooGenericExceptionThrown` is relaxed for the test source sets only. A test
that reproduces a failed native load has to throw what the library throws,
and a tidier subclass would leave it passing against a defect it no longer
reproduces. Main source is untouched by that and throws nothing generic.
`ConversionDependencies.probe`'s KDoc is rewritten rather than left. It said
this hazard was "deliberately not fixed here ... its own commit, with its
own test", which this is -- leaving it would have replaced one true comment
with a false one, which is the same defect class as the D11 work.
Both halves are covered independently: reverting the `MediaProbe` catch
reds only the two `MediaProbeNativeLoadTest` cases, reverting the ViewModel
guard reds only the two injected-seam cases, and widening the ViewModel
guard to `Throwable` reds the OutOfMemoryError case -- so the narrowness is
pinned, not just the catch.
Audited the sibling boundaries named in the audit and left all three alone:
`FFmpegEngine` and `ConcatEngine` both construct and run under
`catch (e: Throwable)` in their workers, and `Media3Engine` has no native
loader of this kind and already routes failures through `runCatching`.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
JMR-devandClaude Opus 5
97fdc49f01