FIX: stop raising exceptions when a post goes missing

app/jobs/regular/emit_web_hook_event.rb

@@ -6,34 +6,48 @@ module Jobs
       raise Discourse::InvalidParameters.new(:web_hook_id) unless args[:web_hook_id].present?
       raise Discourse::InvalidParameters.new(:event_type) unless args[:event_type].present?
 
-      @web_hook = WebHook.find(args[:web_hook_id])
+      args = args.dup
 
-      unless args[:event_type] == 'ping'
-        return unless @web_hook.active?
-        return if @web_hook.group_ids.present? && (args[:group_id].present? ||
-          !@web_hook.group_ids.include?(args[:group_id]))
-        return if @web_hook.category_ids.present? && (!args[:category_id].present? ||
-          !@web_hook.category_ids.include?(args[:category_id]))
+      if args[:topic_id]
+        args[:topic_view] = TopicView.new(args[:topic_id], Discourse.system_user)
       end
 
-      @opts = args
+      if args[:post_id]
+        # deleted post so skip
+        return unless args[:post] = Post.find_by(id: args[:post_id])
+      end
 
-      web_hook_request
+      if args[:user_id]
+        return unless args[:user] = User.find_by(id: args[:user_id])
+      end
+
+      web_hook = WebHook.find(args[:web_hook_id])
+
+      unless args[:event_type] == 'ping'
+        return unless web_hook.active?
+        return if web_hook.group_ids.present? && (args[:group_id].present? ||
+          !web_hook.group_ids.include?(args[:group_id]))
+        return if web_hook.category_ids.present? && (!args[:category_id].present? ||
+          !web_hook.category_ids.include?(args[:category_id]))
+      end
+
+      web_hook_request(args, web_hook)
     end
 
     private
 
-    def web_hook_request
-      uri = URI(@web_hook.payload_url)
+    def web_hook_request(args, web_hook)
+
+      uri = URI(web_hook.payload_url)
       conn = Excon.new(uri.to_s,
-                       ssl_verify_peer: @web_hook.verify_certificate,
+                       ssl_verify_peer: web_hook.verify_certificate,
                        retry_limit: 0)
 
-      body = build_web_hook_body
-      web_hook_event = WebHookEvent.create!(web_hook_id: @web_hook.id)
+      body = build_web_hook_body(args, web_hook)
+      web_hook_event = WebHookEvent.create!(web_hook_id: web_hook.id)
 
       begin
-        content_type = case @web_hook.content_type
+        content_type = case web_hook.content_type
                        when WebHook.content_types['application/x-www-form-urlencoded']
                          'application/x-www-form-urlencoded'
                        else
@@ -48,12 +62,12 @@ module Jobs
           'User-Agent' => "Discourse/" + Discourse::VERSION::STRING,
           'X-Discourse-Instance' => Discourse.base_url,
           'X-Discourse-Event-Id' => web_hook_event.id,
-          'X-Discourse-Event-Type' => @opts[:event_type]
+          'X-Discourse-Event-Type' => args[:event_type]
         }
-        headers['X-Discourse-Event'] = @opts[:event_name].to_s if @opts[:event_name].present?
+        headers['X-Discourse-Event'] = args[:event_name].to_s if args[:event_name].present?
 
-        if @web_hook.secret.present?
-          headers['X-Discourse-Event-Signature'] = "sha256=" + OpenSSL::HMAC.hexdigest("sha256", @web_hook.secret, body)
+        if web_hook.secret.present?
+          headers['X-Discourse-Event-Signature'] = "sha256=" + OpenSSL::HMAC.hexdigest("sha256", web_hook.secret, body)
         end
 
         now = Time.zone.now
@@ -68,33 +82,29 @@ module Jobs
                                         response_headers: MultiJson.dump(response.headers),
                                         response_body: response.body,
                                         duration: ((Time.zone.now - now) * 1000).to_i)
-      MessageBus.publish("/web_hook_events/#{@web_hook.id}", {
+      MessageBus.publish("/web_hook_events/#{web_hook.id}", {
         web_hook_event_id: web_hook_event.id,
-        event_type: @opts[:event_type]
+        event_type: args[:event_type]
       }, user_ids: User.staff.pluck(:id))
     end
 
-    def build_web_hook_body
+    def build_web_hook_body(args, web_hook)
       body = {}
-      web_hook_user = Discourse.system_user
-      guardian = Guardian.new(web_hook_user)
+      guardian = Guardian.new(Discourse.system_user)
 
-      if @opts[:topic_id]
-        topic_view = TopicView.new(@opts[:topic_id], web_hook_user)
+      if topic_view = args[:topic_view]
         body[:topic] = TopicViewSerializer.new(topic_view, scope: guardian, root: false).as_json
       end
 
-      if @opts[:post_id]
-        post = Post.find(@opts[:post_id])
+      if post = args[:post]
         body[:post] = PostSerializer.new(post, scope: guardian, root: false).as_json
       end
 
-      if @opts[:user_id]
-        user = User.find(@opts[:user_id])
+      if user = args[:user]
         body[:user] = UserSerializer.new(user, scope: guardian, root: false).as_json
       end
 
-      body[:ping] = 'OK' if @opts[:event_type] == 'ping'
+      body[:ping] = 'OK' if args[:event_type] == 'ping'
 
       raise Discourse::InvalidParameters.new if body.empty?
 

spec/jobs/emit_web_hook_event_spec.rb

@@ -69,6 +69,12 @@ describe Jobs::EmitWebHookEvent do
       end.to change(WebHookEvent, :count).by(1)
     end
 
+    it 'skips silently on missing post' do
+      expect do
+        subject.execute(web_hook_id: post_hook.id, event_type: 'post', post_id: (Post.maximum(:id).to_i + 1))
+      end.not_to raise_error
+    end
+
     it 'sets up proper request headers' do
       Excon.stub({ url: "https://meta.discourse.org/webhook_listener" },
                  { headers: { test: 'string' }, body: 'OK', status: 200 })