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

Change the behaviour of .query: optional params and explicit error in the response body #384

Merged
merged 1 commit into from
Jan 15, 2020

Conversation

LukeMathWalker
Copy link
Contributor

@LukeMathWalker LukeMathWalker commented Dec 24, 2019

This PR changes the behavior of .query: if the caller has not specified any query parameter, we still try to deserialise the desired type from an empty string.

This allows successful deserialisation of structs which do not have any mandatory field, e.g.

struct OptionalParams {
   author: Option<String>,
   theme: Option<String>
}

On master, this requires using unwrap_or on the outcome of .query which I think it's unnecessary.
If you forget about it, it leads to surprising behavior: calling the endpoint with any query parameter (none of which in the struct) would trigger a successful deserialisation, while a call without query params would result in a 400.

I have also added, for the unhappy case, the deserialisation error in the response body: it makes for a much more pleasant experience when trying to debug why an endpoint is returning Bad Request 😅

…ry parameters, try to deserialise in any case from an empty string. This allows successful deserialisation of structs which do not have mandatory fields (e.g. all fields are Option<X>).

Return detailed error message when deserialisation fails, to help the caller debug what was wrong with their request.
Copy link
Member

@yoshuawuyts yoshuawuyts left a comment

Choose a reason for hiding this comment

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

This is great; thank you!

@yoshuawuyts yoshuawuyts merged commit 00f76c9 into http-rs:master Jan 15, 2020
@LukeMathWalker LukeMathWalker deleted the query-parsing branch January 15, 2020 10:26
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