fix(alerts): send one e-mail per recipient so a bad address can't drop all
The alert e-mail addressed ALL recipients in a single To:. When the admin fallback list contains an undeliverable address (e.g. a seeded admin on a non-routable domain), a receiving provider like Gmail sees the mixed To: including a nonexistent recipient, scores the message as spam, and silently drops it — for the valid recipients too (not even the spam folder). A single-recipient test mail from the same relay arrived fine, which is what isolated it: the shared To: was the difference, not SMTP. Send one message per recipient instead (best-effort, per-address error logging). A bad address now only fails its own send, and no recipient's address leaks to the others. Diagnosis confirmed the worker sends over real SMTP (mail.default=smtp, host=mail.thinkidoo.com) — this was purely the recipient-list shape hurting deliverability. Tests updated: two configured recipients now queue two messages, each with a single To. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>feat/v1-foundation
parent
f9a08a98d2
commit
5024675c94
|
|
@ -27,14 +27,17 @@ class AlertNotifier
|
||||||
|
|
||||||
private function email(AlertIncident $incident, bool $resolved): void
|
private function email(AlertIncident $incident, bool $resolved): void
|
||||||
{
|
{
|
||||||
try {
|
// ONE message per recipient — NOT a single To: with everyone. A mixed To that contains an
|
||||||
$recipients = $this->recipients();
|
// undeliverable address (e.g. a seeded admin on a non-routable domain) reads as spam to
|
||||||
if ($recipients === []) {
|
// providers like Gmail and gets the WHOLE message silently dropped for the valid recipients
|
||||||
return;
|
// too. Per-recipient sends isolate that (and don't leak co-recipients' addresses). Each send
|
||||||
|
// is best-effort: a bad address is logged, never thrown, so it can't break the poll loop.
|
||||||
|
foreach ($this->recipients() as $recipient) {
|
||||||
|
try {
|
||||||
|
Mail::to($recipient)->queue(new AlertNotification($incident, $resolved));
|
||||||
|
} catch (Throwable $e) {
|
||||||
|
Log::warning('alert email failed for '.$recipient.': '.$e->getMessage());
|
||||||
}
|
}
|
||||||
Mail::to($recipients)->queue(new AlertNotification($incident, $resolved));
|
|
||||||
} catch (Throwable $e) {
|
|
||||||
Log::warning('alert email failed: '.$e->getMessage());
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
|
|
|
||||||
|
|
@ -0,0 +1,11 @@
|
||||||
|
<?php
|
||||||
|
|
||||||
|
use Illuminate\Support\Facades\Cache;
|
||||||
|
|
||||||
|
Cache::forget('wmc');
|
||||||
|
dispatch(function () {
|
||||||
|
Cache::put('wmc',
|
||||||
|
config('mail.default').' | host='.config('mail.mailers.smtp.host').' | transport='.config('mail.mailers.'.config('mail.default').'.transport', 'n/a'),
|
||||||
|
300);
|
||||||
|
});
|
||||||
|
echo "dispatched\n";
|
||||||
|
|
@ -26,14 +26,18 @@ class AlertNotifierTest extends TestCase
|
||||||
return AlertIncident::create(['alert_rule_id' => $rule->id, 'server_id' => $server->id, 'state' => 'firing', 'value' => 95, 'started_at' => now()]);
|
return AlertIncident::create(['alert_rule_id' => $rule->id, 'server_id' => $server->id, 'state' => 'firing', 'value' => 95, 'started_at' => now()]);
|
||||||
}
|
}
|
||||||
|
|
||||||
public function test_email_is_queued_to_the_configured_recipients(): void
|
public function test_email_is_queued_once_per_recipient_with_a_single_to(): void
|
||||||
{
|
{
|
||||||
Mail::fake();
|
Mail::fake();
|
||||||
Setting::put('alert_email_to', "ops@example.com\nsecond@example.com");
|
Setting::put('alert_email_to', "ops@example.com\nsecond@example.com");
|
||||||
|
|
||||||
app(AlertNotifier::class)->notify($this->incident(), resolved: false);
|
app(AlertNotifier::class)->notify($this->incident(), resolved: false);
|
||||||
|
|
||||||
Mail::assertQueued(AlertNotification::class, fn (AlertNotification $m) => $m->hasTo('ops@example.com'));
|
// One message PER recipient, each with only its own address — a shared To that included an
|
||||||
|
// undeliverable recipient would otherwise get the whole mail spam-dropped for everyone.
|
||||||
|
Mail::assertQueued(AlertNotification::class, 2);
|
||||||
|
Mail::assertQueued(AlertNotification::class, fn (AlertNotification $m) => $m->hasTo('ops@example.com') && ! $m->hasTo('second@example.com'));
|
||||||
|
Mail::assertQueued(AlertNotification::class, fn (AlertNotification $m) => $m->hasTo('second@example.com') && ! $m->hasTo('ops@example.com'));
|
||||||
}
|
}
|
||||||
|
|
||||||
public function test_falls_back_to_admin_emails_when_none_configured(): void
|
public function test_falls_back_to_admin_emails_when_none_configured(): void
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue