stompgem / stomp

A ruby gem for sending and receiving messages from a Stomp protocol compliant message queue. Includes: failover logic, ssl support.
http://stomp.github.com
Apache License 2.0
152 stars 80 forks source link

1.3.0 breaks Ruby 1.8 compatibility with Timeout call #87

Closed david-mccullars closed 10 years ago

david-mccullars commented 10 years ago

With the addition of "Timeout.timeout(@start_timeout, Stomp::Error::StartTimeoutException.new(@start_timeout))" in lib/stomp/client.rb, Ruby 1.8 support is lost. The error received is "class or module required for rescue clause (TypeError)".

I did not see anywhere in the changelog where Ruby 1.8 support has officially been discontinued. If that was indeed intentional it would be really nice to add a note to the changelog so others won't fall into the same trap.

gmallard commented 10 years ago

On 10/16/2013 08:49 AM, David McCullars wrote:

With the addition of "Timeout.timeout(@start_timeout, Stomp::Error::StartTimeoutException.new(@start_timeout))" in lib/stomp/client.rb, Ruby 1.8 support is lost. The error received is "class or module required for rescue clause (TypeError)".

I did not see anywhere in the changelog where Ruby 1.8 support has officially been discontinued. If that was indeed intentional it would be really nice to add a note to the changelog so others won't fall into the same trap.

— Reply to this email directly or view it on GitHub https://github.com/stompgem/stomp/issues/87.

There is no intention to drop 1.8.x support at this time.

Can you add some information about your exact Ruby version (output of ruby -v)? And possibly supply a brief test that shows the problem? I can not recreate that here, with four different 1.8.x versions.

Thanks, Guy

david-mccullars commented 10 years ago

We are using ree 1.8.7:

ruby 1.8.7 (2012-02-08 MBARI 8/0x6770 on patchlevel 358) [x86_64-linux], MBARI 0x6770, Ruby Enterprise Edition 2012.02

To reproduce I can do:

[~] ➔ ruby -rtimeout -e "Timeout.timeout(1, StandardError.new) { sleep 10 }" /usr/local/rvm/rubies/ree-1.8.7-2012.02/lib/ruby/1.8/timeout.rb:69:in `timeout': class or module required for rescue clause (TypeError) from -e:1

However, that same line of code against 1.9.3p194 works:

[~] ➔ ruby -rtimeout -e "Timeout.timeout(1, StandardError.new) { sleep 10 }" -e:1:in sleep': execution expired (StandardError) from -e:1:inblock in

' from /usr/local/rvm/rubies/ruby-1.9.3-p194/lib/ruby/1.9.1/timeout.rb:68:in timeout' from -e:1:in
'

david-mccullars commented 10 years ago

I should also point out this works in REE 1.8.7:

[~] ➔ ruby -rtimeout -e "Timeout.timeout(1, StandardError) { sleep 10 }" /usr/local/rvm/rubies/ree-1.8.7-2012.02/lib/ruby/1.8/timeout.rb:64: execution expired (StandardError) from -e:1

david-mccullars commented 10 years ago

Here is another way of reproducing:

in ree 1.8.7

[~] ➔ ruby -e "p RUBY_VERSION; e = StandardError.new; begin; raise e; rescue StandardError; p :CAUGHT; end" "1.8.7" :CAUGHT

[~] ➔ ruby -e "p RUBY_VERSION; e = StandardError.new; begin; raise e; rescue e; p :CAUGHT; end" "1.8.7" -e:1: class or module required for rescue clause (TypeError)

in 1.9.3

[~] ➔ ruby -e "p RUBY_VERSION; e = StandardError.new; begin; raise e; rescue StandardError; p :CAUGHT; end" "1.9.3" :CAUGHT

[~] ➔ ruby -e "p RUBY_VERSION; e = StandardError.new; begin; raise e; rescue e; p :CAUGHT; end" "1.9.3" :CAUGHT

gmallard commented 10 years ago

REE specific.

From an ancient CentOS system, standard Ruby install:

ruby -v* ruby 1.8.5 (2006-08-25) [x86_64-linux] ruby -rtimeout -e "Timeout.timeout(1, StandardError.new) { sleep 10 }" /usr/lib/ruby/1.8/timeout.rb:54: execution expired (StandardError) \ from /usr/lib/ruby/1.8/timeout.rb:56:in `timeout' \ from -e:1* Are you willing to experiment?

Meanwhile, I will try to get REE installed somewhere, and experiment with that.

gmallard commented 10 years ago

Bizarre.

Well, after investigation, not REE specific. Even with a REE install all the gem unit tests pass! I do not understand that yet, because ....

Your example (which is totally not related to the gem) of:

ruby -rtimeout -e "Timeout.timeout(1, StandardError.new) { sleep 10 }"

is an entirely different story.

For that example, I see:

Issue status: thinking.

For the record my (Ubuntu) systems, built from Ruby repo, are:

1) 185_231 (2008-06-20 patchlevel 231) 2) 186_383 (2009-08-04 patchlevel 383) 3) 187_000 1.8.7 (2008-04-15 patchlevel 0) 4) 187_299 (2010-06-23 patchlevel 299) 5) 191_378 1.9.1p378 (2010-01-10) 6) 192_188 1.9.2p188 (2011-03-28 revision 31204) 7) 193_134 1.9.3p134 (2012-02-19 revision 34690) 8) ruby 1.9.2p290 (2011-07-04 revision 32477) 9) ruby 1.9.1p431 (2011-08-25 revision 26781) LOCALMODS a) ruby 1.9.3p358 (2012-12-22 revision 38541) b) ruby 2.0.0p0 (2013-02-24 revision 39473) c) ruby 2.1.0dev (2013-03-23 trunk 39878) d) ruby 2.1.0dev (2013-05-05 trunk 40584) e) ruby 2.0.0p195 (2013-05-14 revision 40731) f) ruby 2.0.0p247 (2013-06-27 revision 41672)

Ref:

https://gist.github.com/gmallard/7034128

On 10/17/2013 10:25 AM, Guy M. Allard wrote:

REE specific.

From an ancient CentOS system, standard Ruby install:

ruby -v* ruby 1.8.5 (2006-08-25) [x86_64-linux] ruby -rtimeout -e "Timeout.timeout(1, StandardError.new) { sleep 10 }" /usr/lib/ruby/1.8/timeout.rb:54: execution expired (StandardError) \ from /usr/lib/ruby/1.8/timeout.rb:56:in `timeout' \ from -e:1* Are you willing to experiment?

Meanwhile, I will try to get REE installed somewhere, and experiment with that.

david-mccullars commented 10 years ago

I certainly appreciate your thoroughness in looking at this -- thanks!

The reason I posted that piece of seemingly unrelated code was line 87 of lib/stomp/client.rb. From 1.2.16 to 1.3, the line "Timeout.timeout(@start_timeout, Stomp::Error::StartTimeoutException.new(@start_timeout))" was added, and that uses the Timeout.timeout(n, some_exception_instance) syntax as opposed to Timeout.timeout(n, some_exception_class). I thought by pointing out the difference in those two syntaxes in REE 1.8 (at least for me) it might expedite your troubleshooting.

gmallard commented 10 years ago

There is a fix for this on my personal github clone and a pull request created for comments.

You could clone my personal repo:

https://github.com/gmallard/stomp.git

and build a gem from the 'dev' branch if you want to test.

gmallard commented 10 years ago

Status: Closed Ref: bfa882a Gem Version: 1.3.2

If the issue persists please reopen this issue or create a new issue.