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

Already on GitHub? Sign in to your account

call #to_s on exception_message before calling #split #622

Merged
merged 2 commits into from May 18, 2012

Conversation

Projects
None yet
3 participants
Contributor

slyphon commented May 18, 2012

I figure I should mention this, even though i can't come up with an exact reproduction case (also, since it only seems to affect 1.8.7 it's probably just worth it to wait until EOL and say "sorry"), but...

When 1.8.7 is shutting down its threads, and you're stuck in a Monitor, it will eventually raise a SystemExit, and that SystemExit may have an instance of Exception as its #message (instead of a String) which causes the formatter to blow up.

I know this is the edgiest of edge cases (I seem to be on a streak this week of hitting all of these), but I hit this particular case, and thought "what the hell it's a one line patch"

@slyphon slyphon call #to_s on exception_message before calling #split
When 1.8.7 is shutting down its threads, and you're stuck in a Monitor,
it will eventually raise a SystemExit, and that SystemExit may have an
instance of Exception as its `#message` (instead of a String) which
causes the formatter to blow up.
2d39c46

This pull request passes (merged 2d39c46 into c9b8681).

Contributor

slyphon commented May 18, 2012

this causes the exception on 1.8.7-p358 w/ rspec 3.8.0 (same code exists in master)

      it %[should report the correct error?] do
        th = Thread.new do
          raise RuntimeError, nil
        end

        lambda do
          begin
            th.join(5)
          rescue
            $stderr.puts $!.inspect
            raise
          end
        end.should raise_error(RuntimeError)
      end

winds up failing the test and shows this at the end:

  1) ZK::Pool Simple force_close! should report the correct error?
     Failure/Error: raise RuntimeError, nil
     SystemExit:
/Users/slyphon/.rvm/gems/ruby-1.8.7-p358@zk/gems/rspec-core-2.8.0/lib/rspec/core/formatters/base_text_formatter.rb:177:in `dump_failure_info': private method `split' called for #<RuntimeError: RuntimeError> (NoMethodError)
    from /Users/slyphon/.rvm/gems/ruby-1.8.7-p358@zk/gems/rspec-core-2.8.0/lib/rspec/core/formatters/base_text_formatter.rb:170:in `dump_failure'
    from /Users/slyphon/.rvm/gems/ruby-1.8.7-p358@zk/gems/rspec-core-2.8.0/lib/rspec/core/formatters/base_text_formatter.rb:19:in `dump_failures'
    from /Users/slyphon/.rvm/gems/ruby-1.8.7-p358@zk/gems/backports-2.5.3/lib/backports/1.8.7/enumerable.rb:67:in `each_with_index'
    from /Users/slyphon/.rvm/gems/ruby-1.8.7-p358@zk/gems/backports-2.5.3/lib/backports/1.8.7/enumerable.rb:67:in `each'
    from /Users/slyphon/.rvm/gems/ruby-1.8.7-p358@zk/gems/backports-2.5.3/lib/backports/1.8.7/enumerable.rb:67:in `each_with_index'
    from /Users/slyphon/.rvm/gems/ruby-1.8.7-p358@zk/gems/rspec-core-2.8.0/lib/rspec/core/formatters/base_text_formatter.rb:17:in `dump_failures'
    from /Users/slyphon/.rvm/gems/ruby-1.8.7-p358@zk/gems/rspec-core-2.8.0/lib/rspec/core/reporter.rb:98:in `send'
    from /Users/slyphon/.rvm/gems/ruby-1.8.7-p358@zk/gems/rspec-core-2.8.0/lib/rspec/core/reporter.rb:98:in `notify'
    from /Users/slyphon/.rvm/gems/ruby-1.8.7-p358@zk/gems/rspec-core-2.8.0/lib/rspec/core/reporter.rb:97:in `each'
    from /Users/slyphon/.rvm/gems/ruby-1.8.7-p358@zk/gems/rspec-core-2.8.0/lib/rspec/core/reporter.rb:97:in `notify'
    from /Users/slyphon/.rvm/gems/ruby-1.8.7-p358@zk/gems/rspec-core-2.8.0/lib/rspec/core/reporter.rb:81:in `finish'
    from /Users/slyphon/.rvm/gems/ruby-1.8.7-p358@zk/gems/rspec-core-2.8.0/lib/rspec/core/reporter.rb:36:in `report'
    from /Users/slyphon/.rvm/gems/ruby-1.8.7-p358@zk/gems/rspec-core-2.8.0/lib/rspec/core/command_line.rb:25:in `run'
    from /Users/slyphon/.rvm/gems/ruby-1.8.7-p358@zk/gems/rspec-core-2.8.0/lib/rspec/core/runner.rb:80:in `run_in_process'
    from /Users/slyphon/.rvm/gems/ruby-1.8.7-p358@zk/gems/rspec-core-2.8.0/lib/rspec/core/runner.rb:69:in `run'
    from /Users/slyphon/.rvm/gems/ruby-1.8.7-p358@zk/gems/rspec-core-2.8.0/lib/rspec/core/runner.rb:10:in `autorun'
    from /Users/slyphon/.rvm/gems/ruby-1.8.7-p358@zk/bin/rspec:23
Owner

dchelimsky commented May 18, 2012

Be glad to merge this if you add an example that fails without this change.

Contributor

slyphon commented May 18, 2012

@dchelimsky ok, that spec acts as proof

@dchelimsky dchelimsky added a commit that referenced this pull request May 18, 2012

@dchelimsky dchelimsky Merge pull request #622 from slyphon/dump_failure_info_fix
call #to_s on exception_message before calling #split
0d26b06

@dchelimsky dchelimsky merged commit 0d26b06 into rspec:master May 18, 2012

@dchelimsky dchelimsky added a commit that referenced this pull request May 19, 2012

@dchelimsky dchelimsky Changelog for #622 [ci skip] 1482bfe

@dchelimsky dchelimsky added a commit that referenced this pull request May 19, 2012

@dchelimsky dchelimsky Merge pull request #622 from slyphon/dump_failure_info_fix
call #to_s on exception_message before calling #split
6d83a5e

@dchelimsky dchelimsky added a commit that referenced this pull request May 19, 2012

@dchelimsky dchelimsky Changelog for #622 [ci skip] 53454f3
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment