sup

commit 7edbb7ee676b4789296bd733b28b60f669724f88
parent e624fea237e262a3b22e944b0eb343e028b02272
Author: Rich Lane <rlane@club.cc.cmu.edu>
Date:   Sat, 10 Apr 2010 15:57:40 -0700

source error handling rework

Diffstat:

9 files changed, 85 insertions(+), 112 deletions(-)
diff --git a/lib/sup/draft.rb b/lib/sup/draft.rb
@@ -11,7 +11,7 @@ class DraftManager
 
   def self.source_name; "sup://drafts"; end
   def self.source_id; 9999; end
-  def new_source; @source = Recoverable.new DraftLoader.new; end
+  def new_source; @source = DraftLoader.new; end
 
   def write_draft
     offset = @source.gen_offset
diff --git a/lib/sup/index.rb b/lib/sup/index.rb
@@ -197,7 +197,7 @@ EOS
     locations = entry[:locations].map do |source_id,source_info|
       source = SourceManager[source_id]
       raise "invalid source #{source_id}" unless source
-      [source, source_info]
+      Location.new source, source_info
     end
 
     m = Message.new :locations => locations,
@@ -603,7 +603,7 @@ EOS
 
     entry = {
       :message_id => m.id,
-      :locations => m.locations.map { |source,source_info| [source.id, source_info] },
+      :locations => m.locations.map { |x| [x.source.id, x.info] },
       :date => truncate_date(m.date),
       :snippet => snippet,
       :labels => m.labels.to_a,
diff --git a/lib/sup/maildir.rb b/lib/sup/maildir.rb
@@ -138,6 +138,10 @@ class Maildir < Source
   def mark_seen id; maildir_mark_file id, "S" unless seen? id; end
   def mark_trashed id; maildir_mark_file id, "T" unless trashed? id; end
 
+  def valid? id
+    File.exists? File.join(@dir, id)
+  end
+
 private
 
   def new_maildir_basefn
diff --git a/lib/sup/message.rb b/lib/sup/message.rb
@@ -9,9 +9,6 @@ module Redwood
 ## i would like, for example, to be able to add in a ruby-talk
 ## specific module that would detect and link to /ruby-talk:\d+/
 ## sequences in the text of an email. (how sweet would that be?)
-##
-## this class catches all source exceptions. if the underlying source
-## throws an error, it is caught and handled.
 
 class Message
   SNIPPET_LEN = 80
@@ -171,7 +168,7 @@ class Message
 
   attr_reader :snippet
   def is_list_message?; !@list_address.nil?; end
-  def is_draft?; source.is_a? DraftLoader; end
+  def is_draft?; @labels.member? :draft; end
   def draft_filename
     raise "not a draft" unless is_draft?
     source.fn_for_offset source_info
@@ -225,87 +222,59 @@ class Message
     @chunks
   end
 
+  def location
+    @locations.find { |x| x.valid? } || raise(OutOfSyncSourceError.new)
+  end
+
   def source
-    fail if @locations.empty?
-    @locations.last[0]
+    location.source
   end
 
   def source_info
-    fail if @locations.empty?
-    @locations.last[1]
+    location.info
   end
 
   ## this is called when the message body needs to actually be loaded.
   def load_from_source!
     @chunks ||=
-      if source.respond_to?(:has_errors?) && source.has_errors?
-        [Chunk::Text.new(error_message(source.error.message).split("\n"))]
-      else
-        begin
-          ## we need to re-read the header because it contains information
-          ## that we don't store in the index. actually i think it's just
-          ## the mailing list address (if any), so this is kinda overkill.
-          ## i could just store that in the index, but i think there might
-          ## be other things like that in the future, and i'd rather not
-          ## bloat the index.
-          ## actually, it's also the differentiation between to/cc/bcc,
-          ## so i will keep this.
-          rmsg = source.load_message(source_info)
-          parse_header rmsg.header
-          message_to_chunks rmsg
-        rescue SourceError, SocketError => e
-          warn "problem getting messages from #{source}: #{e.message}"
-          ## we need force_to_top here otherwise this window will cover
-          ## up the error message one
-          source.error ||= e
-          Redwood::report_broken_sources :force_to_top => true
-          [Chunk::Text.new(error_message(e.message).split("\n"))]
-        end
+      begin
+        ## we need to re-read the header because it contains information
+        ## that we don't store in the index. actually i think it's just
+        ## the mailing list address (if any), so this is kinda overkill.
+        ## i could just store that in the index, but i think there might
+        ## be other things like that in the future, and i'd rather not
+        ## bloat the index.
+        ## actually, it's also the differentiation between to/cc/bcc,
+        ## so i will keep this.
+        rmsg = location.parsed_message
+        parse_header rmsg.header
+        message_to_chunks rmsg
+      rescue SourceError, SocketError => e
+        warn "problem reading message #{id}"
+        [Chunk::Text.new(error_message.split("\n"))]
       end
   end
 
-  def error_message msg
+  def error_message
     <<EOS
 #@snippet...
 
 ***********************************************************************
- An error occurred while loading this message. It is possible that
- the source has changed, or (in the case of remote sources) is down.
- You can check the log for errors, though hopefully an error window
- should have popped up at some point.
-
- The message location was:
- #{source}##{source_info}
+ An error occurred while loading this message.
 ***********************************************************************
-
-The error message was:
-  #{msg}
 EOS
   end
 
-  ## wrap any source methods that might throw sourceerrors
-  def with_source_errors_handled
-    begin
-      yield
-    rescue SourceError => e
-      warn "problem getting messages from #{source}: #{e.message}"
-      source.error ||= e
-      Redwood::report_broken_sources :force_to_top => true
-      error_message e.message
-    end
-  end
-
   def raw_header
-    with_source_errors_handled { source.raw_header source_info }
+    location.raw_header
   end
 
   def raw_message
-    with_source_errors_handled { source.raw_message source_info }
+    location.raw_message
   end
 
-  ## much faster than raw_message
   def each_raw_message_line &b
-    with_source_errors_handled { source.each_raw_message_line(source_info, &b) }
+    location.each_raw_message_line &b
   end
 
   ## returns all the content from a message that will be indexed
@@ -347,7 +316,7 @@ EOS
   end
 
   def self.build_from_source source, source_info
-    m = Message.new :locations => [[source, source_info]]
+    m = Message.new :locations => [Location.new(source, source_info)]
     m.load_from_source!
     m
   end
@@ -611,4 +580,43 @@ private
   end
 end
 
+class Location
+  attr_reader :source
+  attr_reader :info
+
+  def initialize source, info
+    @source = source
+    @info = info
+  end
+
+  def raw_header
+    source.raw_header info
+  end
+
+  def raw_message
+    source.raw_message info
+  end
+
+  ## much faster than raw_message
+  def each_raw_message_line &b
+    source.each_raw_message_line info, &b
+  end
+
+  def parsed_message
+    source.load_message info
+  end
+
+  def valid?
+    source.valid? info
+  end
+
+  def == o
+    o.source.id == source.id and o.info == info
+  end
+
+  def hash
+    [source.id, info].hash
+  end
+end
+
 end
diff --git a/lib/sup/modes/thread-index-mode.rb b/lib/sup/modes/thread-index-mode.rb
@@ -231,7 +231,7 @@ EOS
     old_cursor_thread = cursor_thread
     @mutex.synchronize do
       ## let's see you do THIS in python
-      @threads = @ts.threads.select { |t| !@hidden_threads[t] }.sort_by { |t| [t.date, t.first.id] }.reverse
+      @threads = @ts.threads.select { |t| !@hidden_threads[t] }.select(&:first).sort_by { |t| [t.date, t.first.id] }.reverse
       @size_widgets = @threads.map { |t| size_widget_for_thread t }
       @size_widget_width = @size_widgets.max_of { |w| w.display_length }
       @date_widgets = @threads.map { |t| date_widget_for_thread t }
diff --git a/lib/sup/poll.rb b/lib/sup/poll.rb
@@ -103,10 +103,9 @@ EOS
     @mutex.synchronize do
       @poll_sources.each do |source|
         begin
-          yield "Loading from #{source}... " unless source.has_errors?
+          yield "Loading from #{source}... "
         rescue SourceError => e
           warn "problem getting messages from #{source}: #{e.message}"
-          Redwood::report_broken_sources :force_to_top => true
           next
         end
 
@@ -117,7 +116,7 @@ EOS
             yield "Deleting #{m.id}"
           elsif action == :add
             if old_m
-              if not old_m.locations.member? [source, m.source_info]
+              if not old_m.locations.member? m.location
                 yield "Message at #{m.source_info} is an updated of an old message. Updating labels from #{old_m.labels.to_a * ','} => #{m.labels.to_a * ','}"
               else
                 yield "Skipping already-imported message at #{m.source_info}"
@@ -153,14 +152,7 @@ EOS
   ## from the index after being yielded.
   def poll_from source, opts={}
     begin
-      return if source.has_errors?
-
       source.poll do |sym, args|
-        if source.has_errors?
-          warn "error loading messages from #{source}: #{source.error.message}"
-          return
-        end
-
         case sym
         when :add
           m = Message.build_from_source source, args[:info]
@@ -177,16 +169,15 @@ EOS
           UpdateManager.relay self, :added, m
         when :delete
           Index.each_message :location => [source.id, args[:info]] do |m|
-            m.locations.delete [source,args[:info]]
+            m.locations.delete Location.new(source, args[:info])
             yield :delete, m, [source,args[:info]] if block_given?
             Index.sync_message m, false
-            UpdateManager.relay self, :deleted, m
+            #UpdateManager.relay self, :deleted, m
           end
         end
       end
     rescue SourceError => e
       warn "problem getting messages from #{source}: #{e.message}"
-      Redwood::report_broken_sources :force_to_top => true
     end
   end
 
diff --git a/lib/sup/sent.rb b/lib/sup/sent.rb
@@ -19,7 +19,7 @@ class SentManager
   end
 
   def default_source
-    @source = Recoverable.new SentLoader.new
+    @source = SentLoader.new
     @source_uri = @source.uri
     @source
   end
diff --git a/lib/sup/source.rb b/lib/sup/source.rb
@@ -88,6 +88,10 @@ class Source
     unimplemented
   end
 
+  def valid? info
+    true
+  end
+
   ## utility method to read a raw email header from an IO stream and turn it
   ## into a hash of key-value pairs. minor special semantics for certain headers.
   ##
@@ -188,7 +192,7 @@ class SourceManager
   def unusual_sources; sources.find_all { |s| !s.usual? }; end
 
   def load_sources fn=Redwood::SOURCE_FN
-    source_array = (Redwood::load_yaml_obj(fn) || []).map { |o| Recoverable.new o }
+    source_array = Redwood::load_yaml_obj(fn) || []
     @source_mutex.synchronize do
       @sources = Hash[*(source_array).map { |s| [s.id, s] }.flatten]
       @sources_dirty = false
diff --git a/lib/sup/util.rb b/lib/sup/util.rb
@@ -594,40 +594,6 @@ module Singleton
   end
 end
 
-## wraps an object. if it throws an exception, keeps a copy.
-class Recoverable
-  def initialize o
-    @o = o
-    @error = nil
-    @mutex = Mutex.new
-  end
-
-  attr_accessor :error
-
-  def clear_error!; @error = nil; end
-  def has_errors?; !@error.nil?; 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 is_a? c; @o.is_a? c; end
-
-  def respond_to?(m, include_private=false)
-    @o.respond_to?(m, include_private)
-  end
-
-  def __pass m, *a, &b
-    begin
-      @o.send(m, *a, &b)
-    rescue Exception => e
-      @error ||= e
-      raise
-    end
-  end
-end
-
 ## acts like a hash with an initialization block, but saves any
 ## newly-created value even upon lookup.
 ##