FIX: capture Akismet API errors (#55)

FIX: capture Akismet API errors (#55)

  • FIX: capture Akismet API errors
  • Move ‘mark_as’ methods to parent class
diff --git a/assets/javascripts/discourse/templates/components/reviewable-akismet-api-error.hbs b/assets/javascripts/discourse/templates/components/reviewable-akismet-api-error.hbs
new file mode 100644
index 0000000..f8fabd3
--- /dev/null
+++ b/assets/javascripts/discourse/templates/components/reviewable-akismet-api-error.hbs
@@ -0,0 +1,6 @@
+<div class='reviewable-score-reason'>
+  {{ i18n "admin.akismet_api_error" }}
+  {{ external_error.error }}
+  ({{ external_error.code }})
+  {{ external_error.msg }}
+</div>
diff --git a/assets/javascripts/discourse/templates/components/reviewable-akismet-post.hbs b/assets/javascripts/discourse/templates/components/reviewable-akismet-post.hbs
index effef6f..3b4f72f 100644
--- a/assets/javascripts/discourse/templates/components/reviewable-akismet-post.hbs
+++ b/assets/javascripts/discourse/templates/components/reviewable-akismet-post.hbs
@@ -7,5 +7,8 @@
     {{{ reviewable.payload.post_cooked }}}
   </div>
   {{yield}}
+  {{#if reviewable.payload.external_error}}
+  	{{reviewable-akismet-api-error external_error=reviewable.payload.external_error}}
+  {{/if}}
 </div>
 </div>
diff --git a/assets/javascripts/discourse/templates/components/reviewable-akismet-user.hbs b/assets/javascripts/discourse/templates/components/reviewable-akismet-user.hbs
index cb47ba3..8945b15 100644
--- a/assets/javascripts/discourse/templates/components/reviewable-akismet-user.hbs
+++ b/assets/javascripts/discourse/templates/components/reviewable-akismet-user.hbs
@@ -27,4 +27,8 @@
   </div>
   
   {{yield}}
+
+  {{#if reviewable.payload.external_error}}
+    {{reviewable-akismet-api-error external_error=reviewable.payload.external_error}}
+  {{/if}}
 </div>
diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml
index a18995d..15ef047 100644
--- a/config/locales/client.en.yml
+++ b/config/locales/client.en.yml
@@ -14,7 +14,7 @@ en:
         skipped: This user doesn't have enough information to be reviewed by Akismet.
         confirmed_ham: Akismet checked this user and decided that it's not spam.
         confirmed_spam: Akismet decided that this user is spam.
-
+      akismet_api_error: "Akismet API Error:"
             
     akismet:
       title: "Akismet"
diff --git a/lib/akismet.rb b/lib/akismet.rb
index 58570e1..60bad9a 100644
--- a/lib/akismet.rb
+++ b/lib/akismet.rb
@@ -11,6 +11,7 @@ class Akismet
     VALID_SUBMIT_RESPONSE = 'Thanks for making the web a better place.'.freeze
     UNKNOWN_ERROR_MESSAGE = 'Unknown error'.freeze
     DEBUG_HEADER = 'X-akismet-debug-help'.freeze
+    ERROR_HEADER = 'X-akismet-error'.freeze
 
     def initialize(api_key:, base_url:)
       @api_url =  "https://#{api_key}.rest.akismet.com/1.1"
@@ -25,12 +26,21 @@ class Akismet
       response = post('comment-check', body)
       response_body = response.body
 
+      if (response.is_a?(Excon::Response) and response.get_header(ERROR_HEADER))
+        api_error = {}
+        api_error[:error] = response.get_header('X-akismet-error')
+        api_error[:code]  = response.get_header('X-akismet-alert-code')
+        api_error[:msg]   = response.get_header('X-akismet-alert-msg')
+
+        return 'error', api_error.compact
+      end
+
       if !(VALID_COMMENT_CHECK_RESPONSE.include?(response_body))
         debug_help = response.headers[DEBUG_HEADER] || UNKNOWN_ERROR_MESSAGE
         raise Akismet::Error.new(debug_help)
       end
 
-      response_body == VALID_COMMENT_CHECK_RESPONSE.first
+      response_body == VALID_COMMENT_CHECK_RESPONSE.first ? 'spam' : 'ham'
     end
 
     def submit_feedback(state, body)
diff --git a/lib/discourse_akismet/bouncer.rb b/lib/discourse_akismet/bouncer.rb
index efdf81f..4d4d194 100644
--- a/lib/discourse_akismet/bouncer.rb
+++ b/lib/discourse_akismet/bouncer.rb
@@ -27,8 +27,15 @@ module DiscourseAkismet
       pre_check_passed = before_check(target)
 
       if pre_check_passed
-        client.comment_check(args_for(target)).tap do |spam_detected|
-          spam_detected ? mark_as_spam(target) : mark_as_clear(target)
+        client.comment_check(args_for(target)).tap do |result, error_status|
+          case result
+          when 'spam'
+            mark_as_spam(target)
+          when 'error'
+            mark_as_errored(target, error_status)
+          else
+            mark_as_clear(target)
+          end
         end
       else
         move_to_state(target, 'skipped')
@@ -56,5 +63,29 @@ module DiscourseAkismet
     def spam_reporter
       @spam_reporter ||= Discourse.system_user
     end
+
+    # subclasses must implement "mark_as_spam" to change state/track/log/notify as appropraite
+    def mark_as_spam(target)
+      raise NotImplementedError
+    end
+
+    def mark_as_clear(target)
+      move_to_state(target, 'confirmed_ham')
+    end
+
+    # subclass this, and pass in a block that will create an appropriate Reviewable object
+    def mark_as_errored(target, reason)
+      raise NotImplementedError unless block_given?
+
+      limiter = RateLimiter.new(nil, "akismet_error_#{reason[:code].to_i}", 1, 10.minutes)
+
+      if limiter.performed!(raise_error: false)
+        reviewable = yield 
+
+        add_score(reviewable, 'akismet_server_error')
+        move_to_state(target, 'needs_review')
+      end
+    end
+
   end
 end
diff --git a/lib/discourse_akismet/posts_bouncer.rb b/lib/discourse_akismet/posts_bouncer.rb
index 8ff5a3a..2b98a33 100644
--- a/lib/discourse_akismet/posts_bouncer.rb
+++ b/lib/discourse_akismet/posts_bouncer.rb
@@ -111,8 +111,13 @@ module DiscourseAkismet
       move_to_state(post, 'confirmed_spam')
     end
 
-    def mark_as_clear(post)
-      move_to_state(post, 'confirmed_ham')
+    def mark_as_errored(post, reason)
+      super do 
+        ReviewableAkismetPost.needs_review!(
+          created_by: spam_reporter, target: post, topic: post.topic, reviewable_by_moderator: true,
+          payload: { post_cooked: post.cooked, external_error: reason }
+        )
+      end
     end
 
     def notify_poster(post)
diff --git a/lib/discourse_akismet/users_bouncer.rb b/lib/discourse_akismet/users_bouncer.rb
index 657aafb..4ca4206 100644
--- a/lib/discourse_akismet/users_bouncer.rb
+++ b/lib/discourse_akismet/users_bouncer.rb
@@ -42,8 +42,14 @@ module DiscourseAkismet
       move_to_state(user, 'confirmed_spam')
     end
 
-    def mark_as_clear(user)
-      move_to_state(user, 'confirmed_ham')
+    def mark_as_errored(user, reason)
+      super do 
+        ReviewableAkismetUser.needs_review!(
+          target: user, reviewable_by_moderator: true,
+          created_by: spam_reporter,
+          payload: { username: user.username, name: user.name, email: user.email, bio: user.user_profile.bio_raw, external_error: reason }
+        )
+      end
     end
 
     def args_for(user)
diff --git a/models/reviewable_akismet_post.rb b/models/reviewable_akismet_post.rb
index 944e590..a5e6bdc 100644
--- a/models/reviewable_akismet_post.rb
+++ b/models/reviewable_akismet_post.rb
@@ -23,6 +23,9 @@ class ReviewableAkismetPost < Reviewable
     bouncer.submit_feedback(post, 'spam')
     log_confirmation(performed_by, 'confirmed_spam')
 
+    # Double-check the original post is deleted
+    PostDestroyer.new(performed_by, post).destroy unless post.deleted_at?
+
     successful_transition :approved, :agreed
   end
 
diff --git a/serializers/reviewable_akismet_post_serializer.rb b/serializers/reviewable_akismet_post_serializer.rb
index fbc919b..2d29b9a 100644
--- a/serializers/reviewable_akismet_post_serializer.rb
+++ b/serializers/reviewable_akismet_post_serializer.rb
@@ -3,5 +3,5 @@
 require_dependency 'reviewable_serializer'
 
 class ReviewableAkismetPostSerializer < ReviewableSerializer
-  payload_attributes :post_cooked
+  payload_attributes :post_cooked, :external_error
 end
diff --git a/serializers/reviewable_akismet_user_serializer.rb b/serializers/reviewable_akismet_user_serializer.rb
index 00580a3..5eb7a3f 100644
--- a/serializers/reviewable_akismet_user_serializer.rb

[... diff too long, it was truncated ...]

GitHub sha: de22f5f2

This commit appears in #55 which was approved by eviltrout. It was merged by jbrw.