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

Load Oj instead of JSON. #300

Merged
merged 2 commits into from
Sep 16, 2015
Merged

Load Oj instead of JSON. #300

merged 2 commits into from
Sep 16, 2015

Conversation

jondeandres
Copy link
Contributor

No description provided.

@@ -796,14 +796,7 @@ def require_core_extensions
end

def monkey_patch_socket?
return false unless defined?(ActiveSupport::VERSION::STRING)
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Testing failed with active support 4.2. So just monkey patch always active support is loaded.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am just curious how this change is related with this issue

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@PetrKaleta ActiveSupport has some bugs when calling #to_json or #as_json. Most of the JSON serialization libraries calls somewhere #as_json if it's defined.

We've experienced many customers that in their payloads, for some reason, have objects that internally have an instance of BasicSocket. ActiveSupport then arrives to an infinite loop trying to execute #as_json. This caused by the monkey patch they add to Object.

And in this PR we realized we were having problems with all the ActiveSupport versions.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for explanation, makes sense

jondeandres added a commit that referenced this pull request Sep 16, 2015
Load Oj instead of JSON.
@jondeandres jondeandres merged commit ff750eb into master Sep 16, 2015
@jondeandres jondeandres deleted the oj branch September 16, 2015 21:45
@PetrKaleta
Copy link

You can't use Oj as its C extension gem! So what about JRuby omg

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