Trouble with Oj parser on parse error

[RST-J]

Hi. We encountered a problem when using the Oj library as parser today. @iainbeeston Would you mind, if we also rescued the specific Oj-Error in parse? Or @hoxworth or @pd ? This is crucial to us and we would otherwise use a temporary fork unless we found a proper workaround. Or if we agree to handle the Oj error I'd prepare a fix and a patch release.

[iainbeeston]

If we rescued an OJ specific error would that also mean checking to see if Oj is present? Could you (in a pull request) add an extra Gemfile for Travis that includes Oj and have it reproduce the error?

On Tue, Feb 16, 2016 at 11:47 am, Jonas Peschla notifications@github.com wrote: Hi. We encountered a problem when using the Oj library as parser today. @iainbeeston [https://github.com/iainbeeston] Would you mind, if we also rescued the specific Oj-Error in parse [https://github.com/ruby-json-schema/json-schema/blob/28f0f4936ed9439fef1b1b3b2e73857bc5eed9d0/lib/json-schema/validator.rb#L413] ? Or @hoxworth [https://github.com/hoxworth] or @pd [https://github.com/pd] ? This is crucial to us and we would otherwise use a temporary fork unless we found a proper workaround. Or if we agree to handle the Oj error I'd prepare a fix and a patch release.

— Reply to this email directly or view it on GitHub [https://github.com/ruby-json-schema/json-schema/issues/305] .[https://github.com/notifications/beacon/AAG1WzI9AtU7mkA0bn3tegK8SIRm2Rz9ks5pkwPlgaJpZM4HbE1F.gif]

[RST-J]

I guess it would mean to check whether it is defined via defined?. And I actually think, as we use Oj via mimic_JSON this should not be necessary or its not the best mimicry. So for now I hacked a quick fix for us and reported the issue to Oj with the request to consider raising the respective JSON error instead which would make the replacement transparent in this case. If this gets accepted we only have to wait. Otherwise we need to consider whether we want to support this.

[iainbeeston]

That sounds sensible - I would have thought that if you use mimic_JSON Oj should at least extend any custom errors from a base class in the json gem

On 16 Feb 2016, at 13:56, Jonas Peschla notifications@github.com wrote:

I guess it would mean to check whether it is defined via defined?. And I actually think, as we use Oj via mimic_JSON this should not be necessary or its not the best mimicry. So for now I hacked a quick fix for us and reported the issue to Oj with the request to consider raising the respective JSON error instead which would make the replacement transparent in this case. If this gets accepted we only have to wait. Otherwise we need to consider whether we want to support this.

— Reply to this email directly or view it on GitHub https://github.com/ruby-json-schema/json-schema/issues/305#issuecomment-184692299.

[RST-J]

Ok, I think I have news. First, Peter Ohler already prepared a fix for this issue.

But I directly ran into another issue while testing his fix. Because I have multi_json installed on my machine, json-schema used it to do the parsing. I don't know whether it prefers oj over default json by default when both are available. But the behaviour was the following: A parse error with mimic_JSON ran into line 417 but without rescuing anything. I investigated this, the raised error was a JSON::ParserError which I thought was expected. Why this happens, I don't know and I'm not sure whether we should investigate that any more, or just see this as another argument to drop multi_json support.

Because what I actually expected until I detected multi_json being installed on my machine, is that the parser runs into line 425 which now with the oj-fix should work as expected.

The problem is, we have different behaviour just by having multi_json installed or not. And if you have other dependencies in your Gemfile which depend on it, you have no choice. Now consider this to only be development dependencies and then the fun of deploying to production. I don't like this. At least we should have the possibility to disable the use of multi_json if installed.

Fun fact: With multi_json on the machine, everything works just fine (latest releases of json-schema and oj, no hacks) if JSON::Validator.json_backend = :oj is set, but I don't know yet why that is the case. Edit: Now I know - because that setting is just forwarded to multi_json.

[iainbeeston]

Yeah, I've seen similar behaviour before. I would love to remove multi json (it's basically deprecated nowadays)

On Thursday, 18 February 2016, Jonas Peschla notifications@github.com wrote:

Ok, I think I have news. First, Peter Ohler already prepared a fix for this issue.

But I directly ran into another issue while testing his fix. Because I have multi_json installed on my machine, json-schema used it to do the parsing. I don't know whether it prefers oj over default json by default when both are available. But the behaviour was the following: A parse error with mimic_JSON ran into line 417 https://github.com/ruby-json-schema/json-schema/blob/28f0f4936ed9439fef1b1b3b2e73857bc5eed9d0/lib/json-schema/validator.rb#L417 but without rescuing anything. I investigated this, the raised error was a JSON::ParserError which I thought was expected. Why this happens, I don't know and I'm not sure whether we should investigate that any more, or just see this as another argument to drop multi_json support.

Because what I actually expected until I detected multi_json being installed on my machine, is that the parser runs into line 425 https://github.com/ruby-json-schema/json-schema/blob/28f0f4936ed9439fef1b1b3b2e73857bc5eed9d0/lib/json-schema/validator.rb#L425 which now with the oj-fix should work as expected.

The problem is, we have different behaviour just by having multi_json installed or not. And if you have other dependencies in your Gemfile which depend on it, you have no choice. Now consider this to only be development dependencies and then the fun of deploying to production. I don't like this. At least we should have the possibility to disable the use of multi_json if installed.

Fun fact: With multi_json on the machine, everything works just fine (latest releases of json-schema and oj, no hacks) if JSON::Validator.json_backend = :oj is set, but I don't know yet why that is the case.

— Reply to this email directly or view it on GitHub https://github.com/ruby-json-schema/json-schema/issues/305#issuecomment-185821579 .

[RST-J]

I close this, oj got updated in the meantime and now raises the correct error classes when it is told to mimic default JSON.