From 5b95e15402b10bf32a9e234f747e2a50ed9c265d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Ha=C5=82as?= Date: Fri, 1 May 2026 00:04:32 +0200 Subject: [PATCH 1/2] fix(handlers): validate dynamic method calls to prevent worker crashes Extract shared validation into ServiceMethodHelper and move method existence check inside try-catch for graceful error handling. ProcessHandler and TaskHandler now: - Validate service method format (serviceId::methodName) via shared helper - Check method_exists() inside try-catch, dispatching error events instead of crashing the worker - Gracefully handle runtime exceptions from the service method Adds unit tests for: - ServiceMethodHelper (valid/invalid formats) - ProcessHandler (success, missing method, runtime error, invalid format) - TaskHandler (success, missing method, runtime error, invalid format) Closes #153 --- src/Scheduler/TaskHandler.php | 8 +- src/Supervisor/ProcessHandler.php | 8 +- src/Util/ServiceMethodHelper.php | 34 +++++++ tests/Scheduler/TaskHandlerTest.php | 123 ++++++++++++++++++++++++ tests/Supervisor/ProcessHandlerTest.php | 123 ++++++++++++++++++++++++ tests/Util/ServiceMethodHelperTest.php | 62 ++++++++++++ 6 files changed, 356 insertions(+), 2 deletions(-) create mode 100644 src/Util/ServiceMethodHelper.php create mode 100644 tests/Scheduler/TaskHandlerTest.php create mode 100644 tests/Supervisor/ProcessHandlerTest.php create mode 100644 tests/Util/ServiceMethodHelperTest.php diff --git a/src/Scheduler/TaskHandler.php b/src/Scheduler/TaskHandler.php index 396e3a2..a9ba2bd 100644 --- a/src/Scheduler/TaskHandler.php +++ b/src/Scheduler/TaskHandler.php @@ -6,6 +6,7 @@ use CrazyGoat\WorkermanBundle\Event\TaskErrorEvent; use CrazyGoat\WorkermanBundle\Event\TaskStartEvent; +use CrazyGoat\WorkermanBundle\Util\ServiceMethodHelper; use Psr\Container\ContainerInterface; use Symfony\Contracts\EventDispatcher\EventDispatcherInterface; @@ -19,13 +20,18 @@ public function __construct( public function __invoke(string $service, string $taskName): void { - [$serviceName, $method] = explode('::', $service, 2); + [$serviceName, $method] = ServiceMethodHelper::split($service); $service = $this->locator->get($serviceName); assert(is_object($service)); $this->eventDispatcher->dispatch(new TaskStartEvent($service::class, $taskName)); try { + if (!method_exists($service, $method)) { + throw new \InvalidArgumentException( + sprintf('Method "%s" does not exist on service "%s" (class "%s").', $method, $serviceName, $service::class), + ); + } $service->$method(); } catch (\Throwable $e) { $this->eventDispatcher->dispatch(new TaskErrorEvent($e, $service::class, $taskName)); diff --git a/src/Supervisor/ProcessHandler.php b/src/Supervisor/ProcessHandler.php index 37d0b68..6ce3142 100644 --- a/src/Supervisor/ProcessHandler.php +++ b/src/Supervisor/ProcessHandler.php @@ -6,6 +6,7 @@ use CrazyGoat\WorkermanBundle\Event\ProcessErrorEvent; use CrazyGoat\WorkermanBundle\Event\ProcessStartEvent; +use CrazyGoat\WorkermanBundle\Util\ServiceMethodHelper; use Psr\Container\ContainerInterface; use Symfony\Contracts\EventDispatcher\EventDispatcherInterface; @@ -19,13 +20,18 @@ public function __construct( public function __invoke(string $service, string $processName): void { - [$serviceName, $method] = explode('::', $service, 2); + [$serviceName, $method] = ServiceMethodHelper::split($service); $service = $this->locator->get($serviceName); assert(is_object($service)); $this->eventDispatcher->dispatch(new ProcessStartEvent($service::class, $processName)); try { + if (!method_exists($service, $method)) { + throw new \InvalidArgumentException( + sprintf('Method "%s" does not exist on service "%s" (class "%s").', $method, $serviceName, $service::class), + ); + } $service->$method(); } catch (\Throwable $e) { $this->eventDispatcher->dispatch(new ProcessErrorEvent($e, $service::class, $processName)); diff --git a/src/Util/ServiceMethodHelper.php b/src/Util/ServiceMethodHelper.php new file mode 100644 index 0000000..1bff81a --- /dev/null +++ b/src/Util/ServiceMethodHelper.php @@ -0,0 +1,34 @@ +called = true; + } + }; + + $locator = $this->createMock(ContainerInterface::class); + $locator->method('get')->with('my_task_service')->willReturn($service); + + $eventDispatcher = $this->createMock(EventDispatcherInterface::class); + $eventDispatcher->expects($this->once()) + ->method('dispatch') + ->with($this->isInstanceOf(TaskStartEvent::class)); + + $handler = new TaskHandler($locator, $eventDispatcher); + $handler->__invoke('my_task_service::execute', 'test_task'); + + $this->assertTrue($service->called); + } + + public function testInvokeDispatchesErrorEventWhenMethodDoesNotExist(): void + { + $service = new class { + }; + + $locator = $this->createMock(ContainerInterface::class); + $locator->method('get')->with('my_task_service')->willReturn($service); + + $eventDispatcher = $this->createMock(EventDispatcherInterface::class); + $eventDispatcher->expects($this->exactly(2)) + ->method('dispatch') + ->willReturnCallback(function ($event) { + static $callCount = 0; + ++$callCount; + if ($callCount === 1) { + $this->assertInstanceOf(TaskStartEvent::class, $event); + } elseif ($callCount === 2) { + $this->assertInstanceOf(TaskErrorEvent::class, $event); + } + + return $event; + }); + + $handler = new TaskHandler($locator, $eventDispatcher); + $handler->__invoke('my_task_service::nonexistent', 'test_task'); + // Should not throw - error is caught by try-catch and dispatched as event + } + + public function testInvokeDispatchesErrorEventWhenServiceMethodThrowsException(): void + { + $service = new class { + public function execute(): void + { + throw new \RuntimeException('Task failed'); + } + }; + + $locator = $this->createMock(ContainerInterface::class); + $locator->method('get')->with('my_task_service')->willReturn($service); + + $eventDispatcher = $this->createMock(EventDispatcherInterface::class); + $eventDispatcher->expects($this->exactly(2)) + ->method('dispatch') + ->willReturnCallback(function ($event) { + static $callCount = 0; + ++$callCount; + if ($callCount === 1) { + $this->assertInstanceOf(TaskStartEvent::class, $event); + } elseif ($callCount === 2) { + $this->assertInstanceOf(TaskErrorEvent::class, $event); + $this->assertInstanceOf(\RuntimeException::class, $event->getError()); + $this->assertSame('Task failed', $event->getError()->getMessage()); + } + + return $event; + }); + + $handler = new TaskHandler($locator, $eventDispatcher); + $handler->__invoke('my_task_service::execute', 'test_task'); + } + + /** @dataProvider provideInvalidServiceStrings */ + public function testInvokeThrowsExceptionOnInvalidFormat(string $input): void + { + $locator = $this->createMock(ContainerInterface::class); + $eventDispatcher = $this->createMock(EventDispatcherInterface::class); + + $this->expectException(\InvalidArgumentException::class); + $this->expectExceptionMessage('Invalid service method format'); + + $handler = new TaskHandler($locator, $eventDispatcher); + $handler->__invoke($input, 'test_task'); + } + + /** @return iterable */ + public static function provideInvalidServiceStrings(): iterable + { + yield 'missing separator' => ['JustAService']; + yield 'empty service ID' => ['::method']; + yield 'empty method name' => ['service::']; + yield 'empty string' => ['']; + } +} diff --git a/tests/Supervisor/ProcessHandlerTest.php b/tests/Supervisor/ProcessHandlerTest.php new file mode 100644 index 0000000..31d4253 --- /dev/null +++ b/tests/Supervisor/ProcessHandlerTest.php @@ -0,0 +1,123 @@ +called = true; + } + }; + + $locator = $this->createMock(ContainerInterface::class); + $locator->method('get')->with('my_service')->willReturn($service); + + $eventDispatcher = $this->createMock(EventDispatcherInterface::class); + $eventDispatcher->expects($this->once()) + ->method('dispatch') + ->with($this->isInstanceOf(ProcessStartEvent::class)); + + $handler = new ProcessHandler($locator, $eventDispatcher); + $handler->__invoke('my_service::run', 'test_process'); + + $this->assertTrue($service->called); + } + + public function testInvokeDispatchesErrorEventWhenMethodDoesNotExist(): void + { + $service = new class { + }; + + $locator = $this->createMock(ContainerInterface::class); + $locator->method('get')->with('my_service')->willReturn($service); + + $eventDispatcher = $this->createMock(EventDispatcherInterface::class); + $eventDispatcher->expects($this->exactly(2)) + ->method('dispatch') + ->willReturnCallback(function ($event) { + static $callCount = 0; + ++$callCount; + if ($callCount === 1) { + $this->assertInstanceOf(ProcessStartEvent::class, $event); + } elseif ($callCount === 2) { + $this->assertInstanceOf(ProcessErrorEvent::class, $event); + } + + return $event; + }); + + $handler = new ProcessHandler($locator, $eventDispatcher); + $handler->__invoke('my_service::nonexistent', 'test_process'); + // Should not throw - error is caught by try-catch and dispatched as event + } + + public function testInvokeDispatchesErrorEventWhenServiceMethodThrowsException(): void + { + $service = new class { + public function run(): void + { + throw new \RuntimeException('Something went wrong'); + } + }; + + $locator = $this->createMock(ContainerInterface::class); + $locator->method('get')->with('my_service')->willReturn($service); + + $eventDispatcher = $this->createMock(EventDispatcherInterface::class); + $eventDispatcher->expects($this->exactly(2)) + ->method('dispatch') + ->willReturnCallback(function ($event) { + static $callCount = 0; + ++$callCount; + if ($callCount === 1) { + $this->assertInstanceOf(ProcessStartEvent::class, $event); + } elseif ($callCount === 2) { + $this->assertInstanceOf(ProcessErrorEvent::class, $event); + $this->assertInstanceOf(\RuntimeException::class, $event->getError()); + $this->assertSame('Something went wrong', $event->getError()->getMessage()); + } + + return $event; + }); + + $handler = new ProcessHandler($locator, $eventDispatcher); + $handler->__invoke('my_service::run', 'test_process'); + } + + /** @dataProvider provideInvalidServiceStrings */ + public function testInvokeThrowsExceptionOnInvalidFormat(string $input): void + { + $locator = $this->createMock(ContainerInterface::class); + $eventDispatcher = $this->createMock(EventDispatcherInterface::class); + + $this->expectException(\InvalidArgumentException::class); + $this->expectExceptionMessage('Invalid service method format'); + + $handler = new ProcessHandler($locator, $eventDispatcher); + $handler->__invoke($input, 'test_process'); + } + + /** @return iterable */ + public static function provideInvalidServiceStrings(): iterable + { + yield 'missing separator' => ['JustAService']; + yield 'empty service ID' => ['::method']; + yield 'empty method name' => ['service::']; + yield 'empty string' => ['']; + } +} diff --git a/tests/Util/ServiceMethodHelperTest.php b/tests/Util/ServiceMethodHelperTest.php new file mode 100644 index 0000000..2b9764f --- /dev/null +++ b/tests/Util/ServiceMethodHelperTest.php @@ -0,0 +1,62 @@ +assertSame($expectedServiceId, $serviceId); + $this->assertSame($expectedMethod, $method); + } + + /** @return iterable */ + public static function provideValidServiceStrings(): iterable + { + yield 'simple service and method' => ['App\Service::handle', 'App\Service', 'handle']; + yield 'service with dots' => ['app.my_service::execute', 'app.my_service', 'execute']; + yield 'method with underscore' => ['SomeService::my_method', 'SomeService', 'my_method']; + } + + /** @dataProvider provideInvalidServiceStrings */ + public function testSplitThrowsExceptionOnInvalidFormat(string $input, string $expectedMessage): void + { + $this->expectException(\InvalidArgumentException::class); + $this->expectExceptionMessage($expectedMessage); + + ServiceMethodHelper::split($input); + } + + /** @return iterable */ + public static function provideInvalidServiceStrings(): iterable + { + yield 'missing separator' => [ + 'JustAService', + 'Invalid service method format "JustAService". Expected "serviceId::methodName".', + ]; + yield 'empty service ID' => [ + '::method', + 'Invalid service method format "::method". Expected "serviceId::methodName".', + ]; + yield 'empty method name' => [ + 'service::', + 'Invalid service method format "service::". Expected "serviceId::methodName".', + ]; + yield 'empty string' => [ + '', + 'Invalid service method format "". Expected "serviceId::methodName".', + ]; + yield 'only separator' => [ + '::', + 'Invalid service method format "::". Expected "serviceId::methodName".', + ]; + } +} From 67345b53f6d7d423b70e11528c79c98ebf39a4e8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Piotr=20Ha=C5=82as?= Date: Fri, 1 May 2026 00:08:03 +0200 Subject: [PATCH 2/2] fix: apply ReturnNeverTypeRector to test methods --- tests/Scheduler/TaskHandlerTest.php | 2 +- tests/Supervisor/ProcessHandlerTest.php | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/Scheduler/TaskHandlerTest.php b/tests/Scheduler/TaskHandlerTest.php index 1e61986..b7bcf38 100644 --- a/tests/Scheduler/TaskHandlerTest.php +++ b/tests/Scheduler/TaskHandlerTest.php @@ -69,7 +69,7 @@ public function testInvokeDispatchesErrorEventWhenMethodDoesNotExist(): void public function testInvokeDispatchesErrorEventWhenServiceMethodThrowsException(): void { $service = new class { - public function execute(): void + public function execute(): never { throw new \RuntimeException('Task failed'); } diff --git a/tests/Supervisor/ProcessHandlerTest.php b/tests/Supervisor/ProcessHandlerTest.php index 31d4253..a110393 100644 --- a/tests/Supervisor/ProcessHandlerTest.php +++ b/tests/Supervisor/ProcessHandlerTest.php @@ -69,7 +69,7 @@ public function testInvokeDispatchesErrorEventWhenMethodDoesNotExist(): void public function testInvokeDispatchesErrorEventWhenServiceMethodThrowsException(): void { $service = new class { - public function run(): void + public function run(): never { throw new \RuntimeException('Something went wrong'); }