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

Add rescue for JSON::ParserError #183

Closed
wants to merge 1 commit into from
Closed

Add rescue for JSON::ParserError #183

wants to merge 1 commit into from

Conversation

leoarnold
Copy link

The Oj parser v3.3.7+ will throw an Oj::ParseError and
(when mimicking json) a JSON::ParserError.

MultiJSON assumes that any JSON parser will just throw
one specific type of exception, but it seems fair to
also rescue any JSON::ParserError thrown while parsing.

Solves: voxpupuli/json-schema#399

The Oj parser v3.3.7+ will throw an Oj::ParseError and
(when mimicking json) a JSON::ParserError.

MultiJSON assumes that any JSON parser will just throw
one specific type of exception, but it seems fair to
also rescue any JSON::ParserError thrown while parsing.

Solves: voxpupuli/json-schema#399
@leoarnold leoarnold closed this Jan 8, 2018
@leoarnold leoarnold reopened this Jan 8, 2018
@leoarnold
Copy link
Author

Travis CI retriggered

@leoarnold
Copy link
Author

Okay, these build failures are due to outdated dependencies, not my changes.

@rwz
Copy link
Member

rwz commented Jan 8, 2018

I'm not sure why you'd use Oj in mimic-stdlib-JSON mode with Oj adapter vs JsonGem adapter.

This seems more like a configuration issue to me, but in any case it's worth addressing I guess.

@rwz
Copy link
Member

rwz commented Jan 8, 2018

This should be addressed by 275e3ff

@rwz rwz closed this Jan 8, 2018
@rwz
Copy link
Member

rwz commented Jan 8, 2018

Released 0.13.0

@leoarnold leoarnold deleted the json-schema-399 branch January 9, 2018 09:42
@leoarnold
Copy link
Author

Thanks! Interesting way to solve this.

To the drive-by reader: This is of course fixed in v1.13.0, not in v0.13.0.

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