Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support additional ways of specifying the error source or backtrace #10

Closed
shepmaster opened this issue Feb 2, 2019 · 4 comments
Closed
Labels
enhancement New feature or request good first issue Good for newcomers

Comments

@shepmaster
Copy link
Owner

shepmaster commented Feb 2, 2019

Right now, we only support an underlying error denoted by the field named source and a backtrace by the name backtrace. We should also support a field attribute (#[snafu(source)] / #[snafu(backtrace)]), with an optional boolean to disable the automatic (#[snafu(source(false))] / #[snafu(backtrace(false))])

Precedence wise, it should be:

  1. attribute
  2. source
@shepmaster shepmaster added enhancement New feature or request help wanted Extra attention is needed good first issue Good for newcomers labels Feb 2, 2019
@shepmaster shepmaster added the feedback requested User feedback desired on a design decision label Feb 26, 2019
@lnicola
Copy link

lnicola commented Apr 25, 2019

Should the inner error be optional? Right now you get a compile error if you don't have one.

@shepmaster
Copy link
Owner Author

Should the inner error be optional

I’m not sure I’m following. If you don’t include a source field, then there will be no inner error.

Do you mean optional as in source: Option<SomeError>? If so, could you expand a bit about why you’d want that and what the use case is?

@lnicola
Copy link

lnicola commented Apr 25, 2019

In your words :-):

If the original variant had a source field, its context selector will have an implementation of From for a Context

I didn't notice that, and this error got me a little confused, because it's not exactly clear what the issue is:

error[E0277]: the trait bound `Error: std::convert::From<snafu::Context<std::io::Error, Rename<&std::path::Path>>>` is not satisfied
   --> src/main.rs:113:13
    |
113 |             fs::rename(path, cleaned).context(Rename { path })?;
    |             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ the trait `std::convert::From<snafu::Context<std::io::Error, Rename<&std::path::Path>>>` is not implemented for `Error`
    |
    = help: the following implementations were found:
              <Error as std::convert::From<snafu::Context<snafu::NoneError, PathDecode<__T0, __T1>>>>
              <Error as std::convert::From<snafu::Context<snafu::NoneError, Rename<__T0>>>>
              <Error as std::convert::From<snafu::Context<snafu::NoneError, UnsupportedEncoding<__T0>>>>
              <Error as std::convert::From<snafu::Context<uchardet::errors::Error, UnknownEncoding<__T0>>>>
              <Error as std::convert::From<snafu::Context<walkdir::Error, Enumerate>>>
    = note: required by `std::convert::From::from`

I'm not saying you should make that code work, it's just a thing I ran into.

@shepmaster shepmaster changed the title Support additional ways of specifying the underlying error Support additional ways of specifying the error source or backtrace May 4, 2019
shepmaster added a commit that referenced this issue May 5, 2019
@shepmaster shepmaster removed feedback requested User feedback desired on a design decision help wanted Extra attention is needed labels May 5, 2019
@aramrw
Copy link

aramrw commented Jan 4, 2025

I have a usecase but maybe I have the wrong idea

#[derive(Debug, Snafu)]
#[snafu(display("{website}:\n  {kind}"))]
pub struct WebsiteError {
    website: SupportedSites,
    kind: WebsiteErrorKind,
    source: fantoccini::error::CmdError,
}

#[derive(Debug, Snafu)]
pub enum WebsiteErrorKind {
    #[snafu(display("the query matched 0 elements on the page.\n  query: '{query}'"))]
    QuerySelectorAll { query: String },
    #[snafu(display(
        "the query only matched {img_count} <img />s when {page_count} were expected.\n  query: '{query}'"
    ))]
    MissingImagesAfterMaxRetries {
        img_count: usize,
        page_count: usize,
        query: String,
    },
}

then I can do this:

//  Result<Vec<Element>, fantoccini::error::CmdError>          
let imgs = self.find_all(Locator::Css(&query)).await.context(WebsiteSnafu {
    website: SupportedSites::MangaFire,
    kind: WebsiteErrorKind::QuerySelectorAll { query },
})?;

but now you cant return errors that dont need a source

// error[E0063]: missing field `source` in initializer of `WebsiteError`
return Err(WebsiteError {
    website: SupportedSites::MangaFire,
    kind: WebsiteErrorKind::QuerySelectorAll { query },
});

ideally you could make source an Option

// only provides a source if .context is used
#[derive(Debug, Snafu)]
#[snafu(display("{website}:\n  {kind}"))]
pub struct WebsiteError {
    website: SupportedSites,
    kind: WebsiteErrorKind,
    source: Option<fantoccini::error::CmdError>,
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

3 participants