Rust SDK: remove anyhow, use thiserror w/ structured error types
#2169
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
Based on the lints failure, it seems we need to get #2111 merged, then get this updated to latest master. |
jsdt
approved these changes
Jan 24, 2025
bfops
added
release-any
1 task
Our use of `anyhow` in the Rust SDK was originally due to my being lazy. I figured I'd kick the can on writing actual error types as long as possible, and it turns out now's the time that's no longer possible, because while working on a follow-up I needed to `clone` an error, and `anyhow::Error: !Clone`. This commit removes the Rust SDK's use of `anyhow`, replacing it with a `thiserror`-annotated `enum Error`. This resulted in essentially simple mechanical changes to a lot of the SDK code, and also to the codegen, resulting in a truly enormous diff.
Per out-of-band suggestion from @jsdt, rework the `Error` type so that many former variants are now under `Error::Internal(InternalError)`, with `InternalError` not exposing its contents except via `std::error::Error`.
gefjon
force-pushed
the
phoebe/thiserror
branch
from
ceb0586 to
bb7fd06
Compare
|
Test failure is unrelated and is fixed by #2199 . |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description of Changes
Our use of
anyhowin the Rust SDK was originally due to my being lazy. I figured I'd kick the can on writing actual error types as long as possible, and it turns out now's the time that's no longer possible, because while working on another ticket I needed toclonean error, andanyhow::Error: !Clone.This commit removes the Rust SDK's use of
anyhow, replacing it with athiserror-annotatedenum Error.This resulted in essentially simple mechanical changes to a lot of the SDK code, and also to the codegen, resulting in a truly enormous diff.
Advice for reviewers
Start in
crates/sdk/src/error.rsand see if the error enum makes sense. Then look through the rest of the SDKsrcto verify that I'm constructing them sensibly. Then checkcrates/cli/src/subcommands/generate/rust.rs. Do not at any point look at the generated code insidecrates/sdk/tests, as it will be uninteresting and verbose.API and ABI breaking changes
Yep! The errors returned from the SDK functions are now of a different type.
Expected complexity level and risk
1: large volume of mechanical changes. It was essentially impossible to depend on the actual nature of the emitted errors before, as
anyhowisn't really useful for that, so we're effectively adding strictly more information in a way that should not break any existing client code.Testing