Repository navigation
Darling: alert email goes through notification routes when the default recipient list is blank (#4751) - #4777
Merged
Merged
Conversation
…t recipient list is blank (#4751) A route that names email recipients as its only destination sent nothing when the SMTP settings had a host and a from address but no default recipients: the send core's gate and DarlingAlertSettings.SmtpEnabled both required the default list, so the branch that reads the route's recipients never ran. SmtpEnabled is now host + from. The gate asks for a default list OR an enabled route with recipients. A firing that resolves to no recipients at all (no covering route, blank default) is not attempted: the repeat budget is released, no cooldown is stamped, no failure is counted, and SendEmailAsync is not called (it throws on an empty list). The router needed no change: a blank default already resolves to a null destination. Such a firing stores an undelivered row, the same shape an alert no webhook route covers already stores. Words that said host + from + to are all required are corrected in the README, sample config, DarlingConfig and the delivery-channel doc comments, and the viewer's Validate message now says what the default list is for.
erikdarlingdata
marked this pull request as ready for review
September 29, 2026 09:37
This was referenced Sep 29, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #4751.
Why
In Darling, a notification route can name email recipients as its only destination. That route sent nothing when the SMTP settings had a server and a from address but a blank default recipient list. Two checks required the default list: the send core's "is SMTP configured" gate, and
DarlingAlertSettings.SmtpEnabled. So the branch that reads the route's recipients never ran. The route editor and the README say only the server, the from address and the credentials come from the main settings, and the viewer accepts such a route, so the behavior contradicted what the product tells the operator. Lite has no routes and is not affected.What changes
DarlingAlertSettings.SmtpEnabledis now server + from. The default recipient list is no longer part of it. The only readers of theIAlertSettings.SmtpEnabledmember outside tests are the send core's gate and the router's default arm (both covered below).NotificationRoute.HasEmailDestinationandNotificationRouter.AnyRouteConfiguresEmail(routes)are new, next to their webhook twins.EmailSendCore: the gate isSmtpEnabled+ server + from + (default recipients not blank OR an enabled route names recipients). The line after it that folds in the webhook state is unchanged.EmailSendCore: when the recipients resolved for a firing are blank (no covering route, blank default), the send core does not call the SMTP send, which throws on an empty list. It releases the repeat budget, logs at debug, and reports email asNotAttempted. It counts no failure, stamps no cooldown and returns no error, and it keeps the routing record. The existing try/catch is unchanged and now sits in theelsebranch.SendTestEmailAsyncis untouched.ResolveChannel's last arm already returns a null destination for a blank default. I added a comment at the call and pinned it with a test.AlertDelivery.FromFanoutrecords that asundelivered, the same shape an alert no webhook route covers already produces.EmailFanoutResult.AnyChannelConfiguredis true and both channel outcomes areNotAttempted. Lite cannot produce this shape: it has no routes, so its gate reduces to the old expression.AlertDelivery.FromFanoutremarks and the matching pin's doc comment (the pin's method is renamedUndelivered_IsReachedOnlyByAConfiguredChannelNothingConsulted), the doc comment on Lite'sUnreachablepredicate, "host + from + to are all set" inDarling/README.md,darling.sample.jsonandDarlingConfig.cs, the README's route paragraph, andDarlingAlertingTests, which now expectsSmtpEnabledonce server and from are set.Test plan
Darling.TestsandLite.Testsbuild with 0 warnings and 0 errors.EmailRouteRecipientsTests(8 tests, 0 skipped): a covering route sends one email whoseToheader is the route's recipients; an uncovered firing isNotAttemptedwith no message, no failure count and anundeliveredrow, and the next firing sends after either a covering route or a default list is added (so no cooldown was stamped); the same with Summary delivery and a last email 20 minutes ago against a 15 minute window isDeliveredand notFolded(so the budget was released); no recipients anywhere, or only a disabled route, is still not a configured channel;SmtpEnabledis true with server + from + blank default, andResolve(...).Email.Destinationis null for a blank or whitespace default on the real settings;AnyRouteConfiguresEmailcases.EmailSendCore.csandDarlingAlertSettings.csput back to their dev versions, the new tests fail: 6 failures (5 in the new class, 1 inDarlingAlertingTests). The tests that pin unchanged behavior (no recipients anywhere,AnyRouteConfiguresEmail) pass on both.EmailRouteRecipientsTests,NotificationRoutingTests,DarlingAlertingTests,AlertDeliveryChannelTestsinDarling.Tests: 77 total, 0 failed, 4 skipped (none in the new class).AlertDeliveryChannelTestsinLite.Tests: 14 passed.Darling.Tests, run once with no database configured: 17167 total, 0 failed, 1121 skipped, 1 not run.Lite.Tests, run once: 5597 total, 0 failed, 0 skipped.Worth a second look: a deployment whose email is set up only through routes will now record an
undeliveredrow for every firing no route covers, where the send core previously reported no channel as set up.CHANGELOG
SECTION: Fixed
ENTRY:
REF:
[An email-only notification route never sends when the default recipients are blank #4751]: An email-only notification route never sends when the default recipients are blank #4751