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

Rails makes Process::Status#to_json less useful #4857

Closed
sunaku opened this issue Feb 2, 2012 · 7 comments · Fixed by #7654
Closed

Rails makes Process::Status#to_json less useful #4857

sunaku opened this issue Feb 2, 2012 · 7 comments · Fixed by #7654

Comments

@sunaku
Copy link
Contributor

sunaku commented Feb 2, 2012

Hello,

Rails overrides Process::Status#to_json in a way that makes it less useful. For example, outside of Rails, Ruby 1.9.3 would report #<Process::Status: pid 21661 exit 0> but inside Rails test environment it would report {}. See the complete demonstration below.

Thanks for your consideration.

$ ruby -ve 'system "date"; p $?; require "json"; puts JSON.dump([$?])'
ruby 1.9.3p0 (2011-10-30 revision 33570) [x86_64-linux]
Thu Feb  2 12:49:17 PST 2012
#<Process::Status: pid 25469 exit 0>
["pid 25469 exit 0"]

$ rails new bogus && cd bogus
... output omitted ...

$ bundle exec rails runner 'system "date"; p $?; require "json"; puts JSON.dump([$?])'
Thu Feb  2 12:49:01 PST 2012
#<Process::Status: pid 25401 exit 0>
[{}]

$ bundle list
Gems included by the bundle:
  * actionmailer (3.2.0)
  * actionpack (3.2.0)
  * activemodel (3.2.0)
  * activerecord (3.2.0)
  * activeresource (3.2.0)
  * activesupport (3.2.0)
  * arel (3.0.0)
  * builder (3.0.0)
  * bundler (1.0.21)
  * coffee-rails (3.2.2)
  * coffee-script (2.2.0)
  * coffee-script-source (1.2.0)
  * erubis (2.7.0)
  * execjs (1.3.0)
  * hike (1.2.1)
  * i18n (0.6.0)
  * journey (1.0.1)
  * jquery-rails (2.0.0)
  * json (1.6.5)
  * mail (2.4.1)
  * mime-types (1.17.2)
  * multi_json (1.0.4)
  * polyglot (0.3.3)
  * rack (1.4.1)
  * rack-cache (1.1)
  * rack-ssl (1.3.2)
  * rack-test (0.6.1)
  * rails (3.2.0)
  * railties (3.2.0)
  * rake (0.9.2.2)
  * rdoc (3.12)
  * sass (3.1.12)
  * sass-rails (3.2.4)
  * sprockets (2.1.2)
  * sqlite3 (1.3.5)
  * thor (0.14.6)
  * tilt (1.3.3)
  * treetop (1.4.10)
  * tzinfo (0.3.31)
  * uglifier (1.2.3)
@isaacsanders
Copy link
Contributor

This issue seems to be related to this file: activesupport/lib/active_support/core_ext/object/to_json.rb

I am not sure how to fix this. The file is there for a reason, but breaking this functionality is not good either.

@steveklabnik
Copy link
Member

This illustrates (closer to) the root cause:

$ bundle exec rails runner 'system "date"; p $?; require "json"; puts ActiveSupport::JSON.encode($?)'        
Sat Sep 15 15:34:29 MSK 2012
#<Process::Status: pid 45367 exit 0>
{}

Which is defined here.

@steveklabnik
Copy link
Member

... which I think is caused by this:

$ bundle exec rails runner 'system "date"; p $?; require "json"; puts $?.instance_values'
Sat Sep 15 15:38:12 MSK 2012
#<Process::Status: pid 45417 exit 0>
{}

@steveklabnik
Copy link
Member

... which is caused by this:

$ bundle exec rails runner 'system "date"; p $?; require "json"; puts $?.instance_variables.empty?'
Sat Sep 15 15:39:43 MSK 2012
#<Process::Status: pid 45478 exit 0>
true

(instance_values defined here )

@steveklabnik
Copy link
Member

So, Process::Status does not have any instance variables. So I'm not sure what the proper fix is. Theoretically, we could introduce a new core ext that would make fake instance variables for Process::Status, but that sounds really, really, really, really sketchy.

@judofyr
Copy link

judofyr commented Sep 15, 2012

Why not write a specific as_json for Process::Status?

class Process::Status
  def as_json; { :exitstatus => exitstatus, :pid => pid } end
end

@steveklabnik
Copy link
Member

Ha, duh. I blame Russia.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants