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

Improve JSONDecodingError #581

Closed
thomasvl opened this issue Jun 2, 2017 · 4 comments
Closed

Improve JSONDecodingError #581

thomasvl opened this issue Jun 2, 2017 · 4 comments
Labels
kind/enhancement Improvements to existing feature.

Comments

@thomasvl
Copy link
Collaborator

thomasvl commented Jun 2, 2017

On client devices most JSON decoding errors will simply be "give up" types of cases. But some applications might want to log the error and report it back to a server via some metrics/usage channel.

In server uses, being able to get more details about bad inputs from clients will likely help debug the clients as well as for better server side logging to help detect DoS attacks vs. bad clients.

So…

Currently JSONDecodingError provides no real context of what went wrong. Some of them can easily include the sub part of the JSON that was bad (.malformedDuration, .malformedTimestamp, .unrecognizedEnumValue). In almost all cases, also including a line number/column number (or atleast offset into the blob) being parsed would likely help.

@mustiikhalil
Copy link

Is this issue still open? and do you want help with it?

@thomasvl
Copy link
Collaborator Author

thomasvl commented Mar 2, 2020

Yes, still open. Adding associated values for the additional context is what would be useful.

@mustiikhalil
Copy link

Okay, I’ll work on it during the weekend 😁

@thomasvl
Copy link
Collaborator Author

thomasvl commented Jul 9, 2024

main has gotten the infrastructure for a new error model that won't be api breaking, so new errors can have more details. However moving existing errors to that new model was counted as api breaking. So I'm closing this off as we won't actually evolve JSONDecodingError, but at some future point with 2.0 work, the existing errors could be given more context like this.

@thomasvl thomasvl closed this as completed Jul 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind/enhancement Improvements to existing feature.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants