Skip to content

Commit

Permalink
webpsan: make detail in errors optional
Browse files Browse the repository at this point in the history
Error stacks add a significant amount of code size, and we want to let users opt-out of that.
  • Loading branch information
jessa0 committed Oct 19, 2023
1 parent 314c878 commit d2bfcda
Show file tree
Hide file tree
Showing 4 changed files with 97 additions and 27 deletions.
103 changes: 78 additions & 25 deletions common/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ use derive_more::Display;

/// Error type returned by `mediasan`.
#[derive(Debug, thiserror::Error)]
pub enum Error<E: Display> {
pub enum Error<E: ReportableError> {
/// An IO error occurred while reading the given input.
#[error("IO error: {0}")]
Io(#[from] io::Error),
Expand All @@ -32,10 +32,10 @@ pub enum Error<E: Display> {
/// retrieved e.g. for matching against with [`get_ref`](Self::get_ref) or [`into_inner`](Self::into_inner).
#[derive(thiserror::Error)]
#[error("{error}")]
pub struct Report<E> {
pub struct Report<E: ReportableError> {
#[source]
error: E,
inner: Box<ReportInner>,
stack: E::Stack,
}

/// A [`Display`]-able indicating there was extra trailing input after parsing.
Expand All @@ -48,10 +48,6 @@ pub struct ExtraUnparsedInput;
#[display(fmt = "while parsing value of type `{}`", _0)]
pub struct WhileParsingType(&'static str);

//
// private types
//

/// A convenience type alias for a [`Result`](std::result::Result) containing an error wrapped by a [`Report`].
pub type Result<T, E> = StdResult<T, Report<E>>;

Expand All @@ -66,16 +62,38 @@ pub trait ResultExt: Sized {
fn while_parsing_type(self) -> Self;
}

struct ReportInner {
/// An error stack.
pub struct ReportStack {
location: &'static Location<'static>,
stack: ReportStack,
entries: Vec<ReportEntry>,
}

#[derive(Default)]
struct ReportStack {
entries: Vec<ReportEntry>,
/// A null error stack which ignores all data attached to it.
#[derive(Clone, Copy, Debug, Display)]
#[display(fmt = "")]
pub struct NullReportStack;

/// A trait for error types which can be used in a [`Report`].
pub trait ReportableError: Display {
/// The error stack type corresponding to this error.
type Stack: ReportableErrorStack;
}

/// A trait for error stack types for use within a [`Report`].
pub trait ReportableErrorStack: Display {
#[track_caller]
/// Construct a new instance of [`Self`].
fn new() -> Self;

#[track_caller]
/// Attach a [`Display`]-able type to the error [`Report`]'s stack trace.
fn attach_printable<P: Display + Send + Sync + 'static>(self, printable: P) -> Self;
}

//
// private types
//

#[derive(derive_more::Display)]
#[display(fmt = "{message} at {location}")]
struct ReportEntry {
Expand All @@ -87,7 +105,7 @@ struct ReportEntry {
// Report impls
//

impl<E> Report<E> {
impl<E: ReportableError> Report<E> {
/// Get a reference to the underlying error.
pub fn get_ref(&self) -> &E {
&self.error
Expand All @@ -101,27 +119,29 @@ impl<E> Report<E> {
#[track_caller]
/// Attach a [`Display`]-able type to the stack trace.
pub fn attach_printable<P: Display + Send + Sync + 'static>(mut self, message: P) -> Self {
let entry = ReportEntry { message: Box::new(message), location: Location::caller() };
self.inner.stack.entries.push(entry);
self.stack = self.stack.attach_printable(message);
self
}
}

impl<E> From<E> for Report<E> {
impl<E: ReportableError> From<E> for Report<E> {
#[track_caller]
fn from(error: E) -> Self {
Self { error, inner: Box::new(ReportInner { location: Location::caller(), stack: Default::default() }) }
Self { error, stack: E::Stack::new() }
}
}

impl<E: Display> Debug for Report<E> {
impl<E: ReportableError> Debug for Report<E> {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
let Self { error, inner } = self;
let ReportInner { location, stack } = &**inner;
write!(f, "{error} at {location}\n{stack}")
let Self { error, stack } = self;
write!(f, "{error}{stack}")
}
}

//
// ReportErrorStack impls
//

//
// WhileParsingType impls
//
Expand All @@ -139,21 +159,50 @@ impl WhileParsingType {

impl Display for ReportStack {
fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result {
for entry in &self.entries[..self.entries.len().saturating_sub(1)] {
let Self { location, entries } = self;
writeln!(f, " at {location}")?;
for entry in &entries[..self.entries.len().saturating_sub(1)] {
writeln!(f, " - {entry}")?;
}
if let Some(entry) = self.entries.last() {
if let Some(entry) = entries.last() {
write!(f, " - {entry}")?;
}
Ok(())
}
}

impl ReportableErrorStack for ReportStack {
#[track_caller]
fn new() -> Self {
Self { location: Location::caller(), entries: Default::default() }
}

fn attach_printable<P: Display + Send + Sync + 'static>(mut self, printable: P) -> Self {
let entry = ReportEntry { message: Box::new(printable), location: Location::caller() };
self.entries.push(entry);
self
}
}

//
// ReportableErrorStack impls
//

impl ReportableErrorStack for NullReportStack {
fn new() -> Self {
Self
}

fn attach_printable<P: Display + Send + Sync + 'static>(self, _printable: P) -> Self {
Self
}
}

//
// ResultExt impls
//

impl<T, E> ResultExt for Result<T, E> {
impl<T, E: ReportableError> ResultExt for Result<T, E> {
#[track_caller]
fn attach_printable<P: Display + Send + Sync + 'static>(self, printable: P) -> Self {
match self {
Expand All @@ -168,7 +217,7 @@ impl<T, E> ResultExt for Result<T, E> {
}
}

impl<T, E: Display> ResultExt for StdResult<T, Error<E>> {
impl<T, E: ReportableError> ResultExt for StdResult<T, Error<E>> {
#[track_caller]
fn attach_printable<P: Display + Send + Sync + 'static>(self, printable: P) -> Self {
match self {
Expand All @@ -195,6 +244,10 @@ mod test {
#[error("{}", TEST_ERROR_DISPLAY)]
struct TestError;

impl ReportableError for TestError {
type Stack = ReportStack;
}

fn test_report() -> Report<TestError> {
report_attach!(TestError, TEST_ATTACHMENT)
}
Expand Down
7 changes: 6 additions & 1 deletion mp4san/src/parse/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
use std::fmt::{Debug, Display};

use derive_more::Display;
use mediasan_common::error::{ReportStack, ReportableError};

use crate::error::{Result, ResultExt};

Expand Down Expand Up @@ -95,4 +96,8 @@ pub(crate) struct WhileParsingChild(pub(crate) BoxType, pub(crate) BoxType);
#[display(fmt = "where `{} = {}`", _0, _1)]
pub(crate) struct WhereEq<T, U>(pub(crate) T, pub(crate) U);

impl<T, E> ParseResultExt for Result<T, E> {}
impl ReportableError for ParseError {
type Stack = ReportStack;
}

impl<T> ParseResultExt for Result<T, ParseError> {}
4 changes: 4 additions & 0 deletions webpsan/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,10 @@ keywords = ["webp", "sanitizer", "images", "media"]
readme = "README.md"
exclude.workspace = true

[features]
default = ["error-detail"]
error-detail = []

[dependencies]
assert_matches = "1.5.0"
bitflags = "2.4.0"
Expand Down
10 changes: 9 additions & 1 deletion webpsan/src/parse/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
use std::fmt::{Debug, Display};

use derive_more::Display;
use mediasan_common::error::ReportableError;
use mediasan_common::parse::FourCC;
use mediasan_common::{Result, ResultExt};

Expand Down Expand Up @@ -72,4 +73,11 @@ pub(crate) struct WhileParsingChunk(pub(crate) FourCC);
#[display(fmt = "while parsing `{}` chunk field `{}`", _0, _1)]
pub(crate) struct WhileParsingField<T>(pub(crate) FourCC, pub(crate) T);

impl<T, E> ParseResultExt for Result<T, E> {}
impl ReportableError for ParseError {
#[cfg(feature = "error-detail")]
type Stack = mediasan_common::error::ReportStack;
#[cfg(not(feature = "error-detail"))]
type Stack = mediasan_common::error::NullReportStack;
}

impl<T> ParseResultExt for Result<T, ParseError> {}

0 comments on commit d2bfcda

Please sign in to comment.