-
Notifications
You must be signed in to change notification settings - Fork 37
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
TypeError when using json_pure with Chef #20
Comments
@xeron We only saw this issue in our build system - can you provide a full stacktrace of where you are seeing this error using Chef? |
I don't have stacktrace right now because we've also locked This error was from our internal cookbook which calls |
all that omnibus is doing is calling JSON.pretty_generate() with a hash. https://github.com/chef/omnibus/blob/master/lib/omnibus/project.rb#L1090 its getting redirected to json_pure because that gem patches all its own methods in for the JSON gem. most likely not a bug in this gem and instead a bug in the json_pure gem itself. |
I have a meeting to run to, but I have been able to reproduce this, and I think I have a few thoughts on how we can avoid this. Sorry for the suspense! |
@trevorrowe No worries - nothing like an exciting friday morning! 🎉 🚀 🎉 |
I put together a simple ruby script that reproduces the issue above. For this script to demonstrate the failure you need to be running Ruby 1.9.3, have jmespath 1.2.2 installed, and must not have version >= 1.8.1 of the json gem. require 'json'
require 'jmespath'
puts "ruby version: #{RUBY_VERSION}" # should be 1.9.3
puts "json version: #{JSON::VERSION}" # should be 1.5.5
puts "jmespath version: #{JMESPath::VERSION}" # should be 1.2.2
# this should raise a JSON::Pure::Generator::State TypeError
some_hash = { 'jsonrpc' => 'xxxx', 'jsonversion' => 1 }
some_hash.to_json As shown above, if your application has begin
# Attempt to load the native version if available, not availble
# by default for Ruby 1.9.3.
gem('json', '>= 1.8.1')
require 'json/ext'
rescue Gem::LoadError
# Fallback on the json_pure gem dependency.
gem('json_pure', '>= 1.8.1')
require 'json/pure'
end The first attempt will fail, because the A suitable workaround may be to have jmespath check to see if the JSON constant is defined first. If it is, it should not attempt to load json/ext or json/pure. This however may result in a version older than 1.8.1 being loaded. I suppose it could log a warning if this is the case. Having an older version will prevent it from correctly handling jmespath expressions that contains literal json values. |
I modified my local copy of jmespath with the following: if Object.const_defined?(:JSON) && JSON::VERSION < '1.8.1'
warn("WARNING: jmespath gem requires json gem >= 1.8.1; json #{JSON::VERSION} already loaded")
else
begin
gem('json', '>= 1.8.1')
require 'json/ext'
rescue Gem::LoadError
gem('json_pure', '>= 1.8.1')
require 'json/pure'
end
end The result of running jmespath with an older version of json prevents it from parsing json literals, for example: puts JMESPath.search('`1` > `2`', {})
#=> correct answer is false
#=> old version json, raises unexpected token 0 (JMESPath::Errors::SyntaxError) This essentially means users that have loaded an older version of json will have a degraded experience using jmespath. To be fair, this is the same situation users found themselves in between 1.1.3 and 1.2. Any thoughts on if the const defined check would be sufficient? |
The condition require has been merged in and released. This should hopefully prevents environments that have loaded json prior to loading jmespath from breaking json methods. |
This is a follow-up to #18 (comment)
We're seeing the following stacktrace when running Omnibus (another Chef product):
Omnibus is calling
JSON.pretty_generate
and passing it a hash. I realize this is probably a bug in json_pure (maybe json_pure#198 or json_pure#186) but we are currently downgrading ourjmespath
version to prevent pulling in json_pure.The text was updated successfully, but these errors were encountered: