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

[experiment/proposal] rust: add better error messages #789

Draft
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

benma
Copy link
Collaborator

@benma benma commented Sep 10, 2021

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.

@benma benma changed the title rust: add better error messages [experiment/proposal] rust: add better error messages Sep 10, 2021
@benma
Copy link
Collaborator Author

benma commented Sep 10, 2021

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.
@benma benma marked this pull request as draft June 12, 2022 21:49
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.

None yet

1 participant