Return a panic inside the decoder as an error, not as the end of the thread
rawler panics on some input rather than returning Err — a DNG whose IFD claims a >50000 px image, which the reference library has: a 521 MB stitched panorama, IMG_4181-Pano.dng. On a worker thread a panic is the end of the thread, so the face sweep that met it stopped thirteen seconds in, three sweeps running on the tablet and three on the desktop, with "17301 image(s) to index" as the last word. FR-RAW-4 says a malformed file must not abort a batch, and that is this crate's promise whatever the library beneath it does: every entry point that calls into rawler now runs under catch_unwind, and a file that panics the decoder is one failed file with the panic's message in the error. Verified on the panorama itself: metadata reads, decode returns the error, the thread survives. The crash hook still records the panic, which is right — it is a defect in a dependency and the record is how it gets reported.
This commit is contained in:
@@ -25,6 +25,42 @@ pub enum DecodeError {
|
||||
CorruptPreview(String),
|
||||
}
|
||||
|
||||
/// Run a decoder call, and return a panic inside it as an error.
|
||||
///
|
||||
/// TRACES: FR-RAW-4 | NFR-SEC-1
|
||||
/// rawler `panic!`s on some malformed input rather than returning `Err` — a
|
||||
/// DNG whose IFD claims a >50000 px image, for one, which is in the reference
|
||||
/// library. A panic on a worker thread ends the thread: the face sweep that
|
||||
/// met that file stopped 13 seconds in, three sweeps running, with "17301
|
||||
/// image(s) to index" as the last word and nothing to say why. FR-RAW-4's
|
||||
/// rule — a malformed file must not abort a batch — is this crate's to keep
|
||||
/// whatever the library beneath it does, so every entry point that calls into
|
||||
/// rawler runs through here, and a file that panics the decoder is one failed
|
||||
/// file like any other.
|
||||
///
|
||||
/// The crash hook still records the panic, because it runs before unwinding
|
||||
/// reaches this frame; that is right — it is a real defect in a dependency
|
||||
/// and the record is how it gets reported upstream — and a repeat is the same
|
||||
/// file being met again rather than a new fault.
|
||||
pub(crate) fn guarded<T>(
|
||||
what: &'static str,
|
||||
f: impl FnOnce() -> Result<T, DecodeError>,
|
||||
) -> Result<T, DecodeError> {
|
||||
match std::panic::catch_unwind(std::panic::AssertUnwindSafe(f)) {
|
||||
Ok(result) => result,
|
||||
Err(payload) => {
|
||||
let msg = payload
|
||||
.downcast_ref::<&str>()
|
||||
.map(|s| s.to_string())
|
||||
.or_else(|| payload.downcast_ref::<String>().cloned())
|
||||
.unwrap_or_else(|| "no message".to_string());
|
||||
Err(DecodeError::Decode(format!(
|
||||
"{what}: the decoder panicked on this file: {msg}"
|
||||
)))
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
impl DecodeError {
|
||||
/// Whether a fallback path might still produce an image.
|
||||
///
|
||||
@@ -49,4 +85,28 @@ mod tests {
|
||||
// A genuinely unsupported file has nowhere to fall through to.
|
||||
assert!(!DecodeError::Unsupported("unknown".into()).has_fallback());
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_panic_in_the_decoder_is_an_error_and_the_thread_survives() {
|
||||
// The property the face sweep relies on: one file that panics rawler
|
||||
// is one failed file, not the end of the pass. The message travels,
|
||||
// because "decode failed" alone sends the reader to the crash log.
|
||||
let err = guarded("decode", || -> Result<(), DecodeError> {
|
||||
panic!("rawler: surely there's no such thing as a {}MP image!", 600)
|
||||
})
|
||||
.unwrap_err();
|
||||
let text = err.to_string();
|
||||
assert!(text.contains("panicked"), "{text}");
|
||||
assert!(text.contains("600MP"), "{text}");
|
||||
assert!(!err.has_fallback(), "a panic is not a missing preview");
|
||||
}
|
||||
|
||||
#[test]
|
||||
fn a_result_passes_through_untouched() {
|
||||
assert_eq!(guarded("decode", || Ok::<_, DecodeError>(7)).unwrap(), 7);
|
||||
assert!(matches!(
|
||||
guarded("decode", || Err::<(), _>(DecodeError::NoPreview)),
|
||||
Err(DecodeError::NoPreview)
|
||||
));
|
||||
}
|
||||
}
|
||||
|
||||
@@ -285,6 +285,10 @@ pub fn probe(header: &[u8]) -> Option<Format> {
|
||||
/// TRACES: FR-CAT-5 | M-12
|
||||
/// Read capture metadata without decoding sensor data.
|
||||
pub fn metadata(bytes: &[u8]) -> Result<Metadata, DecodeError> {
|
||||
error::guarded("metadata", || metadata_unguarded(bytes))
|
||||
}
|
||||
|
||||
fn metadata_unguarded(bytes: &[u8]) -> Result<Metadata, DecodeError> {
|
||||
use rawler::rawsource::RawSource;
|
||||
|
||||
// rawler has no decoder for a plain JPEG, so without this every JPEG in a
|
||||
@@ -510,6 +514,10 @@ pub(crate) fn parse_exif_offset(s: &str) -> Option<i32> {
|
||||
/// Only develop and export should call it; culling and the grid must not
|
||||
/// (FR-CULL-1).
|
||||
pub fn decode(bytes: &[u8]) -> Result<RawImage, DecodeError> {
|
||||
error::guarded("decode", || decode_unguarded(bytes))
|
||||
}
|
||||
|
||||
fn decode_unguarded(bytes: &[u8]) -> Result<RawImage, DecodeError> {
|
||||
use rawler::rawsource::RawSource;
|
||||
|
||||
let source = RawSource::new_from_slice(bytes);
|
||||
|
||||
@@ -158,6 +158,10 @@ pub enum PreviewSize {
|
||||
/// Returns [`DecodeError::NoPreview`] where there is none at all: a
|
||||
/// fall-through signal, not a failure (see [`DecodeError::has_fallback`]).
|
||||
pub fn extract_preview(bytes: &[u8], size: PreviewSize) -> Result<Preview, DecodeError> {
|
||||
crate::error::guarded("preview", || extract_preview_unguarded(bytes, size))
|
||||
}
|
||||
|
||||
fn extract_preview_unguarded(bytes: &[u8], size: PreviewSize) -> Result<Preview, DecodeError> {
|
||||
use rawler::rawsource::RawSource;
|
||||
|
||||
// A plain JPEG *is* its own preview — rawler has no decoder for one, and
|
||||
|
||||
File diff suppressed because one or more lines are too long
Reference in New Issue
Block a user