Merge pull request #192 from ByteInternet/handle_brancher_already_cancelled

Brancher: Handle already cancelled error

Author: tdgroot

src/Brancher/BrancherHypernodeManager.php

@@ -337,7 +337,20 @@ public function cancel(string ...$brancherHypernodes): void
     {
         foreach ($brancherHypernodes as $brancherHypernode) {
             $this->log->info(sprintf('Stopping brancher Hypernode %s...', $brancherHypernode));
-            $this->hypernodeClient->brancherApp->cancel($brancherHypernode);
+            try {
+                $this->hypernodeClient->brancherApp->cancel($brancherHypernode);
+            } catch (HypernodeApiClientException $e) {
+                // If the brancher is already cancelled or not found, that's fine -
+                // our goal was to cancel it anyway
+                if ($e->getCode() === 404 || str_contains($e->getMessage(), 'has already been cancelled')) {
+                    $this->log->info(sprintf(
+                        'Brancher Hypernode %s was already cancelled or not found, skipping.',
+                        $brancherHypernode
+                    ));
+                    continue;
+                }
+                throw $e;
+            }
         }
     }
 }

tests/Unit/Brancher/BrancherHypernodeManagerTest.php

@@ -7,6 +7,7 @@
 use Hypernode\Api\Exception\HypernodeApiClientException;
 use Hypernode\Api\HypernodeClient;
 use Hypernode\Api\Resource\Logbook\Flow;
+use Hypernode\Api\Service\BrancherApp;
 use Hypernode\Api\Service\Logbook;
 use Hypernode\Deploy\Brancher\BrancherHypernodeManager;
 use Hypernode\Deploy\Exception\CreateBrancherHypernodeFailedException;
@@ -20,6 +21,7 @@ class BrancherHypernodeManagerTest extends TestCase
 {
     private LoggerInterface&MockObject $logger;
     private HypernodeClient&MockObject $hypernodeClient;
+    private BrancherApp&MockObject $brancherApp;
     private Logbook&MockObject $logbook;
     private TestSshPoller $sshPoller;
     private BrancherHypernodeManager $manager;
@@ -28,7 +30,9 @@ protected function setUp(): void
     {
         $this->logger = $this->createMock(LoggerInterface::class);
         $this->hypernodeClient = $this->createMock(HypernodeClient::class);
+        $this->brancherApp = $this->createMock(BrancherApp::class);
         $this->logbook = $this->createMock(Logbook::class);
+        $this->hypernodeClient->brancherApp = $this->brancherApp;
         $this->hypernodeClient->logbook = $this->logbook;
         $this->sshPoller = new TestSshPoller();
         $this->sshPoller->setMicrotime(1000.0);
@@ -258,4 +262,88 @@ private function createFlow(string $name, string $state): Flow
             'state' => $state,
         ]);
     }
+
+    public function testCancelSucceeds(): void
+    {
+        $this->brancherApp->expects($this->once())
+            ->method('cancel')
+            ->with('test-brancher');
+
+        $this->manager->cancel('test-brancher');
+    }
+
+    public function testCancelAlreadyCancelledBrancherContinues(): void
+    {
+        $response = $this->createMock(ResponseInterface::class);
+        $response->method('getStatusCode')->willReturn(400);
+        $response->method('getBody')->willReturn('["Brancher app test-brancher has already been cancelled."]');
+        $exception = new HypernodeApiClientException($response);
+
+        $this->brancherApp->expects($this->once())
+            ->method('cancel')
+            ->with('test-brancher')
+            ->willThrowException($exception);
+
+        $this->logger->expects($this->exactly(2))
+            ->method('info');
+
+        // Should not throw
+        $this->manager->cancel('test-brancher');
+    }
+
+    public function testCancelNotFoundBrancherContinues(): void
+    {
+        $response = $this->createMock(ResponseInterface::class);
+        $response->method('getStatusCode')->willReturn(404);
+        $response->method('getBody')->willReturn('Not Found');
+        $exception = new HypernodeApiClientException($response);
+
+        $this->brancherApp->expects($this->once())
+            ->method('cancel')
+            ->with('test-brancher')
+            ->willThrowException($exception);
+
+        $this->logger->expects($this->exactly(2))
+            ->method('info');
+
+        // Should not throw
+        $this->manager->cancel('test-brancher');
+    }
+
+    public function testCancelOtherClientErrorPropagates(): void
+    {
+        $response = $this->createMock(ResponseInterface::class);
+        $response->method('getStatusCode')->willReturn(403);
+        $response->method('getBody')->willReturn('Forbidden');
+        $exception = new HypernodeApiClientException($response);
+
+        $this->brancherApp->expects($this->once())
+            ->method('cancel')
+            ->with('test-brancher')
+            ->willThrowException($exception);
+
+        $this->expectException(HypernodeApiClientException::class);
+
+        $this->manager->cancel('test-brancher');
+    }
+
+    public function testCancelMultipleBranchersPartialFailure(): void
+    {
+        $response = $this->createMock(ResponseInterface::class);
+        $response->method('getStatusCode')->willReturn(400);
+        $response->method('getBody')->willReturn('["Brancher app brancher-1 has already been cancelled."]');
+        $exception = new HypernodeApiClientException($response);
+
+        $this->brancherApp->expects($this->exactly(2))
+            ->method('cancel')
+            ->willReturnCallback(function (string $name) use ($exception) {
+                if ($name === 'brancher-1') {
+                    throw $exception;
+                }
+                // brancher-2 succeeds
+            });
+
+        // Should not throw, should continue to second brancher
+        $this->manager->cancel('brancher-1', 'brancher-2');
+    }
 }