Skip to content

Commit 607a919

Browse files
authored
Merge pull request #19855 from mozilla/fxa-12710
fix(emails): Add more email logging
2 parents a20b6fe + d2e24aa commit 607a919

File tree

3 files changed

+107
-22
lines changed

3 files changed

+107
-22
lines changed

libs/accounts/email-sender/src/email-sender.spec.ts

Lines changed: 46 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -345,6 +345,52 @@ describe('EmailSender', () => {
345345
);
346346
expect(mockSentryCaptureException).toHaveBeenCalledWith(appError);
347347
});
348+
349+
it('logs SMTP error details including rejected recipients', async () => {
350+
config = {
351+
...config,
352+
retry: {
353+
maxAttempts: 0,
354+
backOffMs: 100,
355+
jitter: 0,
356+
maxDelayMs: 10_000,
357+
},
358+
};
359+
360+
emailSender = new EmailSender(
361+
config,
362+
mockBounces,
363+
mockStatsd,
364+
mockLogger
365+
);
366+
367+
const smtpError = Object.assign(new Error('Invalid RCPT TO'), {
368+
response: '501 Invalid RCPT TO address provided',
369+
responseCode: 501,
370+
rejected: ['bad@example.com'],
371+
accepted: ['good@example.com'],
372+
});
373+
374+
const emailWithCc = {
375+
...defaultMockEmail,
376+
cc: ['cc@example.com'],
377+
};
378+
379+
mockTransport.sendMail.mockRejectedValueOnce(smtpError);
380+
381+
await expect(emailSender.send(emailWithCc)).rejects.toThrow();
382+
383+
expect(mockLogger.error).toHaveBeenCalledWith(
384+
'mailer.send.error',
385+
expect.objectContaining({
386+
smtpResponse: '501 Invalid RCPT TO address provided',
387+
smtpResponseCode: 501,
388+
rejectedRecipients: ['bad@example.com'],
389+
acceptedRecipients: ['good@example.com'],
390+
cc: ['cc@example.com'],
391+
})
392+
);
393+
});
348394
});
349395
describe('buildHeaders', () => {
350396
it('builds basic headers without SES configuration', async () => {

libs/accounts/email-sender/src/email-sender.ts

Lines changed: 41 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -345,41 +345,62 @@ export class EmailSender {
345345

346346
const backoffDelay = this.calculateBackoffDelay(attempt);
347347

348-
this.log.warn('mailer.send.retry', {
348+
const retryLogDetails: Record<string, unknown> = {
349349
err: err.message,
350350
to: email.to,
351351
template: email.template,
352352
retryAttempt: attempt,
353353
nextAttempt: attempt + 1,
354354
backoffMs: Math.round(backoffDelay),
355-
});
355+
};
356+
357+
if (err.response) {
358+
retryLogDetails['smtpResponse'] = err.response;
359+
}
360+
if (err.responseCode) {
361+
retryLogDetails['smtpResponseCode'] = err.responseCode;
362+
}
363+
if (err.rejected?.length) {
364+
retryLogDetails['rejectedRecipients'] = err.rejected;
365+
}
366+
367+
this.log.warn('mailer.send.retry', retryLogDetails);
356368

357369
// Wait for backoff period, then retry
358370
await new Promise((resolve) => setTimeout(resolve, backoffDelay));
359371
return this.sendMail(email, ++attempt);
360372
}
361373

362-
// This is the final failure - log and capture to Sentry
374+
const smtpErrorDetails: Record<string, unknown> = {
375+
err: err.message,
376+
to: email.to,
377+
template: email.template,
378+
isRetry,
379+
retryAttempt: attempt,
380+
};
381+
382+
if (email.cc?.length) {
383+
smtpErrorDetails['cc'] = email.cc;
384+
}
385+
if (err.response) {
386+
smtpErrorDetails['smtpResponse'] = err.response;
387+
}
388+
if (err.responseCode) {
389+
smtpErrorDetails['smtpResponseCode'] = err.responseCode;
390+
}
391+
if (err.rejected?.length) {
392+
smtpErrorDetails['rejectedRecipients'] = err.rejected;
393+
}
394+
if (err.accepted?.length) {
395+
smtpErrorDetails['acceptedRecipients'] = err.accepted;
396+
}
363397
if (isAppError(err)) {
364-
this.log.error('mailer.send.error', {
365-
err: err.message,
366-
code: err.code,
367-
errno: err.errno,
368-
to: email.to,
369-
template: email.template,
370-
isRetry,
371-
retryAttempt: attempt,
372-
});
373-
} else {
374-
this.log.error('mailer.send.error', {
375-
err: err.message,
376-
to: email.to,
377-
template: email.template,
378-
isRetry,
379-
retryAttempt: attempt,
380-
});
398+
smtpErrorDetails['code'] = err.code;
399+
smtpErrorDetails['errno'] = err.errno;
381400
}
382401

402+
this.log.error('mailer.send.error', smtpErrorDetails);
403+
383404
// Only capture to Sentry on final failure
384405
Sentry.captureException(err);
385406

packages/fxa-auth-server/lib/senders/email.js

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -569,14 +569,32 @@ module.exports = function (log, config, bounces, statsd) {
569569
await new Promise((resolve, reject) => {
570570
this.mailer.sendMail(emailConfig, (err, status) => {
571571
if (err) {
572-
log.error('mailer.send.error', {
572+
const errorLogDetails = {
573573
err: err.message,
574574
code: err.code,
575575
errno: err.errno,
576576
message: status && status.message,
577577
to: emailConfig && emailConfig.to,
578578
template,
579-
});
579+
};
580+
581+
if (emailConfig && emailConfig.cc) {
582+
errorLogDetails.cc = emailConfig.cc;
583+
}
584+
if (err.response) {
585+
errorLogDetails.smtpResponse = err.response;
586+
}
587+
if (err.responseCode) {
588+
errorLogDetails.smtpResponseCode = err.responseCode;
589+
}
590+
if (err.rejected && err.rejected.length) {
591+
errorLogDetails.rejectedRecipients = err.rejected;
592+
}
593+
if (err.accepted && err.accepted.length) {
594+
errorLogDetails.acceptedRecipients = err.accepted;
595+
}
596+
597+
log.error('mailer.send.error', errorLogDetails);
580598

581599
return reject(err);
582600
}

0 commit comments

Comments
 (0)