diff --git a/backend/src/mailer.ts b/backend/src/mailer.ts index bfa1237..ff1fe00 100755 --- a/backend/src/mailer.ts +++ b/backend/src/mailer.ts @@ -4,6 +4,16 @@ import nodemailer from 'nodemailer'; // Brevo — has to set host, port and SMTP_SECURE explicitly rather than // inheriting these, and getting that wrong fails at send time rather than at // boot. See #64 on validating this at startup instead. +// #260 put the first awaited send on a user-facing request path (the admin +// creating an upload link). nodemailer's defaults are two minutes to connect +// and ten minutes on the socket, which is fine for a fire-and-forget send but +// is not a bound anyone waiting on a response can live with: the link row and +// its token are already committed by the time sendMail is called, the token +// is shown exactly once, and a request that hangs long enough for the browser +// or reverse proxy to give up first loses it for good. Five seconds each is +// long enough for a reachable host and short enough that a dead one fails +// fast, leaving the admin with the "not emailed" warning and a link they can +// still copy, instead of a stuck spinner and a token nobody ever saw. const transporter = nodemailer.createTransport({ host: process.env.SMTP_HOST || 'smtp.gmail.com', port: parseInt(process.env.SMTP_PORT || '465', 10), @@ -11,7 +21,10 @@ const transporter = nodemailer.createTransport({ auth: { user: process.env.SMTP_USER, pass: process.env.SMTP_PASSWORD - } + }, + connectionTimeout: 5000, + greetingTimeout: 5000, + socketTimeout: 5000 }); interface ParsedAddress { @@ -105,11 +118,12 @@ export async function sendMail(to: string, subject: string, html: string): Promi // Guarded here rather than at the four call sites, so every sender is covered // by construction and a fifth added later cannot bypass it by forgetting. // - // Skipping rather than throwing, and returning as though it sent: three of - // the callers already swallow send failures into a log, so throwing would - // mostly be caught and logged anyway while risking a 500 on the signup path. - // The flow under test finishes, and the log says why no mail arrived — which - // is the part that was missing when QA was simply muted. + // Skipping rather than throwing, and reporting the skip through MailOutcome + // rather than pretending nothing happened: three of the callers already + // swallow send failures into a log, so throwing would mostly be caught and + // logged anyway while risking a 500 on the signup path. The flow under test + // finishes, and both the log and the returned outcome say why no mail + // arrived — which is the part that was missing when QA was simply muted. if (!isAllowedRecipient(to, process.env.MAIL_ALLOWLIST)) { console.warn(`[mail-blocked] ${to} is not on MAIL_ALLOWLIST — skipping "${subject}"`); return 'skipped-blocked';