Refactoring LongPollingHandler

src/LongPollingHandler.php

@@ -16,12 +16,16 @@ final readonly class LongPollingHandler
      * @param Api $api
      * @param UpdateDispatcher $dispatcher The update dispatcher.
      * @param LoggerInterface $logger PSR LoggerInterface.
+     * @codeCoverageIgnore
      */
     public function __construct(
         private Api $api,
         private UpdateDispatcher $dispatcher,
         private LoggerInterface $logger,
     ) {
+        if (!(\PHP_SAPI === 'cli')) {
+            throw new \RuntimeException('LongPollingHandler can only be used in CLI mode.');
+        }
     }
 
     /**
@@ -32,12 +36,19 @@ final readonly class LongPollingHandler
      * @return int|null The new marker to be used for the next iteration.
      * @throws \Exception Re-throws exceptions from the API or dispatcher.
      */
-    public function processSingleBatch(int $timeout, ?int $marker): ?int
+    public function processUpdates(int $timeout, ?int $marker): ?int
     {
         $updateList = $this->api->getUpdates(timeout: $timeout, marker: $marker);
 
         foreach ($updateList->updates as $update) {
-            $this->dispatcher->dispatch($update);
+            try {
+                $this->dispatcher->dispatch($update);
+            } catch (\Throwable $e) {
+                $this->logger->error('Error dispatching update', [
+                    'message' => $e->getMessage(),
+                    'exception' => $e,
+                ]);
+            }
         }
 
         return $updateList->marker;
@@ -52,10 +63,11 @@ final readonly class LongPollingHandler
      */
     public function handle(int $timeout = 90, ?int $marker = null): void
     {
+        $this->listenSignals();
         // @phpstan-ignore-next-line
         while (true) {
             try {
-                $marker = $this->processSingleBatch($timeout, $marker);
+                $marker = $this->processUpdates($timeout, $marker);
             } catch (NetworkException $e) {
                 $this->logger->error(
                     'Long-polling network error: {message}',
@@ -71,4 +83,22 @@ final readonly class LongPollingHandler
             }
         }
     }
+
+    /**
+     * @codeCoverageIgnore
+     */
+    protected function listenSignals(): void
+    {
+        if (extension_loaded('pcntl')) {
+            pcntl_async_signals(true);
+
+            $kill = static function () {
+                exit(0);
+            };
+
+            pcntl_signal(SIGINT, $kill);
+            pcntl_signal(SIGQUIT, $kill);
+            pcntl_signal(SIGTERM, $kill);
+        }
+    }
 }

tests/LongPollingHandlerTest.php

@@ -20,6 +20,7 @@ use PHPUnit\Framework\Attributes\CoversClass;
 use PHPUnit\Framework\Attributes\DataProvider;
 use PHPUnit\Framework\Attributes\PreserveGlobalState;
 use PHPUnit\Framework\Attributes\RunInSeparateProcess;
+use PHPUnit\Framework\Attributes\Test;
 use PHPUnit\Framework\Attributes\UsesClass;
 use PHPUnit\Framework\TestCase;
 use Psr\Log\LoggerInterface;
@@ -27,6 +28,9 @@ use Psr\Log\LoggerInterface;
 #[CoversClass(LongPollingHandler::class)]
 #[UsesClass(UpdateDispatcher::class)]
 #[UsesClass(UpdateList::class)]
+#[UsesClass(AbstractUpdate::class)]
+#[UsesClass(BotStartedUpdate::class)]
+#[UsesClass(User::class)]
 final class LongPollingHandlerTest extends TestCase
 {
     use PHPMock;
@@ -36,11 +40,12 @@ final class LongPollingHandlerTest extends TestCase
      * @param int $expectedDispatchCount
      * @param int|null $expectedMarker
      */
-    #[DataProvider('processSingleBatchProvider')]
-    public function testProcessSingleBatch(
+    #[Test]
+    #[DataProvider('processUpdatesProvider')]
+    public function processUpdates(
         array $updatesToReturn,
         int $expectedDispatchCount,
-        ?int $expectedMarker
+        ?int $expectedMarker,
     ): void {
         $apiMock = $this->createMock(Api::class);
         $loggerMock = $this->createMock(LoggerInterface::class);
@@ -60,7 +65,7 @@ final class LongPollingHandlerTest extends TestCase
 
         $handler = new LongPollingHandler($apiMock, $dispatcher, $loggerMock);
 
-        $returnedMarker = $handler->processSingleBatch(90, null);
+        $returnedMarker = $handler->processUpdates(90, null);
 
         $this->assertSame(
             $expectedDispatchCount,
@@ -70,7 +75,7 @@ final class LongPollingHandlerTest extends TestCase
         $this->assertSame($expectedMarker, $returnedMarker, 'Method should return the correct marker.');
     }
 
-    public static function processSingleBatchProvider(): array
+    public static function processUpdatesProvider(): array
     {
         $user = new User(1, 'Test', null, null, false, time());
         $update1 = new BotStartedUpdate(time(), 1, $user, null, null);
@@ -90,9 +95,10 @@ final class LongPollingHandlerTest extends TestCase
         ];
     }
 
+    #[Test]
     #[PreserveGlobalState(false)]
     #[RunInSeparateProcess]
-    public function testRunCatchesNetworkExceptionAndSleeps5Seconds(): void
+    public function runCatchesNetworkExceptionAndSleeps5Seconds(): void
     {
         $apiMock = $this->createMock(Api::class);
         $loggerMock = $this->createMock(LoggerInterface::class);
@@ -121,9 +127,10 @@ final class LongPollingHandlerTest extends TestCase
         }
     }
 
+    #[Test]
     #[PreserveGlobalState(false)]
     #[RunInSeparateProcess]
-    public function testRunCatchesGenericExceptionAndSleeps1Second(): void
+    public function runCatchesGenericExceptionAndSleeps1Second(): void
     {
         $apiMock = $this->createMock(Api::class);
         $loggerMock = $this->createMock(LoggerInterface::class);
@@ -151,4 +158,43 @@ final class LongPollingHandlerTest extends TestCase
             $this->assertSame('Stop test loop', $e->getMessage());
         }
     }
+
+    #[Test]
+    public function processUpdatesContinuesAndLogsWhenHandlerThrows(): void
+    {
+        $apiMock = $this->createMock(Api::class);
+        $loggerMock = $this->createMock(LoggerInterface::class);
+
+        $dispatcher = new UpdateDispatcher($apiMock, $loggerMock);
+
+        $user = new User(1, 'Test', null, null, false, time());
+        $updateToFail = new BotStartedUpdate(time(), 1, $user, null, null);
+        $updateToSucceed = new BotStartedUpdate(time(), 2, $user, null, null);
+
+        $updateList = new UpdateList([$updateToFail, $updateToSucceed], 12345);
+        $exception = new Exception('Error inside handler');
+
+        $apiMock->expects($this->once())
+            ->method('getUpdates')
+            ->willReturn($updateList);
+
+        $handlerCallCount = 0;
+        $dispatcher->addHandler(UpdateType::BotStarted, function () use (&$handlerCallCount, $exception) {
+            $currentCall = $handlerCallCount++;
+            if ($currentCall === 0) {
+                throw $exception;
+            }
+        });
+
+        $loggerMock->expects($this->once())
+            ->method('error')
+            ->with('Error dispatching update', ['message' => 'Error inside handler', 'exception' => $exception]);
+
+        $handler = new LongPollingHandler($apiMock, $dispatcher, $loggerMock);
+
+        $returnedMarker = $handler->processUpdates(90, null);
+
+        $this->assertSame(12345, $returnedMarker, 'Method should return marker even if a handler failed.');
+        $this->assertSame(2, $handlerCallCount, 'Dispatcher should have attempted to process both updates.');
+    }
 }