Merge pull request #8907 from mfo/US/dolist-silence-some-errors

amelioration(dolist): ne log erreurs pas les erreurs dans sentry lorsque le contact chez dolist est injoingable ou hardbounce
This commit is contained in:
mfo 2023-04-19 11:49:01 +00:00 committed by GitHub
commit 73184c6dac
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
4 changed files with 90 additions and 12 deletions

View file

@ -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 ladresse 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 ladresse e-mail est en erreur. L'adresse e-mail nexiste pas ou n'existe plus.
]
class << self
def save_rate_limit_headers(headers)
self.limit_remaining = headers["X-Rate-Limit-Remaining"].to_i
@ -124,8 +137,43 @@ 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 dun FAI).
# Dans ce cas lAPI denvoi transactionnel renvoie différentes erreurs.
# Pour connaitre exactement le statut dune 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_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
def ignorable_contact_status?(contact_status)
IGNORABLE_CONTACT_STATUSES.include?(contact_status)
end
def format_url(base)
format(base, account_id: account_id)
end

View file

@ -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

View file

@ -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'])
@ -15,12 +18,18 @@ 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}"
_, invalid_contact_status = client.ignorable_error?(response, mail)
if invalid_contact_status
raise Dolist::IgnorableError.new("DoList delivery error. contact unreachable: #{invalid_contact_status}")
else
fail "DoList delivery error. Body: #{response}"
end
end
end
end

View file

@ -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