From 7837a07ba2bf7b6a6c9b77e7aff8eba5c47cacd4 Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Mon, 12 Oct 2026 14:37:58 +0200 Subject: [PATCH] fix(notifications): disable channels after repeated configuration failures Count invalid keys, tokens, and deleted webhooks in a new failure_count column on each notification settings table, like servers.unreachable_count. After 3 failures in a row the channel is disabled and the team is told by instance email and its other channels. Rate limits and server errors are retried and not counted; test notifications never count. --- app/Jobs/SendMessageToDiscordJob.php | 12 +- app/Jobs/SendMessageToPushoverJob.php | 13 +- app/Jobs/SendMessageToSlackJob.php | 29 +-- app/Jobs/SendMessageToTelegramJob.php | 13 +- app/Jobs/SendWebhookJob.php | 12 +- app/Models/DiscordNotificationSettings.php | 13 +- app/Models/EmailNotificationSettings.php | 14 ++ app/Models/PushoverNotificationSettings.php | 13 +- app/Models/SlackNotificationSettings.php | 13 +- app/Models/TelegramNotificationSettings.php | 13 +- app/Models/WebhookNotificationSettings.php | 13 +- app/Notifications/Channels/DiscordChannel.php | 2 +- app/Notifications/Channels/EmailChannel.php | 40 ++++- .../Channels/PushoverChannel.php | 2 +- app/Notifications/Channels/SlackChannel.php | 2 +- .../Channels/TelegramChannel.php | 2 +- app/Notifications/Channels/WebhookChannel.php | 2 +- app/Traits/HandlesNotificationResponse.php | 45 +++++ app/Traits/TracksDeliveryFailures.php | 136 ++++++++++++++ ..._count_to_notification_settings_tables.php | 45 +++++ .../notification-channel-disabled.blade.php | 8 + .../NotificationDeliveryFailureTest.php | 166 ++++++++++++++++++ .../Channels/EmailChannelTest.php | 62 +++++++ 23 files changed, 631 insertions(+), 39 deletions(-) create mode 100644 app/Traits/HandlesNotificationResponse.php create mode 100644 app/Traits/TracksDeliveryFailures.php create mode 100644 database/migrations/2026_10_12_121520_add_failure_count_to_notification_settings_tables.php create mode 100644 resources/views/emails/notification-channel-disabled.blade.php create mode 100644 tests/Feature/NotificationDeliveryFailureTest.php diff --git a/app/Jobs/SendMessageToDiscordJob.php b/app/Jobs/SendMessageToDiscordJob.php index d5c29efb0f..226e9f4cd7 100644 --- a/app/Jobs/SendMessageToDiscordJob.php +++ b/app/Jobs/SendMessageToDiscordJob.php @@ -2,8 +2,10 @@ namespace App\Jobs; +use App\Models\DiscordNotificationSettings; use App\Notifications\Dto\DiscordMessage; use App\Rules\SafeWebhookUrl; +use App\Traits\HandlesNotificationResponse; use Illuminate\Bus\Queueable; use Illuminate\Contracts\Queue\ShouldBeEncrypted; use Illuminate\Contracts\Queue\ShouldQueue; @@ -16,7 +18,7 @@ use Illuminate\Support\Facades\Validator; class SendMessageToDiscordJob implements ShouldBeEncrypted, ShouldQueue { - use Dispatchable, InteractsWithQueue, Queueable, SerializesModels; + use Dispatchable, HandlesNotificationResponse, InteractsWithQueue, Queueable, SerializesModels; /** * The number of times the job may be attempted. @@ -34,7 +36,8 @@ class SendMessageToDiscordJob implements ShouldBeEncrypted, ShouldQueue public function __construct( public DiscordMessage $message, - public string $webhookUrl + public string $webhookUrl, + public ?DiscordNotificationSettings $settings = null, ) { $this->onQueue('high'); } @@ -69,6 +72,9 @@ class SendMessageToDiscordJob implements ShouldBeEncrypted, ShouldQueue return; } - Http::withOptions($httpOptions)->post($this->webhookUrl, $this->message->toPayload()); + $response = Http::withOptions($httpOptions)->post($this->webhookUrl, $this->message->toPayload()); + + // 401: invalid webhook token, 403: missing access, 404: unknown (deleted) webhook. + $this->handleNotificationResponse($response, 'Discord', in_array($response->status(), [401, 403, 404], true)); } } diff --git a/app/Jobs/SendMessageToPushoverJob.php b/app/Jobs/SendMessageToPushoverJob.php index e2a94cdaa0..406a7e5ea4 100644 --- a/app/Jobs/SendMessageToPushoverJob.php +++ b/app/Jobs/SendMessageToPushoverJob.php @@ -2,7 +2,9 @@ namespace App\Jobs; +use App\Models\PushoverNotificationSettings; use App\Notifications\Dto\PushoverMessage; +use App\Traits\HandlesNotificationResponse; use Illuminate\Bus\Queueable; use Illuminate\Contracts\Queue\ShouldBeEncrypted; use Illuminate\Contracts\Queue\ShouldQueue; @@ -13,7 +15,7 @@ use Illuminate\Support\Facades\Http; class SendMessageToPushoverJob implements ShouldBeEncrypted, ShouldQueue { - use Dispatchable, InteractsWithQueue, Queueable, SerializesModels; + use Dispatchable, HandlesNotificationResponse, InteractsWithQueue, Queueable, SerializesModels; /** * The number of times the job may be attempted. @@ -33,6 +35,7 @@ class SendMessageToPushoverJob implements ShouldBeEncrypted, ShouldQueue public PushoverMessage $message, public string $token, public string $user, + public ?PushoverNotificationSettings $settings = null, ) { $this->onQueue('high'); } @@ -43,8 +46,10 @@ class SendMessageToPushoverJob implements ShouldBeEncrypted, ShouldQueue public function handle(): void { $response = Http::post('https://api.pushover.net/1/messages.json', $this->message->toPayload($this->token, $this->user)); - if ($response->failed()) { - throw new \RuntimeException('Pushover notification failed with '.$response->status().' status code.'.$response->body()); - } + + // Pushover marks the invalid field in the error body, for example {"token": "invalid"}. + $isConfigurationError = $response->clientError() + && ($response->json('token') === 'invalid' || $response->json('user') === 'invalid'); + $this->handleNotificationResponse($response, 'Pushover', $isConfigurationError); } } diff --git a/app/Jobs/SendMessageToSlackJob.php b/app/Jobs/SendMessageToSlackJob.php index 3a306c23da..0b01f7f658 100644 --- a/app/Jobs/SendMessageToSlackJob.php +++ b/app/Jobs/SendMessageToSlackJob.php @@ -2,12 +2,15 @@ namespace App\Jobs; +use App\Models\SlackNotificationSettings; use App\Notifications\Dto\SlackMessage; use App\Rules\SafeWebhookUrl; +use App\Traits\HandlesNotificationResponse; use Illuminate\Bus\Queueable; use Illuminate\Contracts\Queue\ShouldBeEncrypted; use Illuminate\Contracts\Queue\ShouldQueue; use Illuminate\Foundation\Bus\Dispatchable; +use Illuminate\Http\Client\Response; use Illuminate\Queue\InteractsWithQueue; use Illuminate\Queue\SerializesModels; use Illuminate\Support\Facades\Http; @@ -16,7 +19,7 @@ use Illuminate\Support\Facades\Validator; class SendMessageToSlackJob implements ShouldBeEncrypted, ShouldQueue { - use Dispatchable, InteractsWithQueue, Queueable, SerializesModels; + use Dispatchable, HandlesNotificationResponse, InteractsWithQueue, Queueable, SerializesModels; /** * The number of times the job may be attempted. @@ -30,7 +33,8 @@ class SendMessageToSlackJob implements ShouldBeEncrypted, ShouldQueue public function __construct( private SlackMessage $message, - private string $webhookUrl + private string $webhookUrl, + public ?SlackNotificationSettings $settings = null, ) { $this->onQueue('high'); } @@ -62,18 +66,17 @@ class SendMessageToSlackJob implements ShouldBeEncrypted, ShouldQueue return; } - if ($this->isSlackWebhook()) { - $this->sendToSlack($httpOptions); - - return; - } - /** * This works with Mattermost and as a fallback also with Slack, the notifications just look slightly different and advanced formatting for slack is not supported with Mattermost. * * @see https://github.com/coollabsio/coolify/pull/6139#issuecomment-3756777708 */ - $this->sendToMattermost($httpOptions); + $response = $this->isSlackWebhook() + ? $this->sendToSlack($httpOptions) + : $this->sendToMattermost($httpOptions); + + // 403: posting is not allowed, 404: the webhook or channel was removed, 410: the channel is archived. + $this->handleNotificationResponse($response, 'Slack', in_array($response->status(), [403, 404, 410], true)); } private function isSlackWebhook(): bool @@ -93,9 +96,9 @@ class SendMessageToSlackJob implements ShouldBeEncrypted, ShouldQueue /** * @param array $httpOptions */ - private function sendToSlack(array $httpOptions): void + private function sendToSlack(array $httpOptions): Response { - Http::withOptions($httpOptions)->post($this->webhookUrl, [ + return Http::withOptions($httpOptions)->post($this->webhookUrl, [ 'text' => $this->message->title, 'blocks' => [ [ @@ -136,11 +139,11 @@ class SendMessageToSlackJob implements ShouldBeEncrypted, ShouldQueue /** * @param array $httpOptions */ - private function sendToMattermost(array $httpOptions): void + private function sendToMattermost(array $httpOptions): Response { $username = config('app.name'); - Http::withOptions($httpOptions)->post($this->webhookUrl, [ + return Http::withOptions($httpOptions)->post($this->webhookUrl, [ 'username' => $username, 'attachments' => [ [ diff --git a/app/Jobs/SendMessageToTelegramJob.php b/app/Jobs/SendMessageToTelegramJob.php index 6b04d21915..0cb1a7124d 100644 --- a/app/Jobs/SendMessageToTelegramJob.php +++ b/app/Jobs/SendMessageToTelegramJob.php @@ -2,6 +2,8 @@ namespace App\Jobs; +use App\Models\TelegramNotificationSettings; +use App\Traits\HandlesNotificationResponse; use Illuminate\Bus\Queueable; use Illuminate\Contracts\Queue\ShouldBeEncrypted; use Illuminate\Contracts\Queue\ShouldQueue; @@ -13,7 +15,7 @@ use Illuminate\Support\Str; class SendMessageToTelegramJob implements ShouldBeEncrypted, ShouldQueue { - use Dispatchable, InteractsWithQueue, Queueable, SerializesModels; + use Dispatchable, HandlesNotificationResponse, InteractsWithQueue, Queueable, SerializesModels; /** * The number of times the job may be attempted. @@ -38,6 +40,7 @@ class SendMessageToTelegramJob implements ShouldBeEncrypted, ShouldQueue public string $token, public string $chatId, public ?string $threadId = null, + public ?TelegramNotificationSettings $settings = null, ) { $this->onQueue('high'); } @@ -76,8 +79,10 @@ class SendMessageToTelegramJob implements ShouldBeEncrypted, ShouldQueue $payload['message_thread_id'] = $this->threadId; } $response = Http::post($url, $payload); - if ($response->failed()) { - throw new \RuntimeException('Telegram notification failed with '.$response->status().' status code.'.$response->body()); - } + + // 401: invalid bot token, 403: the bot was blocked or removed from the chat, 400 "chat not found": wrong chat ID. + $isConfigurationError = in_array($response->status(), [401, 403], true) + || ($response->status() === 400 && Str::contains((string) $response->json('description'), 'chat not found', true)); + $this->handleNotificationResponse($response, 'Telegram', $isConfigurationError); } } diff --git a/app/Jobs/SendWebhookJob.php b/app/Jobs/SendWebhookJob.php index c14d70a773..29ee58fb2f 100644 --- a/app/Jobs/SendWebhookJob.php +++ b/app/Jobs/SendWebhookJob.php @@ -2,7 +2,9 @@ namespace App\Jobs; +use App\Models\WebhookNotificationSettings; use App\Rules\SafeWebhookUrl; +use App\Traits\HandlesNotificationResponse; use Illuminate\Bus\Queueable; use Illuminate\Contracts\Queue\ShouldBeEncrypted; use Illuminate\Contracts\Queue\ShouldQueue; @@ -15,7 +17,7 @@ use Illuminate\Support\Facades\Validator; class SendWebhookJob implements ShouldBeEncrypted, ShouldQueue { - use Dispatchable, InteractsWithQueue, Queueable, SerializesModels; + use Dispatchable, HandlesNotificationResponse, InteractsWithQueue, Queueable, SerializesModels; /** * The number of times the job may be attempted. @@ -33,7 +35,8 @@ class SendWebhookJob implements ShouldBeEncrypted, ShouldQueue public function __construct( public array $payload, - public string $webhookUrl + public string $webhookUrl, + public ?WebhookNotificationSettings $settings = null, ) { $this->onQueue('high'); } @@ -68,6 +71,9 @@ class SendWebhookJob implements ShouldBeEncrypted, ShouldQueue return; } - Http::withOptions($httpOptions)->post($this->webhookUrl, $this->payload); + $response = Http::withOptions($httpOptions)->post($this->webhookUrl, $this->payload); + + // 401/403: the endpoint rejects us, 404/410: the endpoint does not exist anymore. + $this->handleNotificationResponse($response, 'Webhook', in_array($response->status(), [401, 403, 404, 410], true)); } } diff --git a/app/Models/DiscordNotificationSettings.php b/app/Models/DiscordNotificationSettings.php index 48d5b5d293..4bf3343527 100644 --- a/app/Models/DiscordNotificationSettings.php +++ b/app/Models/DiscordNotificationSettings.php @@ -2,12 +2,13 @@ namespace App\Models; +use App\Traits\TracksDeliveryFailures; use Illuminate\Database\Eloquent\Model; use Illuminate\Notifications\Notifiable; class DiscordNotificationSettings extends Model { - use Notifiable; + use Notifiable, TracksDeliveryFailures; public $timestamps = false; @@ -74,4 +75,14 @@ class DiscordNotificationSettings extends Model { return $this->discord_ping_enabled; } + + public function notificationChannelName(): string + { + return 'Discord'; + } + + protected function disableNotificationChannel(): void + { + $this->discord_enabled = false; + } } diff --git a/app/Models/EmailNotificationSettings.php b/app/Models/EmailNotificationSettings.php index 3b04b482af..94d0802d96 100644 --- a/app/Models/EmailNotificationSettings.php +++ b/app/Models/EmailNotificationSettings.php @@ -2,10 +2,13 @@ namespace App\Models; +use App\Traits\TracksDeliveryFailures; use Illuminate\Database\Eloquent\Model; class EmailNotificationSettings extends Model { + use TracksDeliveryFailures; + public $timestamps = false; protected $fillable = [ @@ -93,4 +96,15 @@ class EmailNotificationSettings extends Model { return $this->smtp_enabled || $this->resend_enabled || $this->use_instance_email_settings; } + + public function notificationChannelName(): string + { + return 'Email'; + } + + protected function disableNotificationChannel(): void + { + $this->smtp_enabled = false; + $this->resend_enabled = false; + } } diff --git a/app/Models/PushoverNotificationSettings.php b/app/Models/PushoverNotificationSettings.php index 2ab6693142..9a8be46626 100644 --- a/app/Models/PushoverNotificationSettings.php +++ b/app/Models/PushoverNotificationSettings.php @@ -2,12 +2,13 @@ namespace App\Models; +use App\Traits\TracksDeliveryFailures; use Illuminate\Database\Eloquent\Model; use Illuminate\Notifications\Notifiable; class PushoverNotificationSettings extends Model { - use Notifiable; + use Notifiable, TracksDeliveryFailures; public $timestamps = false; @@ -70,4 +71,14 @@ class PushoverNotificationSettings extends Model { return $this->pushover_enabled; } + + public function notificationChannelName(): string + { + return 'Pushover'; + } + + protected function disableNotificationChannel(): void + { + $this->pushover_enabled = false; + } } diff --git a/app/Models/SlackNotificationSettings.php b/app/Models/SlackNotificationSettings.php index 648869bafe..53125e1e45 100644 --- a/app/Models/SlackNotificationSettings.php +++ b/app/Models/SlackNotificationSettings.php @@ -2,12 +2,13 @@ namespace App\Models; +use App\Traits\TracksDeliveryFailures; use Illuminate\Database\Eloquent\Model; use Illuminate\Notifications\Notifiable; class SlackNotificationSettings extends Model { - use Notifiable; + use Notifiable, TracksDeliveryFailures; public $timestamps = false; @@ -67,4 +68,14 @@ class SlackNotificationSettings extends Model { return $this->slack_enabled; } + + public function notificationChannelName(): string + { + return 'Slack'; + } + + protected function disableNotificationChannel(): void + { + $this->slack_enabled = false; + } } diff --git a/app/Models/TelegramNotificationSettings.php b/app/Models/TelegramNotificationSettings.php index 3376e239d6..6ab6c7a9e9 100644 --- a/app/Models/TelegramNotificationSettings.php +++ b/app/Models/TelegramNotificationSettings.php @@ -2,12 +2,13 @@ namespace App\Models; +use App\Traits\TracksDeliveryFailures; use Illuminate\Database\Eloquent\Model; use Illuminate\Notifications\Notifiable; class TelegramNotificationSettings extends Model { - use Notifiable; + use Notifiable, TracksDeliveryFailures; public $timestamps = false; @@ -117,4 +118,14 @@ class TelegramNotificationSettings extends Model { return $this->telegram_enabled; } + + public function notificationChannelName(): string + { + return 'Telegram'; + } + + protected function disableNotificationChannel(): void + { + $this->telegram_enabled = false; + } } diff --git a/app/Models/WebhookNotificationSettings.php b/app/Models/WebhookNotificationSettings.php index 7ffd20a8c3..9f2f1244cd 100644 --- a/app/Models/WebhookNotificationSettings.php +++ b/app/Models/WebhookNotificationSettings.php @@ -2,12 +2,13 @@ namespace App\Models; +use App\Traits\TracksDeliveryFailures; use Illuminate\Database\Eloquent\Model; use Illuminate\Notifications\Notifiable; class WebhookNotificationSettings extends Model { - use Notifiable; + use Notifiable, TracksDeliveryFailures; public $timestamps = false; @@ -71,4 +72,14 @@ class WebhookNotificationSettings extends Model { return $this->webhook_enabled; } + + public function notificationChannelName(): string + { + return 'Webhook'; + } + + protected function disableNotificationChannel(): void + { + $this->webhook_enabled = false; + } } diff --git a/app/Notifications/Channels/DiscordChannel.php b/app/Notifications/Channels/DiscordChannel.php index b4ba9bf8cc..4bd16a2d5b 100644 --- a/app/Notifications/Channels/DiscordChannel.php +++ b/app/Notifications/Channels/DiscordChannel.php @@ -24,6 +24,6 @@ class DiscordChannel $message->isCritical = false; } - SendMessageToDiscordJob::dispatch($message, $discordSettings->discord_webhook_url); + SendMessageToDiscordJob::dispatch($message, $discordSettings->discord_webhook_url, data_get($notification, 'isTestNotification', false) ? null : $discordSettings); } } diff --git a/app/Notifications/Channels/EmailChannel.php b/app/Notifications/Channels/EmailChannel.php index bcf976f815..9a14da58a1 100644 --- a/app/Notifications/Channels/EmailChannel.php +++ b/app/Notifications/Channels/EmailChannel.php @@ -3,13 +3,16 @@ namespace App\Notifications\Channels; use App\Exceptions\NonReportableException; +use App\Models\EmailNotificationSettings; use App\Models\Team; use App\Support\SmtpTransportFactory; use Exception; use Illuminate\Notifications\Notification; +use Illuminate\Support\Facades\Log; use Resend; use Resend\Exceptions\ErrorException; use Resend\Exceptions\TransporterException; +use Symfony\Component\Mailer\Exception\TransportExceptionInterface; use Symfony\Component\Mailer\Mailer; use Symfony\Component\Mime\Email; @@ -19,6 +22,10 @@ class EmailChannel public function send(SendsEmail $notifiable, Notification $notification): void { + $settings = null; + // Test notifications always show their error and never count as delivery failures. + $isTestNotification = data_get($notification, 'isTestNotification', false); + try { // Get team and validate membership before proceeding $team = data_get($notifiable, 'id'); @@ -49,8 +56,6 @@ class EmailChannel } // Skip team membership validation for test notifications - $isTestNotification = data_get($notification, 'isTestNotification', false); - if (! $isTestNotification) { foreach ($recipients as $recipient) { // Check if the recipient is part of the team @@ -103,6 +108,10 @@ class EmailChannel $mailer->send($email); } + + if (! $isTestNotification && $settings instanceof EmailNotificationSettings) { + $settings->recordDeliverySuccess(); + } } catch (ErrorException $e) { ['status' => $statusCode, 'name' => $errorName] = self::resendErrorDetails($e); @@ -116,6 +125,24 @@ class EmailChannel default => 'Failed to send email via Resend: '.$e->getErrorMessage(), }; + if (! $isTestNotification && $settings instanceof EmailNotificationSettings) { + // The team's own settings: an invalid key or domain counts as a delivery failure, and + // other client errors are only logged. Neither is retried, because a retry fails again. + if (in_array($statusCode, [401, 403], true)) { + $settings->recordDeliveryFailure($userMessage); + + return; + } + + if (in_array($statusCode, [400, 422], true)) { + Log::warning('Resend rejected the email', ['team_id' => $settings->team_id, 'reason' => $userMessage]); + + return; + } + + throw new Exception($userMessage, $e->getCode(), $e); + } + // Log detailed error for admin debugging (redact sensitive data) $emailSettings = $notifiable->emailNotificationSettings ?? instanceSettings(); data_set($emailSettings, 'smtp_password', '********'); @@ -135,6 +162,15 @@ class EmailChannel } throw new Exception($userMessage, $e->getCode(), $e); + } catch (TransportExceptionInterface $e) { + // SMTP 530/535: the SMTP server does not accept the username or password. + if (! $isTestNotification && $settings instanceof EmailNotificationSettings && in_array($e->getCode(), [530, 535], true)) { + $settings->recordDeliveryFailure('SMTP authentication failed. Please verify the SMTP username and password.'); + + return; + } + + throw $e; } catch (TransporterException $e) { send_internal_notification("Resend Transport Error: {$e->getMessage()}"); throw new Exception('Unable to connect to Resend API. Please check your internet connection and try again.'); diff --git a/app/Notifications/Channels/PushoverChannel.php b/app/Notifications/Channels/PushoverChannel.php index 3d3728d01f..1323d874e3 100644 --- a/app/Notifications/Channels/PushoverChannel.php +++ b/app/Notifications/Channels/PushoverChannel.php @@ -16,6 +16,6 @@ class PushoverChannel return; } - SendMessageToPushoverJob::dispatch($message, $pushoverSettings->pushover_api_token, $pushoverSettings->pushover_user_key); + SendMessageToPushoverJob::dispatch($message, $pushoverSettings->pushover_api_token, $pushoverSettings->pushover_user_key, data_get($notification, 'isTestNotification', false) ? null : $pushoverSettings); } } diff --git a/app/Notifications/Channels/SlackChannel.php b/app/Notifications/Channels/SlackChannel.php index cddb7a5611..82d235a269 100644 --- a/app/Notifications/Channels/SlackChannel.php +++ b/app/Notifications/Channels/SlackChannel.php @@ -19,6 +19,6 @@ class SlackChannel return; } - SendMessageToSlackJob::dispatch($message, $slackSettings->slack_webhook_url); + SendMessageToSlackJob::dispatch($message, $slackSettings->slack_webhook_url, data_get($notification, 'isTestNotification', false) ? null : $slackSettings); } } diff --git a/app/Notifications/Channels/TelegramChannel.php b/app/Notifications/Channels/TelegramChannel.php index d0e1c95ed9..5b23f54413 100644 --- a/app/Notifications/Channels/TelegramChannel.php +++ b/app/Notifications/Channels/TelegramChannel.php @@ -73,6 +73,6 @@ class TelegramChannel return; } - SendMessageToTelegramJob::dispatch($message, $buttons, $telegramToken, $chatId, $threadId); + SendMessageToTelegramJob::dispatch($message, $buttons, $telegramToken, $chatId, $threadId, data_get($notification, 'isTestNotification', false) ? null : $settings); } } diff --git a/app/Notifications/Channels/WebhookChannel.php b/app/Notifications/Channels/WebhookChannel.php index e735b88bb8..12b59dd600 100644 --- a/app/Notifications/Channels/WebhookChannel.php +++ b/app/Notifications/Channels/WebhookChannel.php @@ -20,6 +20,6 @@ class WebhookChannel $payload = $notification->toWebhook(); - SendWebhookJob::dispatch($payload, $webhookSettings->webhook_url); + SendWebhookJob::dispatch($payload, $webhookSettings->webhook_url, data_get($notification, 'isTestNotification', false) ? null : $webhookSettings); } } diff --git a/app/Traits/HandlesNotificationResponse.php b/app/Traits/HandlesNotificationResponse.php new file mode 100644 index 0000000000..77d8b8477d --- /dev/null +++ b/app/Traits/HandlesNotificationResponse.php @@ -0,0 +1,45 @@ +successful()) { + $this->settings?->recordDeliverySuccess(); + + return; + } + + if ($response->status() === 429 || $response->serverError()) { + throw new RuntimeException("{$channel} notification failed with {$response->status()} status code."); + } + + $reason = "{$channel} returned HTTP {$response->status()}: ".Str::limit($response->body(), 200); + + if ($isConfigurationError) { + $this->settings?->recordDeliveryFailure($reason); + + return; + } + + Log::warning("{$channel} rejected the notification", ['reason' => $reason]); + } +} diff --git a/app/Traits/TracksDeliveryFailures.php b/app/Traits/TracksDeliveryFailures.php new file mode 100644 index 0000000000..5b8c785437 --- /dev/null +++ b/app/Traits/TracksDeliveryFailures.php @@ -0,0 +1,136 @@ +exists && array_diff(array_keys($settings->getDirty()), ['failure_count']) !== []) { + $settings->failure_count = 0; + } + }); + } + + public function recordDeliveryFailure(string $reason): void + { + $this->increment('failure_count'); + + Log::warning('Notification delivery failed', [ + 'channel' => $this->notificationChannelName(), + 'team_id' => $this->team_id, + 'failure_count' => $this->failure_count, + 'reason' => $reason, + ]); + + if ($this->failure_count < self::DELIVERY_FAILURE_THRESHOLD || ! $this->isEnabled()) { + return; + } + + $this->disableNotificationChannel(); + $this->save(); + + $channel = $this->notificationChannelName(); + + auditLog('notifications.'.Str::lower($channel).'.auto_disabled', [ + 'team_id' => $this->team_id, + 'reason' => $reason, + ], 'warning'); + + send_internal_notification(sprintf( + "%s notifications disabled for team %s after %d failed deliveries.\nReason: %s", + $channel, + $this->team_id, + self::DELIVERY_FAILURE_THRESHOLD, + $reason + )); + + $this->sendChannelDisabledEmail($reason); + $this->notifyOtherChannels($reason); + } + + public function recordDeliverySuccess(): void + { + if ($this->failure_count > 0) { + $this->failure_count = 0; + $this->save(); + } + } + + /** + * Tell the team through its other enabled channels. The disabled channel is + * not in the list anymore, and the "general" event never uses email. The + * root team already gets the internal notification on the same channels. + */ + private function notifyOtherChannels(string $reason): void + { + if ($this->team_id === 0 || ! $this->team) { + return; + } + + $channel = $this->notificationChannelName(); + + $this->team->notify(new GeneralNotification( + "{$channel} notifications were disabled after ".self::DELIVERY_FAILURE_THRESHOLD." failed deliveries in a row.\nReason: {$reason}\nPlease fix the settings and enable {$channel} notifications again: ".base_url().'/notifications/'.Str::lower($channel), + success: false, + )); + } + + /** + * Tell the team admins through the instance email settings, because the + * team's own channel does not work. + */ + private function sendChannelDisabledEmail(string $reason): void + { + if (! is_transactional_emails_enabled() || ! $this->team) { + return; + } + + try { + $channel = $this->notificationChannelName(); + $mail = new MailMessage; + $mail->subject("Coolify: {$channel} notifications were disabled"); + $mail->view('emails.notification-channel-disabled', [ + 'channel' => $channel, + 'failures' => self::DELIVERY_FAILURE_THRESHOLD, + 'reason' => $reason, + 'url' => base_url().'/notifications/'.Str::lower($channel), + ]); + + $admins = $this->team->members()->wherePivotIn('role', ['admin', 'owner'])->get(['users.id', 'users.email']); + foreach ($admins as $admin) { + send_user_an_email($mail, $admin->email); + } + } catch (Throwable $e) { + Log::warning('Failed to send notification channel disabled email: '.$e->getMessage()); + } + } +} diff --git a/database/migrations/2026_10_12_121520_add_failure_count_to_notification_settings_tables.php b/database/migrations/2026_10_12_121520_add_failure_count_to_notification_settings_tables.php new file mode 100644 index 0000000000..4e7e1c54f9 --- /dev/null +++ b/database/migrations/2026_10_12_121520_add_failure_count_to_notification_settings_tables.php @@ -0,0 +1,45 @@ +tables as $table) { + if (! Schema::hasColumn($table, 'failure_count')) { + Schema::table($table, function (Blueprint $table) { + $table->integer('failure_count')->default(0); + }); + } + } + } + + /** + * Reverse the migrations. + */ + public function down(): void + { + foreach ($this->tables as $table) { + if (Schema::hasColumn($table, 'failure_count')) { + Schema::table($table, function (Blueprint $table) { + $table->dropColumn('failure_count'); + }); + } + } + } +}; diff --git a/resources/views/emails/notification-channel-disabled.blade.php b/resources/views/emails/notification-channel-disabled.blade.php new file mode 100644 index 0000000000..8faf5121c4 --- /dev/null +++ b/resources/views/emails/notification-channel-disabled.blade.php @@ -0,0 +1,8 @@ + + +Coolify could not send {{ $channel }} notifications for your team {{ $failures }} times in a row, so we disabled {{ $channel }} notifications. + +Reason: {{ $reason }} + +Please fix the settings and enable {{ $channel }} notifications again [here]({{ $url }}). + diff --git a/tests/Feature/NotificationDeliveryFailureTest.php b/tests/Feature/NotificationDeliveryFailureTest.php new file mode 100644 index 0000000000..64e31385c9 --- /dev/null +++ b/tests/Feature/NotificationDeliveryFailureTest.php @@ -0,0 +1,166 @@ + InstanceSettings::query()->firstOrCreate(['id' => 0])); + $this->team = Team::factory()->create(); +}); + +/** + * @return array{0: Model, 1: Closure(Model|null): object, 2: string} + */ +function deliveryFailureChannel(Team $team, string $channel): array +{ + return match ($channel) { + 'discord' => [ + tap($team->discordNotificationSettings)->update(['discord_enabled' => true, 'discord_webhook_url' => 'https://example.com/discord']), + fn ($settings) => new SendMessageToDiscordJob(new DiscordMessage('Title', 'Description', DiscordMessage::infoColor()), 'https://example.com/discord', $settings), + 'discord_enabled', + ], + 'slack' => [ + tap($team->slackNotificationSettings)->update(['slack_enabled' => true, 'slack_webhook_url' => 'https://example.com/slack']), + fn ($settings) => new SendMessageToSlackJob(new SlackMessage('Title', 'Description'), 'https://example.com/slack', $settings), + 'slack_enabled', + ], + 'telegram' => [ + tap($team->telegramNotificationSettings)->update(['telegram_enabled' => true, 'telegram_token' => 'token', 'telegram_chat_id' => '123']), + fn ($settings) => new SendMessageToTelegramJob('Message', [], 'token', '123', null, $settings), + 'telegram_enabled', + ], + 'pushover' => [ + tap($team->pushoverNotificationSettings)->update(['pushover_enabled' => true, 'pushover_api_token' => 'token', 'pushover_user_key' => 'user']), + fn ($settings) => new SendMessageToPushoverJob(new PushoverMessage('Title', 'Message'), 'token', 'user', $settings), + 'pushover_enabled', + ], + 'webhook' => [ + tap($team->webhookNotificationSettings)->update(['webhook_enabled' => true, 'webhook_url' => 'https://example.com/webhook']), + fn ($settings) => new SendWebhookJob(['event' => 'test'], 'https://example.com/webhook', $settings), + 'webhook_enabled', + ], + }; +} + +dataset('configuration errors', [ + 'discord deleted webhook' => ['discord', 404, ['message' => 'Unknown Webhook', 'code' => 10015]], + 'slack archived channel' => ['slack', 410, 'channel_is_archived'], + 'telegram invalid token' => ['telegram', 401, ['ok' => false, 'description' => 'Unauthorized']], + 'telegram wrong chat' => ['telegram', 400, ['ok' => false, 'description' => 'Bad Request: chat not found']], + 'pushover invalid token' => ['pushover', 400, ['token' => 'invalid', 'errors' => ['application token is invalid'], 'status' => 0]], + 'webhook not found' => ['webhook', 404, 'Not Found'], +]); + +it('disables the channel after three configuration errors in a row', function (string $channel, int $status, array|string $body) { + [$settings, $makeJob, $enabledColumn] = deliveryFailureChannel($this->team, $channel); + Http::fake(['*' => Http::response($body, $status)]); + + $makeJob($settings->fresh())->handle(); + $makeJob($settings->fresh())->handle(); + expect($settings->fresh()->failure_count)->toBe(2) + ->and($settings->fresh()->{$enabledColumn})->toBeTrue(); + + $makeJob($settings->fresh())->handle(); + expect($settings->fresh()->{$enabledColumn})->toBeFalse(); +})->with('configuration errors'); + +it('retries rate limits and server errors without counting them', function (string $channel, int $status) { + [$settings, $makeJob] = deliveryFailureChannel($this->team, $channel); + Http::fake(['*' => Http::response('error', $status)]); + + expect(fn () => $makeJob($settings->fresh())->handle())->toThrow(RuntimeException::class); + expect($settings->fresh()->failure_count)->toBe(0); +})->with(['discord', 'slack', 'telegram', 'pushover', 'webhook'])->with([429, 500]); + +it('does not count errors caused by the message payload', function (string $channel, array|string $body) { + [$settings, $makeJob] = deliveryFailureChannel($this->team, $channel); + Http::fake(['*' => Http::response($body, 400)]); + + $makeJob($settings->fresh())->handle(); + + expect($settings->fresh()->failure_count)->toBe(0); +})->with([ + 'discord' => ['discord', ['message' => 'Invalid Form Body', 'code' => 50035]], + 'slack' => ['slack', 'invalid_payload'], + 'telegram' => ['telegram', ['ok' => false, 'description' => "Bad Request: can't parse entities"]], + 'pushover' => ['pushover', ['message' => 'cannot be longer than 1024 characters', 'status' => 0]], + 'webhook' => ['webhook', 'Bad Request'], +]); + +it('resets the failure count after a successful delivery', function () { + [$settings, $makeJob] = deliveryFailureChannel($this->team, 'discord'); + $settings->increment('failure_count', 2); + Http::fake(['*' => Http::response('', 204)]); + + $makeJob($settings->fresh())->handle(); + + expect($settings->fresh()->failure_count)->toBe(0); +}); + +it('resets the failure count when the user changes the settings', function () { + [$settings] = deliveryFailureChannel($this->team, 'discord'); + $settings->increment('failure_count', 2); + + $settings->fresh()->update(['discord_webhook_url' => 'https://example.com/new-discord']); + + expect($settings->fresh()->failure_count)->toBe(0); +}); + +it('does not track test notifications', function () { + deliveryFailureChannel($this->team, 'discord'); + Queue::fake(); + + (new DiscordChannel)->send($this->team, new Test(channel: 'discord')); + + Queue::assertPushed(SendMessageToDiscordJob::class, fn (SendMessageToDiscordJob $job) => $job->settings === null); +}); + +it('tracks normal notifications', function () { + [$settings] = deliveryFailureChannel($this->team, 'discord'); + Queue::fake(); + + (new DiscordChannel)->send($this->team, new class extends Notification + { + public function toDiscord(): DiscordMessage + { + return new DiscordMessage('Title', 'Description', DiscordMessage::infoColor()); + } + }); + + Queue::assertPushed(SendMessageToDiscordJob::class, fn (SendMessageToDiscordJob $job) => $job->settings?->is($settings)); +}); + +it('tells the team through its other enabled channels when a channel is disabled', function () { + [$settings, $makeJob] = deliveryFailureChannel($this->team, 'discord'); + deliveryFailureChannel($this->team, 'slack'); + $settings->increment('failure_count', 2); + Http::fake(['*' => Http::response(['message' => 'Unknown Webhook', 'code' => 10015], 404)]); + NotificationFacade::fake(); + + $makeJob($settings->fresh())->handle(); + + NotificationFacade::assertSentTo($this->team, GeneralNotification::class, function (GeneralNotification $notification, array $channels) { + return $channels === [SlackChannel::class] + && str_contains($notification->message, 'Discord notifications were disabled'); + }); +}); diff --git a/tests/Unit/Notifications/Channels/EmailChannelTest.php b/tests/Unit/Notifications/Channels/EmailChannelTest.php index 57cc6a7ea1..f5918d6b00 100644 --- a/tests/Unit/Notifications/Channels/EmailChannelTest.php +++ b/tests/Unit/Notifications/Channels/EmailChannelTest.php @@ -1,6 +1,7 @@ 0]); $this->resendStub = null; $this->team = Team::factory()->create(); $user = User::factory()->create(['email' => 'test@example.com']); @@ -28,13 +30,24 @@ beforeEach(function () { ]); $this->team->refresh(); + // A test notification shows every error to the user and never counts as a delivery failure. $this->notification = new class extends Notification { + public bool $isTestNotification = true; + public function toMail(object $notifiable): MailMessage { return (new MailMessage)->subject('Test Email')->line('Test'); } }; + + $this->deliveryNotification = new class extends Notification + { + public function toMail(object $notifiable): MailMessage + { + return (new MailMessage)->subject('Deployment')->line('Deployed'); + } + }; }); afterEach(function () { @@ -165,3 +178,52 @@ it('sends successfully when Resend returns an email id', function () { expect(fn () => (new EmailChannel)->send($this->team, $this->notification)) ->not->toThrow(Throwable::class); }); + +it('counts an invalid team Resend key as a delivery failure without a retry', function () { + $this->resendStub = fakeResendResponse(401, ['statusCode' => 401, 'name' => 'validation_error', 'message' => 'API key is invalid']); + + (new EmailChannel)->send($this->team, $this->deliveryNotification); + + $settings = $this->team->emailNotificationSettings->fresh(); + expect($settings->failure_count)->toBe(1) + ->and($settings->resend_enabled)->toBeTrue(); +}); + +it('disables team Resend after three invalid key failures in a row', function () { + $this->resendStub = fakeResendResponse(401, ['statusCode' => 401, 'name' => 'validation_error', 'message' => 'API key is invalid']); + + foreach (range(1, 3) as $attempt) { + (new EmailChannel)->send($this->team, $this->deliveryNotification); + } + + $settings = $this->team->emailNotificationSettings->fresh(); + expect($settings->resend_enabled)->toBeFalse() + ->and($settings->failure_count)->toBe(0) + ->and($settings->resend_api_key)->toBe('test_api_key'); +}); + +it('retries temporary Resend errors without counting them', function () { + $this->resendStub = fakeResendResponse(500, ['statusCode' => 500, 'name' => 'application_error', 'message' => 'Internal server error.']); + + expect(fn () => (new EmailChannel)->send($this->team, $this->deliveryNotification)) + ->toThrow(Exception::class, 'Failed to send email via Resend: Internal server error.'); + + expect($this->team->emailNotificationSettings->fresh()->failure_count)->toBe(0); +}); + +it('does not count Resend errors for test emails', function () { + $this->resendStub = fakeResendResponse(401, ['statusCode' => 401, 'name' => 'validation_error', 'message' => 'API key is invalid']); + + expect(fn () => (new EmailChannel)->send($this->team, $this->notification))->toThrow(NonReportableException::class); + + expect($this->team->emailNotificationSettings->fresh()->failure_count)->toBe(0); +}); + +it('resets the failure count after a successful delivery', function () { + $this->team->emailNotificationSettings->increment('failure_count', 2); + $this->resendStub = fakeResendResponse(200, ['id' => '49a3999c-0ce1-4ea6-ab68-afcd6dc2e794']); + + (new EmailChannel)->send($this->team, $this->deliveryNotification); + + expect($this->team->emailNotificationSettings->fresh()->failure_count)->toBe(0); +});