Notifications: Logged errors and prevented them blocking user
Some checks are pending
analyse-php / build (push) Waiting to run
lint-php / build (push) Waiting to run
test-migrations / build (8.1) (push) Waiting to run
test-migrations / build (8.2) (push) Waiting to run
test-migrations / build (8.3) (push) Waiting to run
test-migrations / build (8.4) (push) Waiting to run
test-php / build (8.1) (push) Waiting to run
test-php / build (8.2) (push) Waiting to run
test-php / build (8.3) (push) Waiting to run
test-php / build (8.4) (push) Waiting to run

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
This commit is contained in:
Dan Brown 2024-12-12 21:45:52 +00:00
parent fcf0bf79a9
commit 19ee1c9be7
No known key found for this signature in database
GPG Key ID: 46D9F943C24A2EF9
2 changed files with 31 additions and 1 deletions

View File

@ -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()}");
}
}
}
}

View File

@ -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();