Skip to content
This repository has been archived by the owner on Dec 4, 2018. It is now read-only.

Handle jsonparse errors consistently #138

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ataube
Copy link

@ataube ataube commented Jul 7, 2017

closes #136

I am not 100% sure if I like this. Overall the error checks are very fragile in case of jsonparse changing some wording, we would probably miss them.

Also not this PR breaks code If someone relies on the unhandled error message structure.

We could add some test and and simulate all jsonparse errors to ensure every message gets wrapped correctly. What do you think?

@dominictarr
Copy link
Owner

yeah I think matching error strings is likely to come back to bite us. The error messages are not meant to be a hint for a human, not for a machine. Someone might make a PR to jsonparse to improve the error messages, which would break us! I think the right way to do this, would be to make a PR to jsonparse for machine readable errors. Say adding state and position properties, also add tests and then those properties are part of the official api,

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Not all jsonparse errors are handled by JSONStream
2 participants