feat(notifications): SMS notifications retry under SmsConfiguration.MaxRetries/RetryDelay (SMTP policy remains the Email + fallback policy) — wires the dead fields
Claude-Session: https://claude.ai/code/session_01MtdgwpEeCUn6cUA5f1LMPj
This commit is contained in:
@@ -28,19 +28,21 @@ public class SmsConfiguration
|
||||
/// <summary>
|
||||
/// Gets or sets the maximum number of delivery retries before parking.
|
||||
/// <para>
|
||||
/// RESERVED: the Notification Outbox dispatcher currently derives the retry policy
|
||||
/// (max-retries + interval) from the central SMTP configuration for <em>all</em>
|
||||
/// notification types — see the "retry reuses central SMTP max-retry-count and fixed
|
||||
/// interval" design decision and <c>NotificationOutboxActor.ResolveRetryPolicyAsync</c>.
|
||||
/// This per-SMS value is persisted/transported for forward-compatibility but is NOT
|
||||
/// yet read at dispatch time. Honoring it per-type is a deferred enhancement (it would
|
||||
/// supersede the shared-SMTP-policy decision and is not a silent behavioral change).
|
||||
/// LIVE for SMS: the Notification Outbox dispatcher selects the retry policy by
|
||||
/// notification <c>Type</c> — SMS notifications retry under this value (and
|
||||
/// <see cref="RetryDelay"/>), while Email (and every other type) uses the central SMTP
|
||||
/// policy. When no SMS configuration row exists the dispatcher falls back to the SMTP
|
||||
/// policy. A non-positive value is clamped to the outbox fallback (10) with a Warning so
|
||||
/// a misconfiguration cannot silently park SMS on the first transient failure. See
|
||||
/// <c>NotificationOutboxActor.ResolveRetryPoliciesAsync</c>.
|
||||
/// </para>
|
||||
/// </summary>
|
||||
public int MaxRetries { get; set; }
|
||||
/// <summary>
|
||||
/// Gets or sets the delay between retry attempts. RESERVED — see <see cref="MaxRetries"/>:
|
||||
/// the dispatcher currently uses the shared SMTP-derived retry interval for all types.
|
||||
/// Gets or sets the delay between retry attempts. LIVE for SMS — see
|
||||
/// <see cref="MaxRetries"/>: the dispatcher schedules the next SMS attempt with this delay
|
||||
/// (clamped to the 1-minute fallback if non-positive), falling back to the SMTP interval
|
||||
/// when no SMS configuration exists.
|
||||
/// </summary>
|
||||
public TimeSpan RetryDelay { get; set; }
|
||||
|
||||
|
||||
@@ -316,7 +316,7 @@ public class NotificationOutboxActor : ReceiveActor, IWithTimers
|
||||
return;
|
||||
}
|
||||
|
||||
var (maxRetries, retryDelay) = await ResolveRetryPolicyAsync(notificationRepository);
|
||||
var policies = await ResolveRetryPoliciesAsync(notificationRepository);
|
||||
|
||||
// Deliver the claimed batch with bounded parallelism. Delivery is I/O-bound
|
||||
// (multi-second SMTP/Twilio round trips), so a strictly sequential sweep caps
|
||||
@@ -333,7 +333,7 @@ public class NotificationOutboxActor : ReceiveActor, IWithTimers
|
||||
foreach (var notification in due)
|
||||
{
|
||||
deliveries.Add(DeliverGatedAsync(
|
||||
notification, now, maxRetries, retryDelay, adapters, deliveryGate, cancellationToken));
|
||||
notification, now, policies, adapters, deliveryGate, cancellationToken));
|
||||
}
|
||||
|
||||
// Each DeliverGatedAsync isolates its own faults, so WhenAll never throws.
|
||||
@@ -358,8 +358,7 @@ public class NotificationOutboxActor : ReceiveActor, IWithTimers
|
||||
private async Task DeliverGatedAsync(
|
||||
Notification notification,
|
||||
DateTimeOffset now,
|
||||
int maxRetries,
|
||||
TimeSpan retryDelay,
|
||||
RetryPolicies policies,
|
||||
IReadOnlyDictionary<NotificationType, INotificationDeliveryAdapter> adapters,
|
||||
SemaphoreSlim gate,
|
||||
CancellationToken cancellationToken)
|
||||
@@ -388,7 +387,7 @@ public class NotificationOutboxActor : ReceiveActor, IWithTimers
|
||||
using var scope = _serviceProvider.CreateScope();
|
||||
var outboxRepository = scope.ServiceProvider.GetRequiredService<INotificationOutboxRepository>();
|
||||
await DeliverOneAsync(
|
||||
notification, now, maxRetries, retryDelay, outboxRepository, adapters, cancellationToken)
|
||||
notification, now, policies, outboxRepository, adapters, cancellationToken)
|
||||
.ConfigureAwait(false);
|
||||
}
|
||||
catch (OperationCanceledException) when (cancellationToken.IsCancellationRequested)
|
||||
@@ -409,52 +408,107 @@ public class NotificationOutboxActor : ReceiveActor, IWithTimers
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Resolves the retry policy from the first SMTP configuration row. When no SMTP
|
||||
/// configuration exists, falls back to a conservative default — delivery itself will
|
||||
/// permanently fail in that case, so the policy only acts as a guard.
|
||||
/// A single channel's transient-failure retry policy: the max attempt count before a
|
||||
/// row parks and the fixed delay between attempts. Resolved once per sweep and selected
|
||||
/// per notification by <see cref="NotificationType"/> in <see cref="DeliverOneAsync"/>.
|
||||
/// </summary>
|
||||
/// <remarks>
|
||||
/// A non-positive <see cref="SmtpConfiguration.MaxRetries"/> (zero or negative)
|
||||
/// would otherwise satisfy <c>RetryCount >= maxRetries</c> on the very first transient
|
||||
/// failure and park the row without a single retry — silently halving the outbox's
|
||||
/// delivery guarantees. The same applies to a non-positive <c>RetryDelay</c>, which
|
||||
/// would burn-loop the dispatcher. Both values are clamped to the fallback constants
|
||||
/// with a Warning so an operator can spot the misconfiguration in logs.
|
||||
/// </remarks>
|
||||
private async Task<(int MaxRetries, TimeSpan RetryDelay)> ResolveRetryPolicyAsync(
|
||||
private readonly record struct RetryPolicy(int MaxRetries, TimeSpan RetryDelay);
|
||||
|
||||
/// <summary>
|
||||
/// The per-type retry policies resolved once per dispatch sweep: <see cref="SmtpPolicy"/>
|
||||
/// drives Email (and every non-SMS type), <see cref="SmsPolicy"/> drives SMS.
|
||||
/// </summary>
|
||||
private readonly record struct RetryPolicies(RetryPolicy SmtpPolicy, RetryPolicy SmsPolicy);
|
||||
|
||||
/// <summary>
|
||||
/// Resolves the per-type retry policies for the sweep: the SMTP policy from the first
|
||||
/// SMTP configuration row (drives Email and every non-SMS type), and the SMS policy from
|
||||
/// the <see cref="SmsConfiguration"/> row — falling back to the SMTP policy when no SMS
|
||||
/// configuration exists. Resolved once and reused for the whole batch;
|
||||
/// <see cref="DeliverOneAsync"/> selects the right policy by
|
||||
/// <see cref="Notification.Type"/>.
|
||||
/// </summary>
|
||||
private async Task<RetryPolicies> ResolveRetryPoliciesAsync(
|
||||
INotificationRepository notificationRepository)
|
||||
{
|
||||
var smtpPolicy = await ResolveSmtpPolicyAsync(notificationRepository);
|
||||
var smsPolicy = await ResolveSmsPolicyAsync(notificationRepository, smtpPolicy);
|
||||
return new RetryPolicies(smtpPolicy, smsPolicy);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Resolves the SMTP-derived retry policy from the first SMTP configuration row. When no
|
||||
/// SMTP configuration exists, falls back to the conservative default constants — delivery
|
||||
/// itself will permanently fail in that case, so the policy only acts as a guard.
|
||||
/// </summary>
|
||||
private async Task<RetryPolicy> ResolveSmtpPolicyAsync(
|
||||
INotificationRepository notificationRepository)
|
||||
{
|
||||
var configurations = await notificationRepository.GetAllSmtpConfigurationsAsync();
|
||||
var configuration = configurations.Count > 0 ? configurations[0] : null;
|
||||
if (configuration is null)
|
||||
{
|
||||
return (FallbackMaxRetries, FallbackRetryDelay);
|
||||
return new RetryPolicy(FallbackMaxRetries, FallbackRetryDelay);
|
||||
}
|
||||
|
||||
var maxRetries = configuration.MaxRetries;
|
||||
var retryDelay = configuration.RetryDelay;
|
||||
return ClampRetryPolicy(
|
||||
nameof(SmtpConfiguration), configuration.MaxRetries, configuration.RetryDelay);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Resolves the SMS-specific retry policy from the <see cref="SmsConfiguration"/> row,
|
||||
/// honoring its own <see cref="SmsConfiguration.MaxRetries"/>/<see cref="SmsConfiguration.RetryDelay"/>.
|
||||
/// When no SMS configuration row exists the SMTP policy (<paramref name="smtpFallback"/>)
|
||||
/// is reused so SMS delivery keeps the historical shared-SMTP behaviour until an operator
|
||||
/// configures SMS.
|
||||
/// </summary>
|
||||
private async Task<RetryPolicy> ResolveSmsPolicyAsync(
|
||||
INotificationRepository notificationRepository, RetryPolicy smtpFallback)
|
||||
{
|
||||
var configuration = await notificationRepository.GetSmsConfigurationAsync();
|
||||
if (configuration is null)
|
||||
{
|
||||
return smtpFallback;
|
||||
}
|
||||
|
||||
return ClampRetryPolicy(
|
||||
nameof(SmsConfiguration), configuration.MaxRetries, configuration.RetryDelay);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
/// Clamps a configured retry policy's non-positive values to the fallback constants,
|
||||
/// logging a Warning (parameterized by <paramref name="configName"/> so an operator can
|
||||
/// tell which configuration is misconfigured).
|
||||
/// </summary>
|
||||
/// <remarks>
|
||||
/// A non-positive <c>MaxRetries</c> (zero or negative) would otherwise satisfy
|
||||
/// <c>RetryCount >= maxRetries</c> on the very first transient failure and park the row
|
||||
/// without a single retry — silently halving the outbox's delivery guarantees. The same
|
||||
/// applies to a non-positive <c>RetryDelay</c>, which would burn-loop the dispatcher.
|
||||
/// </remarks>
|
||||
private RetryPolicy ClampRetryPolicy(string configName, int maxRetries, TimeSpan retryDelay)
|
||||
{
|
||||
if (maxRetries <= 0)
|
||||
{
|
||||
_logger.LogWarning(
|
||||
"SmtpConfiguration.MaxRetries={ConfiguredMaxRetries} is non-positive; " +
|
||||
"{ConfigName}.MaxRetries={ConfiguredMaxRetries} is non-positive; " +
|
||||
"clamping to FallbackMaxRetries={FallbackMaxRetries} so transient failures " +
|
||||
"actually retry before parking.",
|
||||
maxRetries, FallbackMaxRetries);
|
||||
configName, maxRetries, FallbackMaxRetries);
|
||||
maxRetries = FallbackMaxRetries;
|
||||
}
|
||||
|
||||
if (retryDelay <= TimeSpan.Zero)
|
||||
{
|
||||
_logger.LogWarning(
|
||||
"SmtpConfiguration.RetryDelay={ConfiguredRetryDelay} is non-positive; " +
|
||||
"{ConfigName}.RetryDelay={ConfiguredRetryDelay} is non-positive; " +
|
||||
"clamping to FallbackRetryDelay={FallbackRetryDelay} so the dispatcher does " +
|
||||
"not burn-loop on transient failures.",
|
||||
retryDelay, FallbackRetryDelay);
|
||||
configName, retryDelay, FallbackRetryDelay);
|
||||
retryDelay = FallbackRetryDelay;
|
||||
}
|
||||
|
||||
return (maxRetries, retryDelay);
|
||||
return new RetryPolicy(maxRetries, retryDelay);
|
||||
}
|
||||
|
||||
/// <summary>
|
||||
@@ -519,12 +573,17 @@ public class NotificationOutboxActor : ReceiveActor, IWithTimers
|
||||
private async Task DeliverOneAsync(
|
||||
Notification notification,
|
||||
DateTimeOffset now,
|
||||
int maxRetries,
|
||||
TimeSpan retryDelay,
|
||||
RetryPolicies policies,
|
||||
INotificationOutboxRepository outboxRepository,
|
||||
IReadOnlyDictionary<NotificationType, INotificationDeliveryAdapter> adapters,
|
||||
CancellationToken cancellationToken)
|
||||
{
|
||||
// Select the retry policy by delivery channel: SMS honors SmsConfiguration's own
|
||||
// MaxRetries/RetryDelay; every other type (Email today) uses the SMTP policy.
|
||||
var (maxRetries, retryDelay) = notification.Type == NotificationType.Sms
|
||||
? policies.SmsPolicy
|
||||
: policies.SmtpPolicy;
|
||||
|
||||
if (!adapters.TryGetValue(notification.Type, out var adapter))
|
||||
{
|
||||
// Missing-adapter park: from the dispatcher's perspective this is an
|
||||
|
||||
Reference in New Issue
Block a user