Changed exception type for invalid input between 3.3.6 and 3.3.7

[colszowka]

Hey there,

I noticed a change of behaviour between versions 3.3.6 and 3.3.7: When passing entirely wrong input, before Oj used to raise Oj::ParseError in compat mode. With 3.3.7, it gives EncodingError:

gem 'oj', '= 3.3.6'
require 'oj'

puts "oj version: #{Oj::VERSION}"
Oj.default_options = { mode: :compat }
Oj.load "this is not json"

Output for the two versions:

oj version: 3.3.6
oj-change.rb:6:in `load': expected true at line 1, column 2 [parse.c:120] in 'this is not json (Oj::ParseError)

oj version: 3.3.7
oj-change.rb:6:in `load': Empty input at line 1, column 2 [parse.c:926] in 'this is not json (EncodingError)

Not sure if this is intentional, but this feels like a breaking change to me. It's also awkward that now the exception is a standard-ruby one as opposed to the previous Oj-specific exception, so my guess would be this change was unintentional?

[ohler55]

In 3.3.6 I attempted to catch empty string as an error which also catches the string you provided. Unfortunately I used the wrong exception so it was inconsistent with Rails and the JSON gem. 3.3.7 fixes that. Since it was short period of time between the releases I didn't think too many people would notice the change so I did not bump the version to 3.4 as the change in API from one exception type to another didn't feel that significant.

[ohler55]

Update: I see that for compatibility mode and NOT mimicking the JSON gem that the type is EncodingError. I should have read you issue more carefully. It should be Oj::ParseError is not mimicking JSON.

[colszowka]

I had a look at the code we ran into this with, the relevant line where we used to catch the Oj::ParseError was added around two months ago, so it must have been the behavior for versions prior to 3.3.6, too, since that was released later.

It's not giving us headaches, we simply catch the EncodingError too, now, but it seemed an API change notable enough to at least give you a heads up on it :) Like I said, I'd prefer Oj to give me an exception owned by the gem itself, so I preferred the old behaviour, but feel free to close this issue if the new behaviour is desired :)

[ohler55]

I'll make the error a Oj::ParseError unless in :compat mode or :rails mode. It will have to wait a few more days before I can get to it though.

[ohler55]

Just pushed a 'parse-error' branch that should use Oj::ParseError instead of EncodingError in all cases other than with the JSON gem or Rails.

[ohler55]

Any comments?

[ohler55]

Is this still of interest or should I let it slide into oblivion?

[leoarnold]

Hi Peter, issue has just come to my interest by means of an intricate entanglement:

We are using json-schema to validate our JSON responses. This gem has a nifty mechanism which will unalterably make use of multi_json even if multi_json is just an indirect dependency of the project.

multi_json assumes that any supported JSON parser only throws one single type of parsing exception - hence the relation to this very issue.

This causes a rather unintuitive chain of events when updating oj:

• multi_json will detect Oj::ParseError and convert it into an MultiJson::ParseError, but it will let a JSON::ParserError exception fly through. One could only fix this if every version if oj only ever throws one type of parsing exceptions.
• json-schema is even worse: It will first try to Oj.load('schema.json') - and only if it catches a MultiJson::ParseError will it retry something vaguely similar to Oj.load(File.read('schema.json'))

So, your change is definitely of public interest because of bad architectural decisions on many sides, and I'm not sure which component should best be adapted (or replaced).

[ohler55]

Thanks for the background. Are you asking for a change in the current Oj version?

[leoarnold]

Since the resulting error message references parse.c, I thought the oj repository would be the best place to put my findings in order to get the attention of everyone affected.

Yet I am not sure what to do about this: Reverting back to the old style of throwing exceptions would cure the symptoms, not the problems.

[leoarnold]

Note that multi_json is only tested against oj ~> 2.18, so there's definitely work to do there - and then agian json-schema had many stalling attemps to drop multi_json anyway.

[ohler55]

That is a very old version of Oj. I believe Oj is raising an Oj::ParseError unless it is mimicking rails or the json gem. Isn't that what is desired in your case?

[leoarnold]

Yes, we do also have oj_mimic_json in our project.

[ohler55]

That does present a problem. Of course Oj can not mimic the JSON gem and then raise incompatible exception types. Good luck with getting the gems updated.

[colszowka]

@ohler55 sorry for the late reply, for us the problem is not relevant anymore, since we just adjusted the rescue to also handle encoding errors. Thanks!