Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Exception dumping depends on MRI-specific internals #103

Open
headius opened this Issue · 7 comments

3 participants

@headius
Collaborator

The following method depends on MRI-specific internal variables in Exception objects, and as a result it doesn't marshal exceptions properly in JRuby:

      def visit_Exception o
        tag = ['!ruby/exception', o.class.name].join ':'

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

        {
          'message'   => private_iv_get(o, 'mesg'),
          '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

I'll see if I can come up with a patch.

@headius
Collaborator

Patch...this lets test_yaml_tree.rb pass (test_exception was failing with the original code).

diff --git a/lib/ruby/1.9/psych/visitors/yaml_tree.rb b/lib/ruby/1.9/psych/visitors/yaml_tree.rb
index d420abd..3f615f3 100644
--- a/lib/ruby/1.9/psych/visitors/yaml_tree.rb
+++ b/lib/ruby/1.9/psych/visitors/yaml_tree.rb
@@ -147,8 +147,8 @@ module Psych
         @emitter.start_mapping nil, tag, false, Nodes::Mapping::BLOCK

         {
-          'message'   => private_iv_get(o, 'mesg'),
-          'backtrace' => private_iv_get(o, 'backtrace'),
+          'message'   => private_iv_get(o, 'mesg') || o.message,
+          'backtrace' => private_iv_get(o, 'backtrace' || o.backtrace),
         }.each do |k,v|
           next unless v
           @emitter.scalar k, nil, nil, true, false, Nodes::Scalar::ANY
@headius
Collaborator

private_iv_get is pretty gross, btw, but so is MRI's use of non-ivar ivars :-)

@Fryguy

@headius, from my use cases, I'm not sure Psych is dumping/loading Exception objects properly for MRI either. See issues #85 and #86 for things I've run into.

@tenderlove
Owner

@headius Are you sure this patch is correct? The line in the patch that calls o.backtrace seems broken.

@headius
Collaborator

Updated patch, but we're still discussing...

diff --git a/lib/psych/visitors/yaml_tree.rb b/lib/psych/visitors/yaml_tree.rb
index ce40a17..1d01310 100644
--- a/lib/psych/visitors/yaml_tree.rb
+++ b/lib/psych/visitors/yaml_tree.rb
@@ -147,8 +147,8 @@ module Psych
         @emitter.start_mapping nil, tag, false, Nodes::Mapping::BLOCK

         {
-          'message'   => private_iv_get(o, 'mesg'),
-          'backtrace' => private_iv_get(o, 'backtrace'),
+          'message'   => private_iv_get(o, 'mesg') || o.message,
+          'backtrace' => private_iv_get(o, 'backtrace') || o.backtrace,
         }.each do |k,v|
           next unless v
           @emitter.scalar k, nil, nil, true, false, Nodes::Scalar::ANY
@tenderlove
Owner

@headius how about this:

diff --git a/src/org/jruby/ext/psych/PsychYamlTree.java b/src/org/jruby/ext/psych/PsychYamlTree.java
index 7b2c72e..515a0ef 100644
--- a/src/org/jruby/ext/psych/PsychYamlTree.java
+++ b/src/org/jruby/ext/psych/PsychYamlTree.java
@@ -47,9 +47,6 @@ public class PsychYamlTree {

     @JRubyMethod(visibility = PRIVATE)
     public static IRubyObject private_iv_get(ThreadContext context, IRubyObject self, IRubyObject target, IRubyObject prop) {
-        IRubyObject obj = target.getInstanceVariables().getInstanceVariable(prop.asJavaString());
-        if (obj == null) obj = context.nil;
-
-        return obj;
+        return target.callMethod(context, prop.asJavaString());
     }
 }
@headius
Collaborator

But the method to get the internal variable "mesg" is "message". This won't fix that case.

It also seems a little special-casey. private_iv_get is doing what it's supposed to, but we don't store exception data in those variables.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.