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

Query/Form: Use serde_path_to_error to report fields that failed to parse #3081

Merged
merged 9 commits into from
Dec 20, 2024

Conversation

Turbo87
Copy link
Collaborator

@Turbo87 Turbo87 commented Dec 18, 2024

Motivation

I've started using the Query extractor more and more over the past couple of days and noticed that its error messages don't help the user in understanding which field specifically failed to parse.

Solution

This solves the problem roughly similar to our existing usage of serde_path_to_error in the Json extractor. The resulting error messages change from e.g.

Failed to deserialize query string: invalid digit found in string

to

Failed to deserialize query string: my_broken_field: invalid digit found in string

@Turbo87
Copy link
Collaborator Author

Turbo87 commented Dec 18, 2024

/cc @jplatte :)

Copy link
Member

@jplatte jplatte left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mind doing the same thing for Form and both serde_html_form extractors in axum-extra?

Also, changelog entries

@Turbo87
Copy link
Collaborator Author

Turbo87 commented Dec 20, 2024

doing the same thing for Form

it looks like the Form extractor already includes the field name (from the tests: Invalid URL: Cannot parse reswith value123123-123-123123: UUID parsing failed: invalid group count: expected 5, found 3), though in this case the user isn't aware of the internal name of the field, so I don't know how useful that even is 😅

and both serde_html_form extractors in axum-extra?

yeah, I'll add it to the other Query extractor too

@Turbo87
Copy link
Collaborator Author

Turbo87 commented Dec 20, 2024

nevermind, I was looking at the Path extractor, not the Form extractor... 🤦‍♂️

too early in the morning 😂

@Turbo87 Turbo87 changed the title axum/extract/query: Use serde_path_to_error to report key that failed to parse Query/Form: Use serde_path_to_error to report fields that failed to parse Dec 20, 2024
@Turbo87
Copy link
Collaborator Author

Turbo87 commented Dec 20, 2024

Mind doing the same thing for Form and both serde_html_form extractors in axum-extra?

Also, changelog entries

alright, should all be done now :)

Copy link
Member

@jplatte jplatte left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

@jplatte jplatte merged commit 9cd5cc4 into tokio-rs:main Dec 20, 2024
18 checks passed
@Turbo87 Turbo87 deleted the query-errors branch December 20, 2024 11:01
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.

2 participants