From 19ee1c9be740de037dbe61ec8e82e74f24276322 Mon Sep 17 00:00:00 2001
From: Dan Brown <ssddanbrown@googlemail.com>
Date: Thu, 12 Dec 2024 21:45:52 +0000
Subject: [PATCH] Notifications: Logged errors and prevented them blocking user

Failed notification sends could block the user action, whereas it's
probably more important that the user action takes places uninteruupted
than showing an error screen for the user to debug.
Logs notification errors so issues can still be debugged by admins.

Closes #5315
---
 .../Handlers/BaseNotificationHandler.php      |  7 +++++-
 tests/Activity/WatchTest.php                  | 25 +++++++++++++++++++
 2 files changed, 31 insertions(+), 1 deletion(-)

diff --git a/app/Activity/Notifications/Handlers/BaseNotificationHandler.php b/app/Activity/Notifications/Handlers/BaseNotificationHandler.php
index b5f339b2c..3a9b0c1dc 100644
--- a/app/Activity/Notifications/Handlers/BaseNotificationHandler.php
+++ b/app/Activity/Notifications/Handlers/BaseNotificationHandler.php
@@ -7,6 +7,7 @@ use BookStack\Activity\Notifications\Messages\BaseActivityNotification;
 use BookStack\Entities\Models\Entity;
 use BookStack\Permissions\PermissionApplicator;
 use BookStack\Users\Models\User;
+use Illuminate\Support\Facades\Log;
 
 abstract class BaseNotificationHandler implements NotificationHandler
 {
@@ -36,7 +37,11 @@ abstract class BaseNotificationHandler implements NotificationHandler
             }
 
             // Send the notification
-            $user->notify(new $notification($detail, $initiator));
+            try {
+                $user->notify(new $notification($detail, $initiator));
+            } catch (\Exception $exception) {
+                Log::error("Failed to send email notification to user [id:{$user->id}] with error: {$exception->getMessage()}");
+            }
         }
     }
 }
diff --git a/tests/Activity/WatchTest.php b/tests/Activity/WatchTest.php
index 605b60fd4..c405b07ae 100644
--- a/tests/Activity/WatchTest.php
+++ b/tests/Activity/WatchTest.php
@@ -13,6 +13,8 @@ use BookStack\Activity\Tools\UserEntityWatchOptions;
 use BookStack\Activity\WatchLevels;
 use BookStack\Entities\Models\Entity;
 use BookStack\Settings\UserNotificationPreferences;
+use Illuminate\Contracts\Notifications\Dispatcher;
+use Illuminate\Support\Facades\Mail;
 use Illuminate\Support\Facades\Notification;
 use Tests\TestCase;
 
@@ -365,6 +367,29 @@ class WatchTest extends TestCase
         }
     }
 
+    public function test_failed_notifications_dont_block_and_log_errors()
+    {
+        $logger = $this->withTestLogger();
+        $editor = $this->users->editor();
+        $admin = $this->users->admin();
+        $page = $this->entities->page();
+        $book = $page->book;
+        $activityLogger = app()->make(ActivityLogger::class);
+
+        $watches = new UserEntityWatchOptions($editor, $book);
+        $watches->updateLevelByValue(WatchLevels::UPDATES);
+
+        $mockDispatcher = $this->mock(Dispatcher::class);
+        $mockDispatcher->shouldReceive('send')->once()
+            ->andThrow(\Exception::class, 'Failed to connect to mail server');
+
+        $this->actingAs($admin);
+
+        $activityLogger->add(ActivityType::PAGE_UPDATE, $page);
+
+        $this->assertTrue($logger->hasErrorThatContains("Failed to send email notification to user [id:{$editor->id}] with error: Failed to connect to mail server"));
+    }
+
     public function test_notifications_not_sent_if_lacking_view_permission_for_related_item()
     {
         $notifications = Notification::fake();