fix(server): log the full anyhow cause chain at mailer failure sites - #331
Merged
Conversation
`Display` on an anyhow::Error prints ONLY the outermost context, so
`error = %e` logs the wrapper and silently discards the actual cause.
Every mail path wraps its errors (`.context("parse To address")`,
`.context("SMTP send failed")`, …), so every mail failure has been
logged as its wrapper alone.
That is why the waitlist invite outage read as a broken transport for as
long as it did: the logs only ever said `parse To address`, never which
address or why. The cause — that a display name containing `@` is not a
valid RFC 5322 atom — was one context layer down and never printed.
Switches the 8 sites that log a `Mailer` error to `%format!("{e:#}")`,
whose alternate Display flattens the whole chain onto ONE line. Single
line matters: `?e` (Debug) also carries the chain but emits multi-line
output, which is hostile to the JSON log pipeline.
This is not a new idiom — `format!("{e:#}")` is already used for exactly
this in smtp_admin_routes.rs:351 and telemetry.rs:63. The SMTP test-send
site is included here because its code used `?e` while the comment
directly above it prescribed `{e:#}`; code and comment now agree.
Deliberately NOT a repo-wide sweep. There are 392 `error = %e` sites;
most log a thiserror enum whose Display already interpolates its source
(e.g. `WaitlistError::Database(#[from] sqlx::Error)` renders as
"database error: {0}"), so `{:#}` would add nothing. Only anyhow errors
lose information this way, and the Mailer trait is where that bit.
Adds a test pinning the difference: plain Display yields only
"send_waitlist_invite failed" while `{e:#}` carries every layer down to
the root cause, and stays single-line.
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.
Displayon ananyhow::Errorprints only the outermost context. Every mail path wraps its errors (.context("parse To address"),.context("SMTP send failed"), …), soerror = %ehas been logging the wrapper and silently discarding the actual cause.That is why the waitlist invite outage read as a broken transport for as long as it did. The logs only ever said:
The cause — that a display name containing
@is not a valid RFC 5322 atom — was one context layer down and never printed. The transport was fine the whole time.Change
The 8 sites that log a
Mailererror now use%format!("{e:#}"). The alternateDisplayform flattens the whole chain onto one line:Single-line matters.
?e(Debug) also carries the chain but emits multi-line output withCaused by:stanzas, which is hostile to the JSON log pipeline.waitlist_routes.rsauth_routes.rsmagic_link_routes.rssmtp_admin_routes.rsNot a new idiom.
format!("{e:#}")is already used for exactly this atsmtp_admin_routes.rs:351andtelemetry.rs:63. The SMTP test-send site is included because its code used?ewhile the comment directly above it prescribed{e:#}— code and comment now agree.Deliberately not a repo-wide sweep
There are 392
error = %esites. Most log athiserrorenum whoseDisplayalready interpolates its source — e.g.WaitlistError::Database(#[from] sqlx::Error)renders as"database error: {0}"— so{:#}would add nothing there. Onlyanyhowerrors lose information this way, and theMailertrait (anyhow::Result) is where it actually bit. Each of the 8 edits was verified by its surrounding context to be a mailer call site, not just by line number.Test
alternate_display_surfaces_the_cause_chain_that_plain_display_hidespins the difference directly: plainDisplayyields only"send_waitlist_invite failed"and demonstrably hides the root cause, while{e:#}carries every layer down to it — and stays single-line.cargo test -p starstats-server960 + 932, clippy-D warningsandcargo fmt --all --checkclean. No migration, no behaviour change beyond log content.