better error handling

git-svn-id: svn://rubyforge.org/var/svn/sup/trunk@278 5c8cc53c-5e98-4d25-b20a-d8db53a31250

diff --git a/lib/sup/mbox/ssh-file.rb b/lib/sup/mbox/ssh-file.rb
index aa31b4e5a5fc6bde94999b4cedfb2d3de4156944..c5673cb8fbd493cc32ac97d926696de6f4f107d8 100644 (file)
--- a/lib/sup/mbox/ssh-file.rb
+++ b/lib/sup/mbox/ssh-file.rb
@@ -92,6 +92,9 @@ class SSHFile
   REASONABLE_TRANSFER_SIZE = 1024 * 32
   SIZE_CHECK_INTERVAL = 60 * 1 # seconds
 
+  ## upon these errors we'll try to rereconnect a few times
+  RECOVERABLE_ERRORS = [ Errno::EPIPE, Errno::ETIMEDOUT ]
+
   @@shells = {}
   @@shells_mutex = Mutex.new
 
@@ -105,46 +108,15 @@ class SSHFile
     @say_id = nil
     @broken_msg = nil
     @shell = nil
-    @shell_mutex = Mutex.new
+    @shell_mutex = nil
     @buf_mutex = Mutex.new
   end
 
-  def to_s; "mbox+ssh://#@host/#@fn"; end ## TODO: remove thisis EVILness
+  def to_s; "mbox+ssh://#@host/#@fn"; end ## TODO: remove this EVILness
   def broken?; !@broken_msg.nil?; end
 
-  ## TODO: share this code with imap
-  def say s
-    @say_id = BufferManager.say s, @say_id if BufferManager.instantiated?
-    Redwood::log s
-  end
-  def shutup
-    BufferManager.clear @say_id if BufferManager.instantiated? && @say_id
-    @say_id = nil
-  end
-  private :say, :shutup
-
   def connect
-    raise SSHFileError, @broken_msg if broken?
-    return if @shell
-
-    @key = [@host, @ssh_opts[:username]]
-    begin
-      @shell = @@shells_mutex.synchronize do
-        unless @@shells.member? @key
-          say "Opening SSH connection to #{@host} for #@fn..."
-          #raise SSHFileError, "simulated SSH file error"
-          session = Net::SSH.start @host, @ssh_opts
-          say "Starting SSH shell..."
-          @@shells[@key] = session.shell.sync
-        end
-        @@shells[@key]
-      end
-
-      say "Checking for #@fn..."
-      @shell_mutex.synchronize { raise Errno::ENOENT, @fn unless @shell.test("-e #@fn").status == 0 }
-    ensure
-      shutup
-    end
+    do_remote nil
   end
 
   def eof?; @offset >= size; end
@@ -180,32 +152,72 @@ class SSHFile
 
 private
 
+  ## TODO: share this code with imap
+  def say s
+    @say_id = BufferManager.say "[#{@say_id.inspect}] #{s}", @say_id if BufferManager.instantiated?
+    Redwood::log s
+  end
+
+  def shutup
+    BufferManager.clear @say_id if BufferManager.instantiated? && @say_id
+    @say_id = nil
+  end
+
+  def unsafe_connect
+    raise SSHFileError, @broken_msg if broken?
+    return if @shell
+
+    @key = [@host, @ssh_opts[:username]]
+    begin
+      @shell, @shell_mutex = @@shells_mutex.synchronize do
+        unless @@shells.member? @key
+          say "Opening SSH connection to #{@host} for #@fn..."
+          #raise SSHFileError, "simulated SSH file error"
+          session = Net::SSH.start @host, @ssh_opts
+          say "Starting SSH shell..."
+          @@shells[@key] = [session.shell.sync, Mutex.new]
+        end
+        @@shells[@key]
+      end
+
+      say "Checking for #@fn..."
+      @shell_mutex.synchronize { raise Errno::ENOENT, @fn unless @shell.test("-e #@fn").status == 0 }
+    ensure
+      say "Not checking for #@fn any more"
+      shutup
+    end
+  end
+
   def do_remote cmd, expected_size=0
+    retries = 0
+    result = nil
     begin
-      retries = 0
-      connect
-      # MBox::debug "sending command: #{cmd.inspect}"
       begin
-        result = @shell_mutex.synchronize { x = @shell.send_command cmd; sleep 0.25; x }
-        raise SSHFileError, "Failure during remote command #{cmd.inspect}: #{(result.stderr || result.stdout || "")[0 .. 100]}" unless result.status == 0
-      rescue Net::SSH::Exception # these happen occasionally for no apparent reason. gotta love that nondeterminism!
-        retry if (retries += 1) <= 3
-        raise
-      rescue Errno::EPIPE
-        if (retries += 1) <= e
+        unsafe_connect
+        if cmd
+          # MBox::debug "sending command: #{cmd.inspect}"
+          result = @shell_mutex.synchronize { x = @shell.send_command cmd; sleep 0.25; x }
+          raise SSHFileError, "Failure during remote command #{cmd.inspect}: #{(result.stderr || result.stdout || "")[0 .. 100]}" unless result.status == 0
+        end
+
+        ## Net::SSH::Exceptions seem to happen every once in a while for
+        ## no good reason.
+      rescue Net::SSH::Exception, *RECOVERABLE_ERRORS
+        if (retries += 1) <= 3
           @@shells_mutex.synchronize do
             @shell = nil
             @@shells[@key] = nil
           end
-          connect
           retry
         end
+        raise
       end
-    rescue Net::SSH::Exception, SSHFileError, Errno::ENOENT => e
+    rescue Net::SSH::Exception, SSHFileError, SystemCallError => e
       @broken_msg = e.message
       raise
     end
-    result.stdout
+
+    result.stdout if cmd
   end
 
   def get_bytes offset, size

diff --git a/lib/sup/mbox/ssh-loader.rb b/lib/sup/mbox/ssh-loader.rb
index 33aac121f7cb32788ee2df9743e3cde197ac3a40..a3a8964479a98e87546e9487e6b9af61b5829085 100644 (file)
--- a/lib/sup/mbox/ssh-loader.rb
+++ b/lib/sup/mbox/ssh-loader.rb
@@ -36,26 +36,21 @@ class SSHLoader < Source
     @labels << File.basename(filename).intern unless File.dirname(filename) =~ /\b(var|usr|spool)\b/
   end
 
+  def connect; safely { @f.connect }; end
   def host; @parsed_uri.host; end
   def filename; @parsed_uri.path[1..-1] end
 
   def next
     return if broken?
-    begin
+    safely do
       offset, labels = @loader.next
       self.cur_offset = @loader.cur_offset # superclass keeps @cur_offset which is used by yaml
       [offset, (labels + @labels).uniq] # add our labels
-    rescue Net::SSH::Exception, SocketError, SSHFileError, Errno::ENOENT => e
-      recover_from e
     end
   end
 
   def end_offset
-    begin
-      @f.size
-    rescue Net::SSH::Exception, SocketError, SSHFileError, Errno::ENOENT => e
-      recover_from e
-    end
+    safely { @f.size }
   end
 
   def cur_offset= o; @cur_offset = @loader.cur_offset = o; @dirty = true; end
@@ -64,21 +59,19 @@ class SSHLoader < Source
   # def cur_offset; @loader.cur_offset; end # think we'll be ok without this
   def to_s; @parsed_uri.to_s; end
 
-  def recover_from e
-    m = "error communicating with SSH server #{host} (#{e.class.name}): #{e.message}"
-    Redwood::log m
-    self.broken_msg = @loader.broken_msg = m
-    raise SourceError, m
+  def safely
+    begin
+      yield
+    rescue Net::SSH::Exception, SocketError, SSHFileError, SystemCallError => e
+      m = "error communicating with SSH server #{host} (#{e.class.name}): #{e.message}"
+      Redwood::log m
+      self.broken_msg = @loader.broken_msg = m
+      raise SourceError, m
+    end
   end
 
   [:start_offset, :load_header, :load_message, :raw_header, :raw_full_message].each do |meth|
-    define_method meth do |*a|
-      begin
-        @loader.send meth, *a
-      rescue Net::SSH::Exception, SocketError, SSHFileError, Errno::ENOENT => e
-        recover_from e
-      end
-    end
+    define_method(meth) { |*a| safely { @loader.send meth, *a } }
   end
 end