Skip to content

Commit

Permalink
Improve serde error output (#982)
Browse files Browse the repository at this point in the history
In our tests using the serde impl on `Url`, we found that failing URL serialization says the inverse of what it should say.

```toml
[project]
name = "foo"
version = "0.0.0"
dependencies = [
  "tqdm ==4.66.0",
]
[tool.uv.sources]
tqdm = { url = "§invalid#+#*Ä" }
```

```
error: Failed to parse: `pyproject.toml`
  Caused by: TOML parse error at line 10, column 16
   |
10 | tqdm = { url = "§invalid#+#*Ä" }
   |                ^^^^^^^^^^^^^^^^^
invalid value: string "§invalid#+#*Ä", expected relative URL without a base
```

It says that expected a relative URL without a base, when this was the unexpected input that caused the error. Using `serde::de::Error::custom` oder `serde::de::Error::invalid_value` improves the error message:

```
error: TOML parse error at line 8, column 16
  |
8 | tqdm = { url = "§invalid#+#*Ä" }
  |                ^^^^^^^^^^^^^^^^^
relative URL without a base: "§invalid#+#*Ä"
```
  • Loading branch information
konstin authored Oct 28, 2024
1 parent 30e6258 commit 5d363cc
Show file tree
Hide file tree
Showing 2 changed files with 22 additions and 10 deletions.
15 changes: 5 additions & 10 deletions url/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2700,7 +2700,7 @@ impl Url {
where
D: serde::Deserializer<'de>,
{
use serde::de::{Deserialize, Error, Unexpected};
use serde::de::{Deserialize, Error};
let (
serialization,
scheme_end,
Expand All @@ -2726,10 +2726,8 @@ impl Url {
fragment_start,
};
if cfg!(debug_assertions) {
url.check_invariants().map_err(|reason| {
let reason: &str = &reason;
Error::invalid_value(Unexpected::Other("value"), &reason)
})?
url.check_invariants()
.map_err(|reason| Error::custom(reason))?
}
Ok(url)
}
Expand Down Expand Up @@ -2942,7 +2940,7 @@ impl<'de> serde::Deserialize<'de> for Url {
where
D: serde::Deserializer<'de>,
{
use serde::de::{Error, Unexpected, Visitor};
use serde::de::{Error, Visitor};

struct UrlVisitor;

Expand All @@ -2957,10 +2955,7 @@ impl<'de> serde::Deserialize<'de> for Url {
where
E: Error,
{
Url::parse(s).map_err(|err| {
let err_s = format!("{}", err);
Error::invalid_value(Unexpected::Str(s), &err_s.as_str())
})
Url::parse(s).map_err(|err| Error::custom(format!("{}: {:?}", err, s)))
}
}

Expand Down
17 changes: 17 additions & 0 deletions url/tests/unit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1362,3 +1362,20 @@ fn issue_974() {
let _ = url::quirks::set_port(&mut url, "\u{0000}9000");
assert_eq!(url.port(), Some(8000));
}

#[cfg(feature = "serde")]
#[test]
fn serde_error_message() {
use serde::Deserialize;
#[derive(Debug, Deserialize)]
#[allow(dead_code)]
struct TypeWithUrl {
url: Url,
}

let err = serde_json::from_str::<TypeWithUrl>(r#"{"url": "§invalid#+#*Ä"}"#).unwrap_err();
assert_eq!(
err.to_string(),
r#"relative URL without a base: "§invalid#+#*Ä" at line 1 column 25"#
);
}

0 comments on commit 5d363cc

Please sign in to comment.