Explicitly catch NoMethodError instead of catching with Object when there are no events in queue

[lamroger]

This can be replicated by loading a watch, stopping it, and removing it.

The logs look like this

I [2015-03-23 08:17:03]  INFO: simple-app-0 sent SIGKILL
I [2015-03-23 08:17:03]  INFO: simple-app-0 process stopped
I [2015-03-23 08:17:03]  INFO: simple-app-0 move 'up' to 'unmonitored'
I [2015-03-23 08:17:03]  INFO: simple-app-0 deregistered 'proc_exit' event for pid 4907
I [2015-03-23 08:17:03]  INFO: simple-app-0 moved 'up' to 'unmonitored'
F [2015-03-23 08:17:06] FATAL: Unhandled exception in driver loop - (NoMethodError): undefined         method `handle_event' for nil:NilClass
/var/lib/gems/1.9.1/gems/god-0.13.5/lib/god/driver.rb:181:in `block (2 levels) in initialize'
/var/lib/gems/1.9.1/gems/god-0.13.5/lib/god/driver.rb:179:in `loop'
/var/lib/gems/1.9.1/gems/god-0.13.5/lib/god/driver.rb:179:in `block in initialize'
I [2015-03-23 08:17:06]  INFO: simple-app-0 unwatched

The watch is a simple watch similar to the example on the homepage.

The test fails if you comment out the Object catch.

[eric]

Instead of catching that exception, the code should handle the condition directly so it doesn't generate an exception.

I'm not sure if we should ever be seeing a nil in the event queue. Do you see how it gets there?

[lamroger]

The events are stored in an array and are popped with the shift function. The ruby array's pop function also returns nil if an array is empty so it sounds reasonable.

I can modify it to check if it's nil before calling handle_event.

On Monday, March 23, 2015, Eric Lindvall [email protected] wrote:

Instead of catching that exception, the code should handle the condition directly so it doesn't generate an exception.

I'm not sure if we should ever be seeing a nil in the event queue. Do you see how it gets there?

— Reply to this email directly or view it on GitHub https://github.com/mojombo/god/pull/215#issuecomment-85125432.

[eric]

It looks like the goal of DriverEventQueue#pop is to only return something valid.

I think that another raise ThreadError, "queue empty" if @shutdown needs to be added right before the @events.shift call.

[lamroger]

I think the problem still occurs when DriverEventQueue#pop gets called and @shutdown is false. It never raises an exception.

I don't quite understand why we @resource.wait if it's not shutting down. Is it because we want to give our queue another chance to get populated?