Merge pull request #2750 from riking/mailing-list-errors

"FIX": Add error reporting to NotifyMailingListSubscribers

app/jobs/regular/notify_mailing_list_subscribers.rb

@@ -12,9 +12,8 @@ module Jobs
 
       raise Discourse::InvalidParameters.new(:post_id) unless post
 
-      User.not_suspended
-          .not_blocked
-          .real
+      users =
+          User.activated.not_blocked.not_suspended.real
           .where(mailing_list_mode:  true)
           .where('NOT EXISTS(
                       SELECT 1
@@ -32,11 +31,25 @@ module Jobs
                        cu.user_id = users.id AND
                        cu.notification_level = ?
                   )', post.topic.category_id, CategoryUser.notification_levels[:muted])
-          .each do |user|
-            if Guardian.new(user).can_see?(post)
-              message = UserNotifications.mailing_list_notify(user, post)
-              Email::Sender.new(message, :mailing_list, user).send
+
+      error_count = 0
+      users.each do |user|
+        if Guardian.new(user).can_see?(post)
+          begin
+            message = UserNotifications.mailing_list_notify(user, post)
+            Email::Sender.new(message, :mailing_list, user).send
+          rescue => e
+            Discourse.handle_exception(e, error_context(
+                args,
+                "Sending post to mailing list subscribers", {
+                user_id: user.id,
+                user_email: user.email
+            }))
+            if (++error_count) >= 4
+              raise RuntimeError, "ABORTING NotifyMailingListSubscribers due to repeated failures"
             end
+          end
+        end
       end
 
     end

app/models/user.rb

@@ -99,13 +99,18 @@ class User < ActiveRecord::Base
   # set to true to optimize creation and save for imports
   attr_accessor :import_mode
 
-  scope :blocked, -> { where(blocked: true) } # no index
-  scope :not_blocked, -> { where(blocked: false) } # no index
-  scope :suspended, -> { where('suspended_till IS NOT NULL AND suspended_till > ?', Time.zone.now) } # no index
-  scope :not_suspended, -> { where('suspended_till IS NULL OR suspended_till <= ?', Time.zone.now) }
-  # excluding fake users like the community user
+  # excluding fake users like the system user
   scope :real, -> { where('id > 0') }
 
+  # TODO-PERF: There is no indexes on any of these
+  # and NotifyMailingListSubscribers does a select-all-and-loop
+  # may want to create an index on (active, blocked, suspended_till, mailing_list_mode)?
+  scope :blocked, -> { where(blocked: true) }
+  scope :not_blocked, -> { where(blocked: false) }
+  scope :suspended, -> { where('suspended_till IS NOT NULL AND suspended_till > ?', Time.zone.now) }
+  scope :not_suspended, -> { where('suspended_till IS NULL OR suspended_till <= ?', Time.zone.now) }
+  scope :activated, -> { where(active: true) }
+
   module NewTopicDuration
     ALWAYS = -1
     LAST_VISIT = -2