diff --git a/src/BuildingBlocks/Mailing/Extensions.cs b/src/BuildingBlocks/Mailing/Extensions.cs index fc1625eda4..1915916501 100644 --- a/src/BuildingBlocks/Mailing/Extensions.cs +++ b/src/BuildingBlocks/Mailing/Extensions.cs @@ -28,7 +28,8 @@ public static IServiceCollection AddHeroMailing(this IServiceCollection services { return new SendGridMailService( sp.GetRequiredService>(), - sp.GetRequiredService()); + sp.GetRequiredService(), + sp.GetRequiredService>()); } return new SmtpMailService(sp.GetRequiredService>(), sp.GetRequiredService>()); }); diff --git a/src/BuildingBlocks/Mailing/Services/SendGridMailService.cs b/src/BuildingBlocks/Mailing/Services/SendGridMailService.cs index b25ddd1317..96334b4124 100644 --- a/src/BuildingBlocks/Mailing/Services/SendGridMailService.cs +++ b/src/BuildingBlocks/Mailing/Services/SendGridMailService.cs @@ -1,3 +1,4 @@ +using Microsoft.Extensions.Logging; using Microsoft.Extensions.Options; using SendGrid; using SendGrid.Helpers.Mail; @@ -10,13 +11,16 @@ public sealed class SendGridMailService : IMailService { private readonly MailOptions _settings; private readonly ISendGridClient _client; + private readonly ILogger _logger; - public SendGridMailService(IOptions settings, ISendGridClient client) + public SendGridMailService(IOptions settings, ISendGridClient client, ILogger logger) { ArgumentNullException.ThrowIfNull(settings); ArgumentNullException.ThrowIfNull(client); + ArgumentNullException.ThrowIfNull(logger); _settings = settings.Value; _client = client; + _logger = logger; } public async Task SendAsync(MailRequest request, CancellationToken ct) @@ -40,9 +44,41 @@ public async Task SendAsync(MailRequest request, CancellationToken ct) ConfigureRecipients(msg, request); AddAttachments(msg, request); - await _client.SendEmailAsync(msg, ct).ConfigureAwait(false); + var response = await _client.SendEmailAsync(msg, ct).ConfigureAwait(false); + + // The client is built with HttpErrorAsException=false, so a non-2xx reply (bad key, rejected + // recipient, rate limit) comes back as a Response instead of throwing. A silently discarded + // failure makes the caller believe the mail was delivered, so it must never be ignored. + if (IsSuccess(response.StatusCode)) + { + return; + } + + var status = (int)response.StatusCode; + var body = response.Body is not null + ? await response.Body.ReadAsStringAsync(ct).ConfigureAwait(false) + : string.Empty; + + // Only retry failures that can plausibly succeed later — 429 (rate limited) and 5xx + // (SendGrid-side). Throwing routes these back through the caller's Hangfire automatic retry. + // A permanent 4xx (bad API key, rejected recipient) will never succeed on retry, so log it + // loudly and return instead of throwing — otherwise every enqueued send retries ~10x and + // floods the dead-letter queue with attempts that cannot succeed. + if (status == 429 || status >= 500) + { + throw new InvalidOperationException( + $"SendGrid transiently failed the message with status {status}. {body}".TrimEnd()); + } + + _logger.LogError( + "SendGrid permanently rejected the message with status {StatusCode}. {Body}", + status, + body); } + private static bool IsSuccess(System.Net.HttpStatusCode statusCode) => + (int)statusCode is >= 200 and < 300; + private void ValidateConfiguration() { if (_settings.SendGrid?.ApiKey is null) diff --git a/src/Tests/Framework.Tests/Mailing/SendGridMailServiceTests.cs b/src/Tests/Framework.Tests/Mailing/SendGridMailServiceTests.cs new file mode 100644 index 0000000000..87150fec8b --- /dev/null +++ b/src/Tests/Framework.Tests/Mailing/SendGridMailServiceTests.cs @@ -0,0 +1,77 @@ +using System.Net; +using FSH.Framework.Mailing; +using FSH.Framework.Mailing.Services; +using Microsoft.Extensions.Logging.Abstractions; +using Microsoft.Extensions.Options; +using NSubstitute; +using SendGrid; +using SendGrid.Helpers.Mail; + +namespace Framework.Tests.Mailing; + +// REL-01: SendGridMailService must not swallow a non-2xx SendGrid reply (the client is built with +// HttpErrorAsException=false, so a failure comes back as a Response, not an exception). Transient +// failures (429/5xx) throw so the caller's Hangfire job retries; permanent failures (other 4xx) are +// logged and returned — retrying a bad key / rejected recipient only floods the dead-letter queue. +public sealed class SendGridMailServiceTests +{ + private static SendGridMailService BuildService(ISendGridClient client) + { + var options = Options.Create(new MailOptions + { + UseSendGrid = true, + From = "noreply@x.com", + SendGrid = new SendGridOptions { ApiKey = "sg-key", From = "noreply@x.com" }, + }); + return new SendGridMailService(options, client, NullLogger.Instance); + } + + private static ISendGridClient ClientReturning(HttpStatusCode status) + { + var client = Substitute.For(); + client.SendEmailAsync(Arg.Any(), Arg.Any()) + .Returns(new Response(status, null, null)); + return client; + } + + private static MailRequest ValidRequest() => + new(to: ["dest@x.com"], subject: "hi", body: "body"); + + [Theory] + [InlineData(HttpStatusCode.TooManyRequests)] // 429 — rate limited + [InlineData(HttpStatusCode.InternalServerError)] // 500 — SendGrid-side + [InlineData(HttpStatusCode.ServiceUnavailable)] // 503 — SendGrid-side + public async Task SendAsync_When_TransientFailure_Should_Throw_ForRetry(HttpStatusCode status) + { + var service = BuildService(ClientReturning(status)); + + var send = async () => await service.SendAsync(ValidRequest(), CancellationToken.None); + + // Throwing routes the send back through the caller's Hangfire automatic retry. + await send.ShouldThrowAsync(); + } + + [Theory] + [InlineData(HttpStatusCode.Unauthorized)] // 401 — bad API key + [InlineData(HttpStatusCode.BadRequest)] // 400 — rejected recipient / malformed + [InlineData(HttpStatusCode.Forbidden)] // 403 — sender not verified + public async Task SendAsync_When_PermanentRejection_Should_NotThrow_ToAvoidRetryStorm(HttpStatusCode status) + { + var service = BuildService(ClientReturning(status)); + + var send = async () => await service.SendAsync(ValidRequest(), CancellationToken.None); + + // Logged as an error (surfaced to ops) but not thrown — a retry cannot make it succeed. + await send.ShouldNotThrowAsync(); + } + + [Fact] + public async Task SendAsync_When_SendGridReturnsAccepted_Should_Complete() + { + var service = BuildService(ClientReturning(HttpStatusCode.Accepted)); + + var send = async () => await service.SendAsync(ValidRequest(), CancellationToken.None); + + await send.ShouldNotThrowAsync(); + } +}