Actually print out some errors#100
Conversation
| }, | ||
|
|
||
| #[error("RotRequest")] | ||
| #[error("RotRequest {0}")] |
There was a problem hiding this comment.
would have really liked this earlier
18:53:45.720Z ERRO SledAgent (trust-quorum): failed to load certified key: RotRequest
baseboard_id = 913-0000023:BRM13250012
file = /home/build/.cargo/git/checkouts/sprockets-882d17aeeb0cb343/8ba93f6/tls/src/server.rs:80
There was a problem hiding this comment.
This is very annoying, but: these changes are not correct. We need to update the logging sites, not the error types. More on this whole unfortunate error type design: https://github.com/oxidecomputer/omicron/blob/main/docs/error-types-and-logging.adoc. (The changes in this branch are "Pitfall 2" of that doc.)
There was a problem hiding this comment.
bleh I read that and thought I understood what I needed to do. Clearly not!
labbott
force-pushed
the
seriously_better_errors
branch
from
2943f68 to
ad905a2
Compare
| } | ||
| Err(e) => { | ||
| error!(self.log, "failed to load certified key: {e}"); | ||
| error!(self.log, "failed to load certified key: {e:#}"); |
There was a problem hiding this comment.
Sorry again for the state of this :(. {e:#} only works for anyhow::Error and does nothing for other error types, so this still won't do what you want.
It looks like this is this crate's Error type, which already derives SlogInlineError, so I believe this can be
| error!(self.log, "failed to load certified key: {e:#}"); | |
| error!(self.log, "failed to load certified key"; e); |
since deriving SlogInlineError makes the type implement slog::KV (in a way that will print the entire chain).
There was a problem hiding this comment.
I forgot we weren't using anyhow here 🙃
2 participants
No description provided.