Skip to content

Better error reporting on invalid upload request #860

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

Merged
merged 1 commit into from
Jul 17, 2017
Merged

Better error reporting on invalid upload request #860

merged 1 commit into from
Jul 17, 2017

Conversation

ordian
Copy link

@ordian ordian commented Jul 8, 2017

See comment.

@sgrif
Copy link
Contributor

sgrif commented Jul 14, 2017

Can you explain what this commit does in the message, and why it improves the error reporting? (Probably mention the problem linked in that comment, and what the message is now)

@ordian
Copy link
Author

ordian commented Jul 14, 2017

Sure, the problem is that after switching to serde we're returning custom serde error, for which debug implementation doesn't look nice. Solution to this problem is to use display (to_string) implementation instead of debug.
play example
with debug format:

ErrorImpl { code: Message("invalid value: string \"name\", expected ERROR MESSAGE"), line: 0, column: 0 }

with display:

invalid value: string "name", expected ERROR MESSAGE

@sgrif
Copy link
Contributor

sgrif commented Jul 14, 2017

I'd love to see that in the commit message. 😄

Problem: after switching to serde we're returning custom serde error,
for which debug implementation doesn't look nice.

Solution: use display (to_string) when reporting an error
instead of debug.

example error message before:
`ErrorImpl { code: Message("invalid value: string \"name\", \
expected ERROR MESSAGE"), line: 0, column: 0 }`

after:
`invalid value: string "name", expected ERROR MESSAGE`
@carols10cents carols10cents merged commit ce01a2b into rust-lang:master Jul 17, 2017
@carols10cents
Copy link
Member

Love it, thank you!!! ❤️

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.

4 participants