[experiment/proposal] rust: add better error messages#789
Draft
benma wants to merge 1 commit intoBitBoxSwiss:masterfrom
Draft
[experiment/proposal] rust: add better error messages#789benma wants to merge 1 commit intoBitBoxSwiss:masterfrom
benma wants to merge 1 commit intoBitBoxSwiss:masterfrom
Conversation
Collaborator
Author
|
Best to look at the unit tests in errors.rs first to see how it is used. The rest of the changes are simply adding some error strings to failure paths. |
The protobuf Error response has a message field, but it is hardcoded
to generic strings depending on the error type, e.g. "generic",
"invalid input", etc. When a user/developer reports this error, better
information is useful.
This patch adds error messages based on the error condition.
Unfortunately, for now we still have to stick to static strings with
no runtime information, e.g. "invalid keypath" over "keypath invalid:
{}" cotaining the actual keypath. I experimented with dynamic strings,
but this immediatelly added another ~7kB in binary bloat due the usage
of String and formatting. Only changing the type from `&'static str`
to `String` adds a few kB.
Alternatives considered:
- use String+format!() to be able to return runtime info (e.g. actual
keypaths used), as well as chain context strings together, but that
resulted in too many kilobytes of additional binary bloat.
- error deps like anyhow, snafu, etc. They all add a lot of binary and
code bloat.
- using only numeric error codes instead of static strings to save
binary space. Can still do in the future if needed.
- using `ufmt` crate hoping it produces smaller binaries than
`format!()` for dynamic strings. I tried it and it was about the same.
0e1952d to
6d0e8e5
Compare
This file contains hidden or 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
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.
The protobuf Error response has a message field, but it is hardcoded
to generic strings depending on the error type, e.g. "generic",
"invalid input", etc. When a user/developer reports this error, better
information is useful.
This patch adds error messages based on the error condition.
Unfortunately, for now we still have to stick to static strings with
no runtime information, e.g. "invalid keypath" over "keypath invalid:
{}" cotaining the actual keypath. I experimented with dynamic strings,
but this immediatelly added another ~7kB in binary bloat due the usage
of String and formatting. Only changing the type from
&'static strto
Stringadds a few kB.Alternatives considered:
keypaths used), as well as chain context strings together, but that
resulted in too many kilobytes of additional binary bloat.
code bloat.
binary space. Can still do in the future if needed.
ufmtcrate hoping it produces smaller binaries thanformat!()for dynamic strings. I tried it and it was about the same.