Issue #533: rescue StandardError only

lib/listen/thread.rb

@@ -24,7 +24,7 @@ def new(name, &block)
 
       def rescue_and_log(method_name, *args, caller_stack: nil)
         yield(*args)
-      rescue Exception => exception # rubocop:disable Lint/RescueException
+      rescue => exception
         _log_exception(exception, method_name, caller_stack: caller_stack)
       end
 

listen.gemspec

@@ -5,7 +5,7 @@ $LOAD_PATH.unshift(lib) unless $LOAD_PATH.include?(lib)
 
 require 'listen/version'
 
-Gem::Specification.new do |gem|
+Gem::Specification.new do |gem| # rubocop:disable Metrics/BlockLength
   gem.name        = 'listen'
   gem.version     = Listen::VERSION
   gem.license     = 'MIT'

spec/lib/listen/adapter/linux_spec.rb

@@ -1,7 +1,7 @@
 # frozen_string_literal: true
 
 RSpec.describe Listen::Adapter::Linux do
-  describe 'class' do
+  describe 'class methods' do
     subject { described_class }
 
     if linux?
@@ -12,34 +12,44 @@
   end
 
   if linux?
+  describe 'instance methods' do
     before(:all) do
       require 'rb-inotify'
     end
+
     let(:dir1) { Pathname.new("/foo/dir1") }
 
-    let(:config) { instance_double(Listen::Adapter::Config, "config") }
-    let(:queue) { instance_double(Queue, "queue") }
+    let(:queue) { instance_double(Queue, "queue", close: nil) }
+    let(:config) { instance_double(Listen::Adapter::Config, "config", queue: queue) }
     let(:silencer) { instance_double(Listen::Silencer, "silencer") }
     let(:snapshot) { instance_double(Listen::Change, "snapshot") }
     let(:record) { instance_double(Listen::Record, "record") }
 
     # TODO: fix other adapters too!
     subject { described_class.new(config) }
 
+    after do
+      subject.stop
+    end
+
     describe 'watch events' do
       let(:directories) { [Pathname.pwd] }
       let(:adapter_options) { {} }
       let(:default_events) { [:recursive, :attrib, :create, :modify, :delete, :move, :close_write] }
-      let(:fake_worker) { double(:fake_worker) }
+      let(:fake_worker) { double(:fake_worker_for_watch_events) }
       let(:fake_notifier) { double(:fake_notifier, new: fake_worker) }
 
       before do
         stub_const('INotify::Notifier', fake_notifier)
 
         allow(config).to receive(:directories).and_return(directories)
         allow(config).to receive(:adapter_options).and_return(adapter_options)
-        allow(config).to receive(:queue).and_return(queue)
         allow(config).to receive(:silencer).and_return(silencer)
+        allow(fake_worker).to receive(:close)
+      end
+
+      after do
+        subject.stop
       end
 
       it 'starts by calling watch with default events' do
@@ -53,8 +63,9 @@
       let(:adapter_options) { {} }
 
       before do
-        fake_worker = double(:fake_worker)
+        fake_worker = double(:fake_worker_for_inotify_limit_message)
         allow(fake_worker).to receive(:watch).and_raise(Errno::ENOSPC)
+        allow(fake_worker).to receive(:close)
 
         fake_notifier = double(:fake_notifier, new: fake_worker)
         stub_const('INotify::Notifier', fake_notifier)
@@ -75,16 +86,16 @@
       let(:adapter_options) { { events: [:recursive, :close_write] } }
 
       before do
-        fake_worker = double(:fake_worker)
+        fake_worker = double(:fake_worker_for_callback)
         events = [:recursive, :close_write]
         allow(fake_worker).to receive(:watch).with('/foo/dir1', *events)
+        allow(fake_worker).to receive(:close)
 
         fake_notifier = double(:fake_notifier, new: fake_worker)
         stub_const('INotify::Notifier', fake_notifier)
 
         allow(config).to receive(:directories).and_return(directories)
         allow(config).to receive(:adapter_options).and_return(adapter_options)
-        allow(config).to receive(:queue).and_return(queue)
         allow(config).to receive(:silencer).and_return(silencer)
 
         allow(Listen::Record).to receive(:new).with(dir1).and_return(record)
@@ -142,7 +153,7 @@
     end
 
     describe '#stop' do
-      let(:fake_worker) { double(:fake_worker, close: true) }
+      let(:fake_worker) { double(:fake_worker_for_stop, close: true) }
       let(:directories) { [dir1] }
       let(:adapter_options) { { events: [:recursive, :close_write] } }
 
@@ -159,23 +170,19 @@
           fake_notifier = double(:fake_notifier, new: fake_worker)
           stub_const('INotify::Notifier', fake_notifier)
 
-          allow(config).to receive(:queue).and_return(queue)
-          allow(queue).to receive(:close)
           allow(config).to receive(:silencer).and_return(silencer)
 
           allow(subject).to receive(:require).with('rb-inotify')
           subject.configure
         end
 
         it 'stops the worker' do
-          expect(fake_worker).to receive(:close)
           subject.stop
         end
       end
 
       context 'when not even initialized' do
         before do
-          allow(config).to receive(:queue).and_return(queue)
           allow(queue).to receive(:close)
         end
 
@@ -187,4 +194,5 @@
       end
     end
   end
+  end
 end

spec/lib/listen/event/processor_spec.rb

@@ -224,20 +224,20 @@ def status_for_time(time)
   end
 
   describe '_process_changes' do
-    context 'when it raises an exception derived from StandardError or not' do
+    context 'when it raises an exception derived from StandardError' do
       before do
         allow(event_queue).to receive(:empty?).and_return(true)
         allow(config).to receive(:callable?).and_return(true)
         resulting_changes = { modified: ['foo'], added: [], removed: [] }
         allow(config).to receive(:optimize_changes).with(anything).and_return(resulting_changes)
         expect(config).to receive(:call).and_raise(ArgumentError, "bang!")
         expect(config).to receive(:call).and_return(nil)
-        expect(config).to receive(:call).and_raise(ScriptError, "ruby typo!")
+        expect(config).to receive(:call).and_raise("error!")
       end
 
       it 'rescues and logs exception and continues' do
         expect(Listen.logger).to receive(:error).with(/Exception rescued in _process_changes:\nArgumentError: bang!/)
-        expect(Listen.logger).to receive(:error).with(/Exception rescued in _process_changes:\nScriptError: ruby typo!/)
+        expect(Listen.logger).to receive(:error).with(/Exception rescued in _process_changes:\nRuntimeError: error!/)
         expect(Listen.logger).to receive(:debug).with(/Callback \(exception\) took/)
         expect(Listen.logger).to receive(:debug).with(/Callback took/)
         expect(Listen.logger).to receive(:debug).with(/Callback \(exception\) took/)

spec/lib/listen/thread_spec.rb

@@ -63,6 +63,32 @@
       end
     end
 
+    class TestExceptionDerivedFromException < Exception; end # rubocop:disable Lint/InheritException
+
+    context "when exception raised that is not derived from StandardError" do
+      [SystemExit, SystemStackError, NoMemoryError, SecurityError, TestExceptionDerivedFromException].each do |exception|
+        context exception.name do
+          let(:block) do
+            -> { raise exception, 'boom!' }
+          end
+
+          it "does not rescue" do
+            expect(Thread).to receive(:new) do |&block|
+              expect do
+                block.call
+              end.to raise_exception(exception, 'boom!')
+
+              thread = instance_double(Thread, "thread")
+              allow(thread).to receive(:name=).with(any_args)
+              thread
+            end
+
+            subject
+          end
+        end
+      end
+    end
+
     context "when nested exceptions raised" do
       let(:block) { raise_nested_exception_block }
 
@@ -78,15 +104,6 @@
         subject.join
       end
     end
-
-    context 'when exception raised that is not derived from StandardError' do
-      let(:block) { raise_script_error_block }
-
-      it "still rescues and logs" do
-        expect(Listen.logger).to receive(:error).with(/Exception rescued in listen-worker_thread:\nScriptError: ruby typo!/)
-        subject.join
-      end
-    end
   end
 
   describe '.rescue_and_log' do
@@ -106,9 +123,10 @@
     context 'when exception raised that is not derived from StandardError' do
       let(:block) { raise_script_error_block }
 
-      it 'still rescues and logs' do
-        expect(Listen.logger).to receive(:error).with(/Exception rescued in method:\nScriptError: ruby typo!/)
-        described_class.rescue_and_log("method", &block)
+      it "raises out" do
+        expect do
+          described_class.rescue_and_log("method", &raise_script_error_block)
+        end.to raise_exception(ScriptError, "ruby typo!")
       end
     end
   end