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

Avoid use of anyhow #161

Open
anakrish opened this issue Feb 23, 2024 · 3 comments · May be fixed by #212
Open

Avoid use of anyhow #161

anakrish opened this issue Feb 23, 2024 · 3 comments · May be fixed by #212
Labels
dependencies Pull requests that update a dependency file enhancement New feature or request

Comments

@anakrish
Copy link
Collaborator

anakrish commented Feb 23, 2024

Avoid use of anyhow in Regorus.

Turns out ThisError is not no_std compatible.


Original description:

According to comparison,

"Use Anyhow if you don't care what error type your functions return, you just want it to be easy. This is common in application code. Use thiserror if you are a library that wants to design your own dedicated error type(s) so that on failures the caller gets exactly the information that you choose".

The errors in Regorus need to be changed to ThisError.

mkulke added a commit to mkulke/regorus that referenced this issue Apr 24, 2024
fixes microsoft#161

This change replaces stringly-typed anyhow errors with
thiserror-constructed explicit error enums.

The following strategy has been used in the conversion:

- Every module maintains its own error enum
- Exception is builtins/*.rs in which all errors are consolidated in one
  enum to avoid duplication
- Source errors are being wrapped and propagated in the error string
- Crate-specific businesslogic errors have been convernted into their
  own enum
- A simple `bail!` macro has been added to reduce changes
- Extension fn's require a `Box<dyn std::error::Error>` error type. it
  would be nice if the Extension fn's could have a result with trait
  bound `std::error::Error` but that requires some unergonomic type
  gymnastics on the trait + impls
- A local type alias for `Result` has been added to modules to reduce
  changes in the Signatures where reasonable.

Signed-off-by: Magnus Kulke <magnuskulke@microsoft.com>
mkulke added a commit to mkulke/regorus that referenced this issue Apr 24, 2024
fixes microsoft#161

This change replaces stringly-typed anyhow errors with
thiserror-constructed explicit error enums.

The following strategy has been used in the conversion:

- Every module maintains its own error enum
- Exception is builtins/*.rs in which all errors are consolidated in one
  enum to avoid duplication
- Source errors are being wrapped and propagated in the error string
- Crate-specific businesslogic errors have been convernted into their
  own enum
- A simple `bail!` macro has been added to reduce changes
- Extension fn's require a `Box<dyn std::error::Error>` error type. it
  would be nice if the Extension fn's could have a result with trait
  bound `std::error::Error` but that requires some unergonomic type
  gymnastics on the trait + impls
- A local type alias for `Result` has been added to modules to reduce
  changes in the Signatures where reasonable.

Signed-off-by: Magnus Kulke <magnuskulke@microsoft.com>
@mkulke mkulke linked a pull request Apr 24, 2024 that will close this issue
mkulke added a commit to mkulke/regorus that referenced this issue Apr 24, 2024
fixes microsoft#161

This change replaces stringly-typed anyhow errors with
thiserror-constructed explicit error enums.

The following strategy has been used in the conversion:

- Every module maintains its own error enum
- Exception is builtins/*.rs in which all errors are consolidated in one
  enum to avoid duplication
- Source errors are being wrapped and propagated in the error string
- Crate-specific businesslogic errors have been convernted into their
  own enum
- A simple `bail!` macro has been added to reduce changes
- Extension fn's require a `Box<dyn std::error::Error>` error type. it
  would be nice if the Extension fn's could have a result with trait
  bound `std::error::Error` but that requires some unergonomic type
  gymnastics on the trait + impls
- A local type alias for `Result` has been added to modules to reduce
  changes in the Signatures where reasonable.

Signed-off-by: Magnus Kulke <magnuskulke@microsoft.com>
mkulke added a commit to mkulke/regorus that referenced this issue Apr 24, 2024
fixes microsoft#161

This change replaces stringly-typed anyhow errors with
thiserror-constructed explicit error enums.

The following strategy has been used in the conversion:

- Every module maintains its own error enum
- Exception is builtins/*.rs in which all errors are consolidated in one
  enum to avoid duplication
- Source errors are being wrapped and propagated in the error string
- Crate-specific businesslogic errors have been convernted into their
  own enum
- A simple `bail!` macro has been added to reduce changes
- Extension fn's require a `Box<dyn std::error::Error>` error type. it
  would be nice if the Extension fn's could have a result with trait
  bound `std::error::Error` but that requires some unergonomic type
  gymnastics on the trait + impls
- A local type alias for `Result` has been added to modules to reduce
  changes in the signatures where reasonable.

Signed-off-by: Magnus Kulke <magnuskulke@microsoft.com>
mkulke added a commit to mkulke/regorus that referenced this issue Apr 24, 2024
fixes microsoft#161

This change replaces stringly-typed anyhow errors with
thiserror-constructed explicit error enums.

The following strategy has been used in the conversion:

- Every module maintains its own error enum
- Exception is builtins/*.rs in which all errors are consolidated in one
  enum to avoid duplication
- Source errors are being wrapped and propagated in the error string
- Crate-specific businesslogic errors have been convernted into their
  own enum
- A simple `bail!` macro has been added to reduce changes
- Extension fn's require a `Box<dyn std::error::Error>` error type. it
  would be nice if the Extension fn's could have a result with trait
  bound `std::error::Error` but that requires some unergonomic type
  gymnastics on the trait + impls
- A local type alias for `Result` has been added to modules to reduce
  changes in the signatures where reasonable.

Signed-off-by: Magnus Kulke <magnuskulke@microsoft.com>
@anakrish anakrish added enhancement New feature or request dependencies Pull requests that update a dependency file labels May 10, 2024
@mkulke
Copy link

mkulke commented May 15, 2024

with the recent addition of no-std targets, we probably cannot use thiserror in the near term. The generic error trait is in std::error::Error and the effort to move it to core:: is ongoing.

There are similar libs with no-std support, but maybe the macro niceties are not required and we can do a bit boilerplate ourselves. looking into that.

@anakrish
Copy link
Collaborator Author

anakrish commented May 15, 2024

Interesting that anyhow supports no_std but ThisError doesn't (can't).

@anakrish anakrish changed the title Use ThisError instead of anyhow Avoid use if anyhow May 15, 2024
@anakrish anakrish changed the title Avoid use if anyhow Avoid use of anyhow May 15, 2024
@mkulke
Copy link

mkulke commented May 16, 2024

this makes sense, because afaiu thiserror is just an elaborate machinery of helper-macros that will desuger into implementations of the std::error::Error trait, while anyhow implements its own error type.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants