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 anyhow::Error in state generation functions #264

Merged
merged 3 commits into from
Mar 9, 2023

Conversation

marienz
Copy link
Contributor

@marienz marienz commented Mar 5, 2023

By generalizing the supported errors in state generation from types that directly implement std::error::Error to types that can be converted into a boxed std::error::Error, we can make anyhow::Error work.

I first tried to support just anyhow::Error specifically (in addition to the already-supported error types), but the compiler does not easily allow that: it thinks anyhow::Error might implement std::error::Error in the future, so it will not let me implement things on both. Please make sure the broader support for error types this adds is reasonable (and that I am correct about this change being backwards-compatible) before merging.

@marienz
Copy link
Contributor Author

marienz commented Mar 5, 2023

CI failures:

  • "Error: couldn't parse latest version of 'wasm-opt'" looks unnrelated
  • "assertion failed: greeting.contains("Open up")', examples/core/preload/tests/main.rs:22:5" I don't understand but that test passes locally.

@arctic-hen7
Copy link
Member

Oh this is great, thank you so much!

Those CI failures are normal at this point, don't worry about them.

Copy link
Member

@arctic-hen7 arctic-hen7 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All looks good to me!

`anyhow::Error` cannot implement `std::error::Error` (see
dtolnay/anyhow#136), which Perseus requires
for errors returned from state generation functions. But Perseus
immediately boxes the error, and `anyhow::Error` does support conversion
into `Box<dyn std::error::Error>`. So if we make the conversion from
`BlamedError` to `GenericBlamedError` more general,
`BlamedError<anyhow::Error>` works.

This change should be backwards-compatible because Rust has an impl
`From<E> for Box<dyn Error + Send + Sync + 'a>`. That impl should match
all error types previously accepted by Perseus, and is sufficient to
still accept them after this change.
This makes `?` on an `anyhow::Error` work in a function returning
`Result<S, BlamedError<anyhow::Error>>`.
@marienz
Copy link
Contributor Author

marienz commented Mar 6, 2023

Force-pushed to sign the commits (no content changes).

Copy link
Member

@arctic-hen7 arctic-hen7 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is good to merge!

@arctic-hen7 arctic-hen7 changed the title Support anyhow::Eror in state generation functions Support anyhow::Error in state generation functions Mar 9, 2023
@arctic-hen7 arctic-hen7 merged commit 53ad2ad into framesurge:main Mar 9, 2023
@marienz marienz deleted the anyhow branch March 9, 2023 10:13
arctic-hen7 added a commit that referenced this pull request Mar 22, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants