From 8b9c57f580fd9ae437d104dd4815c33e8c778197 Mon Sep 17 00:00:00 2001 From: Maxime Rainville Date: Tue, 3 Dec 2024 15:51:45 +1300 Subject: [PATCH] Implement error logging and better testing set up --- README.md | 40 +++++++++++-- _config/events.yml | 9 +-- composer.json | 9 ++- src/Service/EventService.php | 17 ++++++ src/Service/TestEventService.php | 65 ++++++++++++++++++++++ tests/php/Service/EventServiceTest.php | 18 ++++++ tests/php/Service/TestEventServiceTest.php | 46 +++++++++++++++ 7 files changed, 193 insertions(+), 11 deletions(-) create mode 100644 src/Service/TestEventService.php create mode 100644 tests/php/Service/TestEventServiceTest.php diff --git a/README.md b/README.md index 42061ce..fc47be0 100644 --- a/README.md +++ b/README.md @@ -236,22 +236,45 @@ return `null` if the DataObject has been deleted. `DataObjectEvent::getObject(true) will attempt to retrieve the exact version of the DataObject that fired the event, assuming it was versioned. -## Testing Your Events +## Handling Errors in Event Listeners -### Writing Event Tests +Exceptions thrown by an event listener will not stop the execution of follow events. By default, those exceptions will be sent to `EventService::handleError()` who will logged them to the default Silverstripe CMS logger. + +You can provide your own error handler with Injector. + +```yml +--- +Name: custom-event-service +After: + - '#event-service' +--- +SilverStripe\Core\Injector\Injector: + ArchiPro\EventDispatcher\AsyncEventDispatcher: + errorhandler: [MyCustomEventHandler, handleError] +``` + +## Testing your Events When testing your event listeners, you'll need to: 1. Dispatch your events 2. Run the event loop 3. Assert the expected outcomes +You can also use the `TestEventService` to test your events. The `TestEventService` will replace the default `EventService` and log any exceptions thrown by listeners. + +You need to require the `colinodell/psr-testlogger` package in your dev dependencies to use the `TestEventService`. + +``` +composer require colinodell/psr-testlogger --dev +``` + Here's an example test: ```php use Revolt\EventLoop; use SilverStripe\Dev\SapphireTest; use SilverStripe\Core\Injector\Injector; -use ArchiPro\Silverstripe\EventDispatcher\Service\EventService; +use ArchiPro\Silverstripe\EventDispatcher\Service\TestEventService; class MyEventTest extends SapphireTest { @@ -260,8 +283,9 @@ class MyEventTest extends SapphireTest // Create your test event $event = new MyCustomEvent('test message'); - // Get the event service - $service = Injector::inst()->get(EventService::class); + // Get the Test Event Service ... this will replace the default EventService with a TestEventService + // with an implementation that will log errors to help with debugging. + $service = TestEventService::bootstrap(); // Add your test listener ... or if you have already $wasCalled = false; @@ -281,6 +305,12 @@ class MyEventTest extends SapphireTest MyCustomEventListener::wasCalled(), 'Assert some side effect of the event being handled' ); + + $this->assertCount( + 0, + $service->getTestLogger()->records, + 'No errors were logged' + ); } } ``` diff --git a/_config/events.yml b/_config/events.yml index 6579fbe..3fd59c0 100644 --- a/_config/events.yml +++ b/_config/events.yml @@ -1,5 +1,5 @@ --- -Name: events +Name: event-service After: - '#coreservices' --- @@ -7,17 +7,18 @@ SilverStripe\Core\Injector\Injector: # Define the listener provider ArchiPro\EventDispatcher\ListenerProvider: class: ArchiPro\EventDispatcher\ListenerProvider - + # Default event dispatcher ArchiPro\EventDispatcher\AsyncEventDispatcher: class: ArchiPro\EventDispatcher\AsyncEventDispatcher constructor: listenerProvider: '%$ArchiPro\EventDispatcher\ListenerProvider' + errorhandler: [ArchiPro\Silverstripe\EventDispatcher\Service\EventService, handleError] Psr\EventDispatcher\EventDispatcherInterface: alias: '%$ArchiPro\EventDispatcher\AsyncEventDispatcher' # Bootstrap the event service ArchiPro\Silverstripe\EventDispatcher\Service\EventService: - constructor: + constructor: dispatcher: '%$ArchiPro\EventDispatcher\AsyncEventDispatcher' - listenerProvider: '%$ArchiPro\EventDispatcher\ListenerProvider' \ No newline at end of file + listenerProvider: '%$ArchiPro\EventDispatcher\ListenerProvider' diff --git a/composer.json b/composer.json index 1a898be..685dceb 100644 --- a/composer.json +++ b/composer.json @@ -9,13 +9,15 @@ "silverstripe/versioned": "^1.13 || ^2.0", "psr/event-dispatcher": "^1.0", "psr/event-dispatcher-implementation": "^1.0", - "archipro/revolt-event-dispatcher": "^0.0.0" + "archipro/revolt-event-dispatcher": "^0.1.0", + "psr/log": "^1 || ^2 || ^3" }, "require-dev": { "phpunit/phpunit": "^9.5", "squizlabs/php_codesniffer": "^3.0", "friendsofphp/php-cs-fixer": "^3.0", - "phpstan/phpstan": "^1.10" + "phpstan/phpstan": "^1.10", + "colinodell/psr-testlogger": "^1.0" }, "autoload": { "psr-4": { @@ -43,5 +45,8 @@ "composer/installers": true, "silverstripe/vendor-plugin": true } + }, + "suggest": { + "colinodell/psr-testlogger": "To use the TestEventService, you must require the 'colinodell/psr-testlogger' package in your dev dependencies." } } diff --git a/src/Service/EventService.php b/src/Service/EventService.php index cf64f92..83587e3 100644 --- a/src/Service/EventService.php +++ b/src/Service/EventService.php @@ -6,9 +6,11 @@ use ArchiPro\EventDispatcher\AsyncEventDispatcher; use ArchiPro\EventDispatcher\ListenerProvider; use ArchiPro\Silverstripe\EventDispatcher\Contract\ListenerLoaderInterface; +use Psr\Log\LoggerInterface; use SilverStripe\Core\Config\Configurable; use SilverStripe\Core\Injector\Injectable; use SilverStripe\Core\Injector\Injector; +use Throwable; /** * Core service class for handling event dispatching in Silverstripe. @@ -148,4 +150,19 @@ public function disableDispatch(): void { $this->suppressDispatch = true; } + + /** + * Handle an error that occurred during event dispatching by logging them + * with the default Silverstripe CMS error handler logger. + * + * @internal This method is wired to the AsyncEventDispatcher with the Injector + * + * @see _config/events.yml + */ + public static function handleError(Throwable $error): void + { + Injector::inst() + ->get(LoggerInterface::class) + ->error($error->getMessage(), ['exception' => $error]); + } } diff --git a/src/Service/TestEventService.php b/src/Service/TestEventService.php new file mode 100644 index 0000000..3670cda --- /dev/null +++ b/src/Service/TestEventService.php @@ -0,0 +1,65 @@ +logger = new TestLogger(); + + $listenerProvider = Injector::inst()->get(ListenerProvider::class); + $dispatcher = new AsyncEventDispatcher( + $listenerProvider, + Closure::fromCallable([$this, 'recordError']) + ); + parent::__construct($dispatcher, $listenerProvider); + } + + /** + * Bootstrap the TestEventService. Will replace the default EventService with a TestEventService. + */ + public static function bootstrap(): self + { + $service = new self(); + Injector::inst()->registerService($service, AsyncEventDispatcher::class); + return $service; + } + + /** + * Catch errors and store them for later inspection. + */ + private function recordError(Throwable $message): void + { + $this->logger->error($message->getMessage(), ['exception' => $message]); + } + + /** + * Test logger where exception thrown by listeners are logged. + */ + public function getTestLogger(): TestLogger + { + return $this->logger; + } +} diff --git a/tests/php/Service/EventServiceTest.php b/tests/php/Service/EventServiceTest.php index 6e253fd..bf4dfd8 100644 --- a/tests/php/Service/EventServiceTest.php +++ b/tests/php/Service/EventServiceTest.php @@ -4,6 +4,8 @@ use ArchiPro\Silverstripe\EventDispatcher\Service\EventService; use ArchiPro\Silverstripe\EventDispatcher\Tests\TestListenerLoader; +use ColinODell\PsrTestLogger\TestLogger; +use Psr\Log\LoggerInterface; use Revolt\EventLoop; use SilverStripe\Core\Injector\Injector; use SilverStripe\Dev\SapphireTest; @@ -114,4 +116,20 @@ public function testEventDispatchWithDisabledDispatch(): void // Assert listener was called $this->assertTrue($result->handled, 'Event listener should have been called when dispatch is re-enabled'); } + + public function testHandleError(): void + { + // Arrange + $testLogger = new TestLogger(); + Injector::inst()->registerService($testLogger, LoggerInterface::class); + $ex = new \Exception('Test error'); + + // Act + EventService::handleError($ex); + + // Assert + $this->assertCount(1, $testLogger->records, 'Error should be to default error logger'); + $this->assertEquals('Test error', $testLogger->records[0]['message'], 'Error message should be "Test error"'); + $this->assertEquals($ex, $testLogger->records[0]['context']['exception'], 'Error should be the same exception'); + } } diff --git a/tests/php/Service/TestEventServiceTest.php b/tests/php/Service/TestEventServiceTest.php new file mode 100644 index 0000000..ae1aa29 --- /dev/null +++ b/tests/php/Service/TestEventServiceTest.php @@ -0,0 +1,46 @@ +service = TestEventService::bootstrap(); + } + + public function testGetTestLogger(): void + { + // Create test event + $event = new class () {}; + + // Add test listener + $this->service->addListener(get_class($event), function ($event) { + throw new Exception('Test exception'); + }); + + $this->assertFalse( + $this->service->getTestLogger()->hasErrorRecords(), + 'No exceptions have been thrown yet' + ); + + // Dispatch event + $this->service->dispatch($event); + + EventLoop::run(); + + $this->assertCount( + 1, + $this->service->getTestLogger()->records, + 'Running the event loop will cause an error to be logged' + ); + } +}