Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Psych can't load a dumped Exception that contains a NameError::message class #85

Closed
Fryguy opened this issue Sep 29, 2012 · 5 comments
Closed

Comments

@Fryguy
Copy link

Fryguy commented Sep 29, 2012

Psych can't load a dumped Exception that contains a NameError::message class. Here is a test showing this.

def test_loading_exception_with_name_error_message
  err = String.xxx rescue $!
  assert_nothing_raised { YAML.load(YAML.dump(err)) }
end

This fails with "TypeError: allocator undefined for NameError::message"

NoMethodError's internal message is not a String but a special class called NameError::message. The message string itself is delayed until called. Once .message is called, the internal NameError::message goes away and you're left with a String. If you try to dump/load a NoMethodError after .message (or .inspect, or .to_s) has been called, then it will work properly. The way to make it fail is to call it without having called .message, as the test above does. Note that if you try the above in irb, as separate lines, it will appear to work, because irb calls .inspect after the first line, and thus .message is called.

When dumping the NoMethodError without having called .message (like the test), the dump looks like this:

--- !ruby/exception:NoMethodError
message: !ruby/object:NameError::message {}

When dumping the NoMethodError after having called .message, the dump looks like this:

--- !ruby/exception:NoMethodError
message: !binary |-
  dW5kZWZpbmVkIG1ldGhvZCBgeHh4JyBmb3IgU3RyaW5nOkNsYXNz

I never knew about NameError::message, but I learned a lot about it from this link: http://www.carboni.ca/blog/p/Ruby-Did-You-Know-That-5-NameErrormessage

@mdesantis
Copy link

The page is not accessible at the moment; you can find it here

@lulalala
Copy link

As a reference: I think I am using libyaml, and I have the same issue. The only difference is that calling ex.message prior to dumping won't fix the dumping.

@JoshCheek
Copy link

Calling NameError#message is not fixing the issue for me. This is a pretty serious problem for me, it means I can't use YAML to serialize my exceptions, but I can't use Marshal to serialize them either (blows up on Windows for some reason I haven't figured out yet), and this is a very common use-case for my gem. I've already swapped the serialization code three times (YAML -> Marshal -> YAML -> back to Marshal, b/c I'd rather have it break on Windows than not be able to handle this case).

@dreammaker
Copy link

Calling #message or #to_s definitely used to work. Now it doesn't. Before, the formatting of the message was cached. A recent patch level must have changed this behavior. I've confirmed that in MRI 1.9.3p194, calling #message works. I'm not sure which patch level introduced the change, but it does not work in 1.9.3-p362. Does anyone know another workaround for this that works in more recent patch levels?

For now, I'm probably going to resort to something extremely hacky like using a regex to see if the YAML dump contains !ruby/object:NameError::message. This only works for me b/c I'm using it just for error reporting.

@beccadax
Copy link

In my Rails app, I've added a method to Psych that special-cases the encoding of NameErrors:

module Psych
  module Visitors
    class YAMLTree < Psych::Visitors::Visitor
      def visit_NameError o
        tag = ['!ruby/exception', o.class.name].join ':'

        @emitter.start_mapping nil, tag, false, Nodes::Mapping::BLOCK

        {
          'message'   => o.message.to_s,
          'backtrace' => private_iv_get(o, 'backtrace'),
        }.each do |k,v|
          next unless v
          @emitter.scalar k, nil, nil, true, false, Nodes::Scalar::ANY
          accept v
        end

        dump_ivars o

        @emitter.end_mapping
      end

    end
  end
end

This could be done more cleanly in Psych itself (a single to_s call at lib/psych/visitors/yaml_tree.rb:199 would do the trick, although you might prefer something more surgical), but I didn't want to modify Psych.

tenderlove added a commit that referenced this issue Jan 8, 2015
* master: (21 commits)
  * ext/psych/lib/psych/visitors/to_ruby.rb: call `allocate` on hash subclasses.  Fixes github.com//issues/196
  * ext/psych/lib/psych/visitors/to_ruby.rb: revive hashes with ivars
  removed isolate task
  removed isolate plugin
  added minitest dependency into gemspec
  added install task into travis
  added ruby-head env
  bumping version to 2.0.8
  fixed build error caused by trunk changes
  bumping version to 2.0.7
  merging from ruby trunk
  backport r48512 from ruby/ruby trunk.
  Add changelog for 2a4d956
  backport r48214 from ruby/ruby trunk.
  Allow dumping any BasicObject that defines #marshal_dump or #marshal_load
  bumping version
  * ext/psych/lib/psych/visitors/yaml_tree.rb: fix NameError dumping and   loading. Fixes GH #85. Thanks @brentdax for the patch! * test/psych/test_exception.rb: test for fix
  * ext/psych/lib/psych/scalar_scanner.rb: fix loading strings that   look like integers but have a newline. Fixes GH #189 * test/psych/test_string.rb: test for fix
  * ext/psych/lib/psych/visitors/to_ruby.rb: merge keys with a hash   should merge the hash in to the parent. * test/psych/test_merge_keys.rb: test for change. Fixes GH #202
  * ext/psych/lib/psych/visitors/to_ruby.rb: quoted "<<" strings   should not be treated as merge keys. * ext/psych/lib/psych/visitors/yaml_tree.rb: hashes with keys   containing "<<" should roundtrip. * test/psych/test_merge_keys.rb: test for change. Fixes GH #203
  ...

Conflicts:
	lib/psych/visitors/yaml_tree.rb
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Development

No branches or pull requests

6 participants