]> git.cworth.org Git - sup/commitdiff
improved source error handling
authorwmorgan <wmorgan@5c8cc53c-5e98-4d25-b20a-d8db53a31250>
Sun, 1 Apr 2007 20:22:38 +0000 (20:22 +0000)
committerwmorgan <wmorgan@5c8cc53c-5e98-4d25-b20a-d8db53a31250>
Sun, 1 Apr 2007 20:22:38 +0000 (20:22 +0000)
git-svn-id: svn://rubyforge.org/var/svn/sup/trunk@357 5c8cc53c-5e98-4d25-b20a-d8db53a31250

bin/sup
lib/sup.rb
lib/sup/imap.rb
lib/sup/index.rb
lib/sup/maildir.rb
lib/sup/mbox/loader.rb
lib/sup/message.rb
lib/sup/poll.rb
lib/sup/source.rb
lib/sup/util.rb

diff --git a/bin/sup b/bin/sup
index 00f0b3d352668d174d126907d9d96b88adadc1a5..dc1f2f9478386e41c904cb75421d4e73f52c81e9 100644 (file)
--- a/bin/sup
+++ b/bin/sup
@@ -105,6 +105,11 @@ begin
 
   bm.draw_screen
 
+  begin
+    Index.usual_sources.each { |s| s.check }
+  rescue SourceError
+    # do nothing! we'll report it at the next step
+  end
   Redwood::report_broken_sources
   
   Index.usual_sources.each do |s|
index cf46b8de7b1153f79d36acc7ce14c998e1bebbe5..9be9a9e911ffa7d09ef4ebc80e49efd01b58b09c 100644 (file)
@@ -98,21 +98,44 @@ module Redwood
   ## not really a good place for this, so I'll just dump it here.
   def report_broken_sources
     return unless BufferManager.instantiated?
-    broken_sources = Index.usual_sources.select { |s| s.broken? }
+
+    broken_sources = Index.usual_sources.select { |s| s.error.is_a? FatalSourceError }
     unless broken_sources.empty?
-      BufferManager.spawn "Out-of-sync soure notification", TextMode.new(<<EOM)
+      BufferManager.spawn "Broken source notification", TextMode.new(<<EOM)
+Source error notification
+-------------------------
+
+Hi there. It looks like one or more message sources is reporting
+errors. Until this is corrected, messages from these sources cannot
+be viewed, and new messages will not be detected.
+
+#{broken_sources.map { |s| "Source: " + s.to_s + "\n Error: " + s.error.message.wrap(70).join("\n        ")}.join('\n\n')}
+EOM
+#' stupid ruby-mode
+    end
+
+    desynced_sources = Index.usual_sources.select { |s| s.error.is_a? OutOfSyncSourceError }
+    unless desynced_sources.empty?
+      BufferManager.spawn_unless_exists "Out-of-sync soure notification" do
+        TextMode.new(<<EOM)
 Out-of-sync source notification
 -------------------------------
 
-Hi there. It looks like one or more sources have fallen out of sync
+Hi there. It looks like one or more sources has fallen out of sync
 with my index. This can happen when you modify these sources with
 other email clients. (Sorry, I don't play well with others.)
 
 Until this is corrected, messages from these sources cannot be viewed,
 and new messages will not be detected. Luckily, this is easy to correct!
 
-#{broken_sources.map { |s| "Source: " + s.to_s + "\n Error: " + s.broken_msg.wrap(70).join("\n        ") }.join('\n\n')}
+#{desynced_sources.map do |s|
+  "Source: " + s.to_s + 
+   "\n Error: " + s.error.message.wrap(70).join("\n        ") + 
+   "\n   Fix: sup-sync --changed #{s.to_s}"
+  end}
 EOM
+#' stupid ruby-mode
+      end
     end
   end
 
index f1a9e43a8365333e87acb7576271d537546ba123..6a6085c0eaa9442a68884eaec638fe057517282d 100644 (file)
@@ -90,7 +90,7 @@ class IMAP < Source
   synchronized :raw_full_message
 
   def connect
-    return if broken? || @imap
+    return if @imap
     safely { } # do nothing!
   end
   synchronized :connect
@@ -122,7 +122,7 @@ class IMAP < Source
         @ids
       end
 
-    start = ids.index(cur_offset || start_offset) or die_from "Unknown message id #{cur_offset || start_offset}.", :suggest_rebuild => true # couldn't find the most recent email
+    start = ids.index(cur_offset || start_offset) or raise OutOfSyncSourceError, "Unknown message id #{cur_offset || start_offset}."
 
     start.upto(ids.length - 1) do |i|         
       id = ids[i]
@@ -197,26 +197,6 @@ private
     @say_id = nil
   end
 
-  def die_from e, opts={}
-    @imap = nil
-
-    message =
-      case e
-      when Exception
-        "Error while #{opts[:while]}: #{e.message.chomp} (#{e.class.name})."
-      when String
-        e
-      end
-
-    message += " It is likely that messages have been deleted from this IMAP mailbox. Please run sup-sync --changed #{to_s} to correct this problem." if opts[:suggest_rebuild]
-
-    self.broken_msg = message
-    Redwood::log message
-    BufferManager.flash "Error communicating with IMAP server. See log for details." if BufferManager.instantiated?
-    raise SourceError, message
-  end
-  
-  ## build a fake unique id
   def make_id imap_stuff
     # use 7 digits for the size. why 7? seems nice.
     msize, mdate = imap_stuff.attr['RFC822.SIZE'] % 10000000, Time.parse(imap_stuff.attr["INTERNALDATE"])
@@ -224,13 +204,12 @@ private
   end
 
   def get_imap_fields id, *fields
-    raise SourceError, broken_msg if broken?
-    imap_id = @imap_ids[id] or die_from "Unknown message id #{id}.", :suggest_rebuild => true
+    imap_id = @imap_ids[id] or raise OutOfSyncSourceError, "Unknown message id #{id}"
 
     retried = false
     results = safely { @imap.fetch imap_id, (fields + ['RFC822.SIZE', 'INTERNALDATE']).uniq }.first
     got_id = make_id results
-    die_from "IMAP message mismatch: requested #{id}, got #{got_id}.", :suggest_rebuild => true unless got_id == id
+    raise OutOfSyncSourceError, "IMAP message mismatch: requested #{id}, got #{got_id}." unless got_id == id
 
     fields.map { |f| results.attr[f] }
   end
@@ -252,7 +231,7 @@ private
         raise
       end
     rescue Net, SocketError, Net::IMAP::Error, SystemCallError => e
-      die_from e, :while => "communicating with IMAP server"
+      raise FatalSourceError, "While communicating with IMAP server: #{e.message}"
     end
   end
 
index 5d1a7b3c5ce0fba905ec9073552d501648b68ba3..18efe86607319c9ff7230f9c5135cb7922ea132a 100644 (file)
@@ -295,7 +295,8 @@ protected
   end
 
   def load_sources fn=Redwood::SOURCE_FN
-    @sources = Hash[*(Redwood::load_yaml_obj(fn) || []).map { |s| [s.id, s] }.flatten]
+    source_array = (Redwood::load_yaml_obj(fn) || []).map { |o| Recoverable.new o }
+    @sources = Hash[*(source_array).map { |s| [s.id, s] }.flatten]
     @sources_dirty = false
   end
 
index ba42d79190b9f644ae686f24524d1cd79ade1dff..5bc884b7ba84d31532e790a72ea8ea7fd4372c7e 100644 (file)
@@ -64,7 +64,7 @@ class Maildir < Source
         [ids.sort, ids_to_fns]
       end
     rescue SystemCallError => e
-      die "Problem scanning Maildir directories: #{e.message}."
+      raise FatalSourceError, "Problem scanning Maildir directories: #{e.message}."
     end
     
     @last_scan = Time.now
@@ -72,7 +72,7 @@ class Maildir < Source
 
   def each
     scan_mailbox
-    start = @ids.index(cur_offset || start_offset) or die "Unknown message id #{cur_offset || start_offset}.", :suggest_rebuild => true # couldn't find the most recent email
+    start = @ids.index(cur_offset || start_offset) or raise OutOfSyncSourceError, "Unknown message id #{cur_offset || start_offset}." # couldn't find the most recent email
 
     start.upto(@ids.length - 1) do |i|         
       id = @ids[i]
@@ -95,25 +95,17 @@ class Maildir < Source
 
 private
 
-  def die message, opts={}
-    message += " It is likely that messages have been deleted from this Maildir mailbox. Please run sup-sync --changed #{to_s} to correct this problem." if opts[:suggest_rebuild]
-    self.broken_msg = message
-    Redwood::log message
-    BufferManager.flash "Error communicating with Maildir. See log for details." if BufferManager.instantiated?
-    raise SourceError, message
-  end
-  
   def make_id fn
     # use 7 digits for the size. why 7? seems nice.
     sprintf("%d%07d", File.mtime(fn), File.size(fn)).to_i
   end
 
   def with_file_for id
-    fn = @ids_to_fns[id] or die "No such id: #{id.inspect}.", :suggest_rebuild => true
+    fn = @ids_to_fns[id] or raise OutOfSyncSourceError, "No such id: #{id.inspect}."
     begin
       File.open(fn) { |f| yield f }
     rescue SystemCallError => e
-      die "Problem reading file for id #{id.inspect}: #{fn.inspect}: #{e.message}."
+      raise FatalSourceError, "Problem reading file for id #{id.inspect}: #{fn.inspect}: #{e.message}."
     end
   end
 end
index 724d49b5f9793807d156f4def80def6e6b8a6283..dfca5b284d3e72444f2e1f401e3faf9ccfa67e66 100644 (file)
@@ -1,4 +1,5 @@
 require 'rmail'
+require 'uri'
 
 module Redwood
 module MBox
@@ -14,7 +15,7 @@ class Loader < Source
     when String
       raise ArgumentError, "not an mbox uri" unless uri_or_fp =~ %r!mbox://!
 
-      fn = uri_or_fp.sub(%r!^mbox://!, "")
+      fn = URI(uri_or_fp).path
       ## heuristic: use the filename as a label, unless the file
       ## has a path that probably represents an inbox.
       @labels << File.basename(fn).intern unless File.dirname(fn) =~ /\b(var|usr|spool)\b/
@@ -22,12 +23,14 @@ class Loader < Source
     else
       @f = uri_or_fp
     end
+  end
 
+  def check
     if cur_offset > end_offset
-      self.broken_msg = "mbox file is smaller than last recorded message offset. Messages have probably been deleted via another client. Run 'sup-sync --changed #{to_s}' to correct this."
+      raise OutOfSyncSourceError, "mbox file is smaller than last recorded message offset. Messages have probably been deleted by another client."
     end
   end
-
+    
   def start_offset; 0; end
   def end_offset; File.size @f; end
 
@@ -37,9 +40,7 @@ class Loader < Source
       @f.seek offset
       l = @f.gets
       unless l =~ BREAK_RE
-        Redwood::log "#{to_s}: offset mismatch in mbox file offset #{offset.inspect}: #{l.inspect}"
-        self.broken_msg = "offset mismatch in mbox file offset #{offset.inspect}: #{l.inspect}. Run 'sup-sync --changed #{to_s}' to correct this." 
-        raise SourceError, self.broken_msg
+        raise OutOfSyncSourceError, "mismatch in mbox file offset #{offset.inspect}: #{l.inspect}." 
       end
       header = MBox::read_header @f
     end
@@ -47,7 +48,6 @@ class Loader < Source
   end
 
   def load_message offset
-    raise SourceError, self.broken_msg if broken?
     @mutex.synchronize do
       @f.seek offset
       begin
@@ -55,13 +55,12 @@ class Loader < Source
           return RMail::Parser.read(input)
         end
       rescue RMail::Parser::Error => e
-        raise SourceError, "error parsing message with rmail: #{e.message}"
+        raise FatalSourceError, "error parsing mbox file: #{e.message}"
       end
     end
   end
 
   def raw_header offset
-    raise SourceError, self.broken_msg if broken?
     ret = ""
     @mutex.synchronize do
       @f.seek offset
@@ -73,7 +72,6 @@ class Loader < Source
   end
 
   def raw_full_message offset
-    raise SourceError, self.broken_msg if broken?
     ret = ""
     @mutex.synchronize do
       @f.seek offset
@@ -86,33 +84,36 @@ class Loader < Source
   end
 
   def next
-    raise SourceError, self.broken_msg if broken?
     returned_offset = nil
     next_offset = cur_offset
 
-    @mutex.synchronize do
-      @f.seek cur_offset
-
-      ## cur_offset could be at one of two places here:
-
-      ## 1. before a \n and a mbox separator, if it was previously at
-      ##    EOF and a new message was added; or,
-      ## 2. at the beginning of an mbox separator (in all other
-      ##    cases).
-
-      l = @f.gets or raise "next while at EOF"
-      if l =~ /^\s*$/ # case 1
-        returned_offset = @f.tell
-        @f.gets # now we're at a BREAK_RE, so skip past it
-      else # case 2
-        returned_offset = cur_offset
-        ## we've already skipped past the BREAK_RE, so just go
-      end
+    begin
+      @mutex.synchronize do
+        @f.seek cur_offset
+
+        ## cur_offset could be at one of two places here:
 
-      while(line = @f.gets)
-        break if line =~ BREAK_RE
-        next_offset = @f.tell
+        ## 1. before a \n and a mbox separator, if it was previously at
+        ##    EOF and a new message was added; or,
+        ## 2. at the beginning of an mbox separator (in all other
+        ##    cases).
+
+        l = @f.gets or raise "next while at EOF"
+        if l =~ /^\s*$/ # case 1
+          returned_offset = @f.tell
+          @f.gets # now we're at a BREAK_RE, so skip past it
+        else # case 2
+          returned_offset = cur_offset
+          ## we've already skipped past the BREAK_RE, so just go
+        end
+
+        while(line = @f.gets)
+          break if line =~ BREAK_RE
+          next_offset = @f.tell
+        end
       end
+    rescue SystemCallError => e
+      raise FatalSourceError, "Error reading #{@f.path}: #{e.message}"
     end
 
     self.cur_offset = next_offset
index 5a9eda751373e5ae8b7d62dfe6cf3908efb348b6..decef6537c19f1b0dfc080337541139f860d9562 100644 (file)
@@ -138,7 +138,6 @@ class Message
   end
   private :read_header
 
-  def broken?; @source.broken?; end
   def snippet; @snippet || chunks && @snippet; end
   def is_list_message?; !@list_address.nil?; end
   def is_draft?; DraftLoader === @source; end
@@ -148,7 +147,6 @@ class Message
   end
 
   def save index
-    return if broken?
     index.sync_message self if @dirty
     @dirty = false
   end
@@ -177,8 +175,8 @@ class Message
   ## this is called when the message body needs to actually be loaded.
   def load_from_source!
     @chunks ||=
-      if @source.broken?
-        [Text.new(error_message(@source.broken_msg.split("\n")))]
+      if @source.has_errors?
+        [Text.new(error_message(@source.error.message.split("\n")))]
       else
         begin
           ## we need to re-read the header because it contains information
index 3483383728100255792279e827f1a8a5b05e3177..cae97aea3c9c156313ed23820108b301ec87b53a 100644 (file)
@@ -43,7 +43,7 @@ class PollManager
     @mutex.synchronize do
       Index.usual_sources.each do |source|
 #        yield "source #{source} is done? #{source.done?} (cur_offset #{source.cur_offset} >= #{source.end_offset})"
-        yield "Loading from #{source}... " unless source.done? || source.broken?
+        yield "Loading from #{source}... " unless source.done? || source.has_errors?
         num = 0
         numi = 0
         add_messages_from source do |m, offset, entry|
@@ -82,11 +82,11 @@ class PollManager
   ## the index labels, if they exist, so that state is not lost when
   ## e.g. a new version of a message from a mailing list comes in.
   def add_messages_from source
-    return if source.done? || source.broken?
+    return if source.done? || source.has_errors?
 
     begin
       source.each do |offset, labels|
-        if source.broken?
+        if source.has_errors?
           Redwood::log "error loading messages from #{source}: #{source.broken_msg}"
           return
         end
index be01780ee8bb61ffec6894ce6ff18cbc18072235..dce2f63501b344a5c308d0c64abf1cdc8f2002e0 100644 (file)
@@ -1,6 +1,8 @@
 module Redwood
 
 class SourceError < StandardError; end
+class OutOfSyncSourceError < SourceError; end
+class FatalSourceError < SourceError; end
 
 class Source
   ## Implementing a new source is typically quite easy, because Sup
@@ -30,75 +32,60 @@ class Source
   ## - load_message offset
   ## - raw_header offset
   ## - raw_full_message offset
+  ## - check
   ## - next (or each, if you prefer)
   ##
   ## ... where "offset" really means unique id. (You can tell I
   ## started with mbox.)
   ##
-  ## You can throw SourceErrors from any of those, but we don't catch
-  ## anything else, so make sure you catch *all* errors and reraise
-  ## them as SourceErrors, and set broken_msg to something if the
-  ## source needs to be rescanned.
+  ## All exceptions relating to accessing the source must be caught
+  ## and rethrown as FatalSourceErrors or OutOfSyncSourceErrors.
+  ## OutOfSyncSourceErrors should be used for problems that a call to
+  ## sup-sync will fix (namely someone's been playing with the source
+  ## from another client); FatalSourceErrors can be used for anything
+  ## else (e.g. the imap server is down or the maildir is missing.)
   ##
-  ## Also, be sure to make the source thread-safe, since it WILL be
+  ## Finally, be sure the source is thread-safe, since it WILL be
   ## pummeled from multiple threads at once.
   ##
-  ## Two examples for you to look at, though sadly neither of them is
-  ## as simple as I'd like: mbox/loader.rb and imap.rb
+  ## Examples for you to look at: mbox/loader.rb, imap.rb, and
+  ## maildir.rb.
 
-
-
-  ## dirty? described whether cur_offset has changed, which means the
-  ## source info needs to be re-saved to sources.yaml.
+  ## let's begin!
   ##
-  ## broken? means no message can be loaded, e.g. IMAP server is
-  ## down, mbox file is corrupt and needs to be rescanned, etc.
+  ## dirty? means cur_offset has changed, so the source info needs to
+  ## be re-saved to sources.yaml.
   bool_reader :usual, :archived, :dirty
-  attr_reader :uri, :cur_offset, :broken_msg
+  attr_reader :uri, :cur_offset
   attr_accessor :id
 
   def initialize uri, initial_offset=nil, usual=true, archived=false, id=nil
     @uri = uri
-    @cur_offset = initial_offset
+    @cur_offset = initial_offset || start_offset
     @usual = usual
     @archived = archived
     @id = id
     @dirty = false
-    @broken_msg = nil
   end
 
-  def broken?; !@broken_msg.nil?; end
   def to_s; @uri.to_s; end
   def seek_to! o; self.cur_offset = o; end
-  def reset!
-    @broken_msg = nil
-    begin
-      seek_to! start_offset
-    rescue SourceError
-    end
-  end
+  def reset!; seek_to! start_offset; end
   def == o; o.to_s == to_s; end
-  def done?;
-    return true if broken? 
-    begin
-      (self.cur_offset ||= start_offset) >= end_offset
-    rescue SourceError => e
-      true
-    end
-  end
+  def done?; (self.cur_offset ||= start_offset) >= end_offset; end
   def is_source_for? uri; URI(self.uri) == URI(uri); end
 
+  ## check should throw a FatalSourceError or an OutOfSyncSourcError
+  ## if it can detect a problem. it is called when the sup starts up
+  ## to proactively notify the user of any source problems.
+  def check; end
+
   def each
-    return if broken?
-    begin
-      self.cur_offset ||= start_offset
-      until done? || broken? # just like life!
-        n, labels = self.next
-        raise "no message" unless n
-        yield n, labels
-      end
-    rescue SourceError => e
-      self.broken_msg = e.message
+    self.cur_offset ||= start_offset
+    until done?
+      n, labels = self.next
+      raise "no message" unless n
+      yield n, labels
     end
   end
 
@@ -108,11 +95,6 @@ protected
     @cur_offset = o
     @dirty = true
   end
-
-  def broken_msg= m
-    @broken_msg = m
-#    Redwood::log "#{to_s}: #{m}"
-  end
 end
 
 Redwood::register_yaml(Source, %w(uri cur_offset usual archived id))
index c9ac70c2473fd706158db21704f3fc7bd594cde5..98632823d96f09c29960057ace47e349432af0f3 100644 (file)
@@ -282,3 +282,31 @@ module Singleton
     klass.extend ClassMethods
   end
 end
+
+## wraps an object. if it throws an exception, keeps a copy, and
+## rethrows it for any further method calls.
+class Recoverable
+  def initialize o
+    @o = o
+    @e = nil
+  end
+
+  def clear_error!; @e = nil; end
+  def has_errors?; !@e.nil?; end
+  def error; @e; end
+
+  def method_missing m, *a, &b; __pass m, *a, &b; end
+  
+  def id; __pass :id; end
+  def to_s; __pass :to_s; end
+  def to_yaml x; __pass :to_yaml, x; end
+
+  def __pass m, *a, &b
+    begin
+      @o.send(m, *a, &b)
+    rescue Exception => e
+      @e = e
+      raise e
+    end
+  end
+end