ux: Lack of key and origin in ConfigError::Message leads to uncertain sources from serde error messages
Nobody has claimed this yet.
- Dominant language
- Rust
- Stars
- 3.2k
- Forks
- 265
- Avg merge
- 2h 42m
- Merged PRs (30d)
- 4
Description
Summary
If a configuration error passes through a serde::de::Error implementation, it is wrapped as a ConfigError::Message, and information about what caused the error is not preserved. This confuses the users, because the message does not tell them which option was set incorrectly.
Example
use serde::Deserialize;
#[derive(Deserialize, Clone, Debug)]
pub struct Settings {
direct: Option<u8>,
via_serde: Option<Item>,
}
#[derive(Deserialize, Clone, Debug)]
#[serde(try_from="String")]
pub struct Item(u8);
impl TryFrom<String> for Item {
type Error = std::num::ParseIntError;
fn try_from(s: String) -> Result<Item, Self::Error> {
s.parse().map(Item)
}
}
fn main() {
let settings = config::Config::builder()
.add_source(config::File::with_name("demo.toml"))
.build()
.unwrap();
let settings = settings
.try_deserialize::<Settings>();
println!("{:?}", settings);
}
If the above code is run with the configuration direct = "foo", then we get a helpful error message:
invalid type: string "foo", expected an integer for key `direct` in demo.toml
But if the above code is run with the configuration via_serde = "foo", then we get the not-so-helpful message:
invalid digit found in string.
Note that it doesn't mention "via_serde" or "demo.toml".
It would be great if instead we go something like:
invalid digit found in string (for key `via_serde` in demo.toml
Possible solutions
My first thought here would be to adjust Message to include an optional key fields, so that prepend_key can set it.
If we don't want to break backward compatibility with code that uses the current Message variant, I would define a new MessageWithKey or InvalidValue error variant, and have prepend_key() convert Message into that variant instead.
Reference
For us this issue is https://gitlab.torproject.org/tpo/core/arti/-/issues/1267
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start by tracing ConfigError::Message and prepend_key(), especially how serde:🇩🇪:Error failures are wrapped and how key and source context are currently handled. Compare the proposed optional key field with a separate error variant, then verify that the resulting error preserves the original message while identifying the configuration key and source without breaking existing Message users.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- backend
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 35/100