From 8fa2bbb67d0fe97ab488eee29ed4943c58d06572 Mon Sep 17 00:00:00 2001 From: Martin Date: Mon, 3 Apr 2023 16:25:45 +0200 Subject: [PATCH 1/2] amelioration(dolist): ne log erreurs pas les erreurs dans sentry lorsque le contact chez dolist est injoingable ou hardbounce --- app/lib/dolist/api.rb | 38 +++++++++++++++++++++++++ config/initializers/dolist.rb | 15 ++++++++-- spec/mailers/application_mailer_spec.rb | 31 ++++++++++++++------ 3 files changed, 73 insertions(+), 11 deletions(-) diff --git a/app/lib/dolist/api.rb b/app/lib/dolist/api.rb index 231a328f5..48d07d5fc 100644 --- a/app/lib/dolist/api.rb +++ b/app/lib/dolist/api.rb @@ -4,6 +4,7 @@ class Dolist::API CONTACT_URL = "https://apiv9.dolist.net/v1/contacts/read?AccountID=%{account_id}" EMAIL_LOGS_URL = "https://apiv9.dolist.net/v1/statistics/email/sendings/transactional/search?AccountID=%{account_id}" EMAIL_KEY = 7 + STATUS_KEY = 72 DOLIST_WEB_DASHBOARD = "https://campaign.dolist.net/#/%{account_id}/contacts/%{contact_id}/sendings" EMAIL_MESSAGES_ADRESSES_REPLIES = "https://apiv9.dolist.net/v1/email/messages/addresses/replies?AccountID=%{account_id}" EMAIL_MESSAGES_ADRESSES_PACKSENDERS = "https://apiv9.dolist.net/v1/email/messages/addresses/packsenders?AccountID=%{account_id}" @@ -13,6 +14,18 @@ class Dolist::API class_attribute :limit_remaining, :limit_reset_at + # those code are just undocumented + IGNORABLE_API_ERROR_CODE = [ + "458", + "402" + ] + + # see: https://usercampaign.dolist.net/wp-content/uploads/2022/12/Comprendre-les-Opt-out-tableau-v2.pdf + IGNORABLE_CONTACT_STATUSES = [ + "4", # Le serveur distant n'accepte pas le mail car il identifie que l’adresse e-mail est en erreur. + "7" # Suite à un envoi, le serveur distant accepte le mail dans un premier temps mais envoie une erreur définitive car l’adresse e-mail est en erreur. L'adresse e-mail n’existe pas ou n'existe plus. + ] + class << self def save_rate_limit_headers(headers) self.limit_remaining = headers["X-Rate-Limit-Remaining"].to_i @@ -124,6 +137,31 @@ class Dolist::API get format_url(EMAIL_MESSAGES_ADRESSES_REPLIES) end + # Une adresse e-mail peut ne pas être adressable pour différentes raisons (injoignable, plainte pour spam, blocage d’un FAI). + # Dans ce cas l’API d’envoi transactionnel renvoie différentes erreurs. + # Pour connaitre exactement le statut d’une adresse, je vous invite à récupérer le champ 72 du contact à partir de son adresse e-mail avec la méthode https://api.dolist.com/documentation/index.html#/40e7751d00dc3-rechercher-un-contact + # + # La liste des différents statuts est disponible sur https://usercampaign.dolist.net/wp-content/uploads/2022/12/Comprendre-les-Opt-out-tableau-v2.pdf + def fetch_contact_status(email_address) + url = format(Dolist::API::CONTACT_URL, account_id: account_id) + body = { + Query: { + FieldValueList: [{ ID: 7, Value: email_address }], + OutputFieldIDList: [72] + } + }.to_json + + post(url, body)["FieldList"].find { _1['ID'] == 72 }['Value'] + end + + def ignorable_api_error_code?(api_error_code) + IGNORABLE_API_ERROR_CODE.include?(api_error_code) + end + + def ignorable_contact_status?(contact_status) + IGNORABLE_CONTACT_STATUSES.include?(contact_status) + end + private def format_url(base) diff --git a/config/initializers/dolist.rb b/config/initializers/dolist.rb index 24471b651..3e04792aa 100644 --- a/config/initializers/dolist.rb +++ b/config/initializers/dolist.rb @@ -15,12 +15,21 @@ ActiveSupport.on_load(:action_mailer) do def initialize(mail); end def deliver!(mail) - response = Dolist::API.new.send_email(mail) - + client = Dolist::API.new + response = client.send_email(mail) if response&.dig("Result") mail.message_id = response.dig("Result") else - fail "DoList delivery error. Body: #{response}" + error_code = response&.dig("ResponseStatus", "ErrorCode") + + contact_status = if client.ignorable_api_error_code?(error_code) + client.fetch_contact_status(mail.to.first) + else + nil + end + if !client.ignorable_contact_status?(contact_status) + fail "DoList delivery error. Body: #{response}" + end end end end diff --git a/spec/mailers/application_mailer_spec.rb b/spec/mailers/application_mailer_spec.rb index 347d845af..53b33e2b5 100644 --- a/spec/mailers/application_mailer_spec.rb +++ b/spec/mailers/application_mailer_spec.rb @@ -28,16 +28,31 @@ RSpec.describe ApplicationMailer, type: :mailer do describe 'dealing with Dolist API error' do let(:dossier) { create(:dossier, procedure: create(:simple_procedure)) } - before do - ActionMailer::Base.delivery_method = :dolist_api - api_error_response = { "ResponseStatus": { "ErrorCode": "Forbidden", "Message": "Blocked non authorized request", "Errors": [] } } - allow_any_instance_of(Dolist::API).to receive(:send_email).and_return(api_error_response) - end subject { DossierMailer.with(dossier:).notify_new_draft.deliver_now } + context 'not ignored error' do + before do + ActionMailer::Base.delivery_method = :dolist_api + api_error_response = { "ResponseStatus": { "ErrorCode": "Forbidden", "Message": "Blocked non authorized request", "Errors": [] } } + allow_any_instance_of(Dolist::API).to receive(:send_email).and_return(api_error_response) + end - it 'raise classic error to retry' do - expect { subject }.to raise_error(MailDeliveryError) - expect(EmailEvent.dolist_api.dispatch_error.count).to eq(1) + it 'raise classic error to retry' do + expect { subject }.to raise_error(MailDeliveryError) + expect(EmailEvent.dolist_api.dispatch_error.count).to eq(1) + end + end + + context 'ignored error' do + before do + ActionMailer::Base.delivery_method = :dolist_api + api_error_response = { "ResponseStatus" => { "ErrorCode" => "458", "Message" => "The contact is disabled.", "Errors" => [] } } + allow_any_instance_of(Dolist::API).to receive(:send_email).and_return(api_error_response) + allow_any_instance_of(Dolist::API).to receive(:fetch_contact_status).with(dossier.user.email).and_return("7") + end + + it 'does not raise' do + expect { subject }.not_to raise_error + end end end From 534ce34f87eddb1d3c0462c4546d30ca683dd701 Mon Sep 17 00:00:00 2001 From: Martin Date: Tue, 18 Apr 2023 16:15:47 +0200 Subject: [PATCH 2/2] =?UTF-8?q?amelioration(Dolist::ApiSender):=20l=C3=A8v?= =?UTF-8?q?e=20une=20Dolist::IgnorableError=20afin=20de=20l'inscrire=20dan?= =?UTF-8?q?s=20l'historique=20des=20EmailEvent?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- app/lib/dolist/api.rb | 14 ++++++++++++-- app/mailers/concerns/mailer_monitoring_concern.rb | 8 +++++++- config/initializers/dolist.rb | 12 ++++++------ 3 files changed, 25 insertions(+), 9 deletions(-) diff --git a/app/lib/dolist/api.rb b/app/lib/dolist/api.rb index 48d07d5fc..388271f67 100644 --- a/app/lib/dolist/api.rb +++ b/app/lib/dolist/api.rb @@ -154,6 +154,18 @@ class Dolist::API post(url, body)["FieldList"].find { _1['ID'] == 72 }['Value'] end + def ignorable_error?(response, mail) + error_code = response&.dig("ResponseStatus", "ErrorCode") + invalid_contact_status = if ignorable_api_error_code?(error_code) + fetch_contact_status(mail.to.first) + else + nil + end + [error_code, invalid_contact_status] + end + + private + def ignorable_api_error_code?(api_error_code) IGNORABLE_API_ERROR_CODE.include?(api_error_code) end @@ -162,8 +174,6 @@ class Dolist::API IGNORABLE_CONTACT_STATUSES.include?(contact_status) end - private - def format_url(base) format(base, account_id: account_id) end diff --git a/app/mailers/concerns/mailer_monitoring_concern.rb b/app/mailers/concerns/mailer_monitoring_concern.rb index 6cbd60247..f140c20fa 100644 --- a/app/mailers/concerns/mailer_monitoring_concern.rb +++ b/app/mailers/concerns/mailer_monitoring_concern.rb @@ -19,12 +19,18 @@ module MailerMonitoringConcern end end + rescue_from Dolist::IgnorableError, with: :log_delivery_error + def log_and_raise_delivery_error(exception) - EmailEvent.create_from_message!(message, status: "dispatch_error") + log_delivery_error(exception) Sentry.capture_exception(exception, extra: { to: message.to, subject: message.subject }) # re-raise another error so job will retry later raise MailDeliveryError.new(exception) end + + def log_delivery_error(exception) + EmailEvent.create_from_message!(message, status: "dispatch_error") + end end end diff --git a/config/initializers/dolist.rb b/config/initializers/dolist.rb index 3e04792aa..cdf1f2384 100644 --- a/config/initializers/dolist.rb +++ b/config/initializers/dolist.rb @@ -1,5 +1,8 @@ ActiveSupport.on_load(:action_mailer) do module Dolist + class IgnorableError < StandardError + end + class SMTP < ::Mail::SMTP def deliver!(mail) mail.from(ENV['DOLIST_NO_REPLY_EMAIL']) @@ -20,14 +23,11 @@ ActiveSupport.on_load(:action_mailer) do if response&.dig("Result") mail.message_id = response.dig("Result") else - error_code = response&.dig("ResponseStatus", "ErrorCode") + _, invalid_contact_status = client.ignorable_error?(response, mail) - contact_status = if client.ignorable_api_error_code?(error_code) - client.fetch_contact_status(mail.to.first) + if invalid_contact_status + raise Dolist::IgnorableError.new("DoList delivery error. contact unreachable: #{invalid_contact_status}") else - nil - end - if !client.ignorable_contact_status?(contact_status) fail "DoList delivery error. Body: #{response}" end end