Browse Source

Fixes #5352 - WhatsApp Business channel is not flagged with error state in case of permanent errors.

Co-authored-by: Florian Liebe <fl@zammad.com>
Co-authored-by: Tobias Schäfer <ts@zammad.com>
Florian Liebe 5 months ago
parent
commit
725c7d39db

+ 8 - 1
app/assets/javascripts/app/views/whatsapp/index.jst.eco

@@ -18,11 +18,18 @@
       <div class="action <% if channel.active isnt true: %>is-inactive<% end %>" data-id="<%= channel.id %>">
         <div class="action-block action-row">
           <h2>
-            <%- @Icon('status', 'supergood-color inline') %>
+            <%- @Icon('status', channel.status_out + " inline") %>
             <%= channel.options.name %>
             <span class="text-muted"><%= channel.options.phone_number %></span>
          </h2>
         </div>
+        <% if !_.isEmpty(channel.last_log_out): %>
+        <div class="action-block action-row">
+          <div class="alert alert--danger">
+            <%= channel.last_log_out %>
+          </div>
+        </div>
+        <% end %>
         <div class="action-flow action-flow--row">
           <div class="action-block">
             <h3><%- @T('Messages') %></h3>

+ 1 - 1
app/models/channel.rb

@@ -276,7 +276,7 @@ send via account
   def handle_delivery_error!(error, adapter)
     message = "Can't use Channel::Driver::#{adapter.to_classname}: #{error.inspect}"
 
-    if error.respond_to?(:retryable?) && !error.retryable?
+    if error.respond_to?(:retryable?) && error.retryable?
       self.status_out = 'ok'
       self.last_log_out = ''
     else

+ 10 - 3
lib/whatsapp/client.rb

@@ -48,9 +48,16 @@ class Whatsapp::Client
     def retryable?
       return true if !original_error
 
-      # WhatsApp API returns code 100 for various input errors
-      # Such as too long body or too large attachment
-      original_error.code != 100
+      # https://developers.facebook.com/docs/graph-api/guides/error-handling
+      recoverable_errors = [
+        130_472, # User's number is part of an experiment'
+        131_021, # Recipient cannot be sender'
+        131_026, # Message undeliverable'
+        131_047, # Re-engagement message
+        131_052, # Media download error'
+        131_053  # Media upload error'
+      ]
+      recoverable_errors.include?(original_error.code)
     end
   end
 

+ 36 - 0
lib/whatsapp/webhook/concerns/handles_error.rb

@@ -0,0 +1,36 @@
+# Copyright (C) 2012-2024 Zammad Foundation, https://zammad-foundation.org/
+
+module Whatsapp::Webhook
+  module Concerns::HandlesError
+    private
+
+    def error
+      raise NotImplementedError
+    end
+
+    def handle_error
+      # https://developers.facebook.com/docs/whatsapp/cloud-api/support/error-codes
+      #
+      # Log any error status to the Rails log. Update the channel status on
+      # any unrecoverable error - errors that need action from an administator
+      # and block the channel from sending or receiving messages.
+
+      Rails.logger.error "WhatsApp channel (#{@channel.options[:callback_url_uuid]}) - failed message: #{error[:title]} (#{error[:code]})"
+
+      recoverable_errors = [
+        130_472, # User's number is part of an experiment'
+        131_021, # Recipient cannot be sender'
+        131_026, # Message undeliverable'
+        131_047, # Re-engagement message
+        131_052, # Media download error'
+        131_053  # Media upload error'
+      ]
+      return if recoverable_errors.include?(error[:code])
+
+      @channel.update!(
+        status_out:   'error',
+        last_log_out: "#{error[:title]} (#{error[:code]})",
+      )
+    end
+  end
+end

+ 3 - 26
lib/whatsapp/webhook/message/status/failed.rb

@@ -1,6 +1,8 @@
 # Copyright (C) 2012-2024 Zammad Foundation, https://zammad-foundation.org/
 
 class Whatsapp::Webhook::Message::Status::Failed < Whatsapp::Webhook::Message::Status
+  include Whatsapp::Webhook::Concerns::HandlesError
+
   private
 
   def create_article?
@@ -42,31 +44,6 @@ class Whatsapp::Webhook::Message::Status::Failed < Whatsapp::Webhook::Message::S
   end
 
   def error
-    @error = status[:errors].first
-  end
-
-  def handle_error
-    # https://developers.facebook.com/docs/whatsapp/cloud-api/support/error-codes
-    #
-    # Log any error status to the Rails log. Update the channel status on
-    # any unrecoverable error - errors that need action from an administator
-    # and block the channel from sending or receiving messages.
-
-    Rails.logger.error "WhatsApp channel (#{@channel.options[:callback_url_uuid]}) - failed status message: #{error[:title]} (#{error[:code]})"
-
-    recoverable_errors = [
-      130_472, # User's number is part of an experiment'
-      131_021, # Recipient cannot be sender'
-      131_026, # Message undeliverable'
-      131_047, # Re-engagement message
-      131_052, # Media download error'
-      131_053  # Media upload error'
-    ]
-    return if recoverable_errors.include?(error[:code])
-
-    @channel.update!(
-      status_out:   'error',
-      last_log_out: "#{error[:title]} (#{error[:code]})",
-    )
+    status[:errors].first
   end
 end

+ 9 - 1
lib/whatsapp/webhook/payload.rb

@@ -3,6 +3,7 @@
 module Whatsapp::Webhook
   class Payload
     include Whatsapp::Webhook::Concerns::HasChannel
+    include Whatsapp::Webhook::Concerns::HandlesError
 
     def initialize(json:, uuid:, signature:)
       channel = find_channel!(uuid)
@@ -34,7 +35,10 @@ module Whatsapp::Webhook
     private
 
     def process_message
-      raise ProcessableError if message_error?
+      if message_error?
+        handle_error
+        raise ProcessableError
+      end
 
       type = @data[:entry].first[:changes].first[:value][:messages].first[:type]
       klass = "Whatsapp::Webhook::Message::#{type.capitalize}"
@@ -71,6 +75,10 @@ module Whatsapp::Webhook
       @data[:entry].first[:changes].first[:value][:messages].first.key?(:errors)
     end
 
+    def error
+      @data[:entry].first[:changes].first[:value][:messages].first[:errors].first
+    end
+
     def message?
       @data[:entry].first[:changes].first[:value].key?(:messages)
     end

+ 29 - 9
spec/lib/whatsapp/webhook/payload_spec.rb

@@ -133,15 +133,9 @@ RSpec.describe Whatsapp::Webhook::Payload, :aggregate_failures, current_user_id:
                   },
                   errors:    [
                     {
-                      message:       '(#130429) Rate limit hit',
-                      type:          'OAuthException',
-                      code:          130_429,
-                      error_data:    {
-                        messaging_product: 'whatsapp',
-                        details:           '<DETAILS>'
-                      },
-                      error_subcode: 2_494_055,
-                      fbtrace_id:    'Az8or2yhqkZfEZ-_4Qn_Bam'
+                      code:    131_051,
+                      details: 'Message type is not currently supported',
+                      title:   'Unsupported message type'
                     }
                   ],
                   type:      type
@@ -156,6 +150,32 @@ RSpec.describe Whatsapp::Webhook::Payload, :aggregate_failures, current_user_id:
       it 'raises ProcessableError' do
         expect { described_class.new(json:, uuid:, signature:).process }.to raise_error(described_class::ProcessableError)
       end
+
+      it 'logs the error' do
+        allow(Rails.logger).to receive(:error)
+
+        begin
+          described_class.new(json:, uuid:, signature:).process
+        rescue
+          # noop
+        end
+
+        expect(Rails.logger).to have_received(:error)
+          .with("WhatsApp channel (#{channel.options[:callback_url_uuid]}) - failed message: Unsupported message type (131051)")
+      end
+
+      it 'updates the channel status' do
+        begin
+          described_class.new(json:, uuid:, signature:).process
+        rescue
+          # noop
+        end
+
+        expect(channel.reload).to have_attributes(
+          status_out:   'error',
+          last_log_out: 'Unsupported message type (131051)',
+        )
+      end
     end
 
     context 'when an unsupported type is used' do