ux: Lack of key and origin in ConfigError::Message leads to uncertain sources from serde error messages #532

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