mailjet / mailjet-gem

[API v3] Mailjet official Ruby GEM
https://dev.mailjet.com
Other
130 stars 72 forks source link

YAJL/json_gem replaces `JSON` in the whole application (not only in the mailjet-gem's context) #255

Closed manuelvanrijn closed 6 months ago

manuelvanrijn commented 8 months ago

Introduction

After the introduction of #243, I'm getting the error JSON::ParserError: input must be a string or IO on places where I'm doing a JSON.parse(response.body) using the http gem. Although yajl-ruby claims in the README that it should be a drop-in replacement, it seems it doesn't work for this use case. Also, the author says he wants to drop the JSON gem compat API in yajl-ruby 2.0

Problem

The main problem is that this gem is uses require 'yajl/json_gem' which replaces the Kernel::JSON for the yajl one.

Reproduce

# Without yajl-ruby
JSON.parse(HTTP.get("http://headers.jsontest.com/").body)
=>
{"X-Cloud-Trace-Context"=>"ab3fc03bef51938f8f17a740bf9382af/2466497661788720302",
 "traceparent"=>"00-ab3fc03bef51938f8f17a740bf9382af-223ac28e2a3f8cae-00",
 "User-Agent"=>"http.rb/5.2.0",
 "Host"=>"headers.jsontest.com"}
# With yajl-ruby
JSON.parse(HTTP.get("http://headers.jsontest.com/").body)
/usr/local/bundle/gems/yajl-ruby-1.4.3/lib/yajl/json_gem/parsing.rb:15:in `rescue in parse': input must be a string or IO (JSON::ParserError)

Workaround

To work around this problem, you could .to_s the response so that Yajl can parse the body

JSON.parse(HTTP.get("http://headers.jsontest.com/").body.to_s)
=>
{"X-Cloud-Trace-Context"=>"ab3fc03bef51938f8f17a740bf9382af/2466497661788720302",
 "traceparent"=>"00-ab3fc03bef51938f8f17a740bf9382af-223ac28e2a3f8cae-00",
 "User-Agent"=>"http.rb/5.2.0",
 "Host"=>"headers.jsontest.com"}

Possible solution

Replace require 'yajl/json_gem' for require 'yajl' and implement the Yajl::Encoder.encode where .to_json is being used and Yajl::Parser.parse instead of the JSON.parse. That way we're not replacing the JSON object and give the rest of the applications the ability to opt in (or not) for using Yajl

nathansamson commented 6 months ago

Similar problem here. Where default JSON gem returns OpenStructs (although thats deprecated as well, but it still does it) yal doesn't

Just upgrading mailjet broke my application. Had to limit to version '< 1.7.8' to make it work.

mgrishko commented 6 months ago

Fixed in 1.7.9 Faraday HTTP client upgrade scheduled for next month