CLI-1778: success message immediately followed by a failure — confusing and misleading. (#1988)

* CLI-1778: success message immediately followed by a failure — confusing and misleading.

* add test for coverage.

* conflict fixes.

* fix test.

* refactor test.

Author: deepakmishra2

src/Command/Pull/PullCommandBase.php

@@ -31,7 +31,6 @@
 use GuzzleHttp\TransferStats;
 use Psr\Http\Message\UriInterface;
 use Psr\Log\LoggerInterface;
-use React\EventLoop\Loop;
 use SelfUpdate\SelfUpdateManager;
 use Symfony\Component\Console\Helper\ProgressBar;
 use Symfony\Component\Console\Input\ArrayInput;
@@ -259,7 +258,9 @@ static function (mixed $totalBytes, mixed $downloadedBytes) use (&$progress, $ou
         try {
             $codebaseUuid = self::getCodebaseUuid();
             if ($codebaseUuid) {
-                // Download the backup file directly from the provided URL.
+                if (!isset($backupResponse->links->download->href)) {
+                    throw new AcquiaCliException('Cloud API failed to provide a valid backup download URL. The backup may have failed on the server.');
+                }
                 $downloadUrl = $backupResponse->links->download->href;
                 $this->httpClient->request('GET', $downloadUrl, [
                     'progress' => static function (mixed $totalBytes, mixed $downloadedBytes) use (&$progress, $output): void {
@@ -395,15 +396,13 @@ private function getCodeabaseDatabaseBackups(string $siteId, string $environment
     protected function waitForBackup(string $notificationUuid, Client $acquiaCloudClient): void
     {
         $spinnerMessage = 'Waiting for database backup to complete...';
-        $successCallback = function (): void {
-            $this->output->writeln('');
-            $this->output->writeln('<info>Database backup is ready!</info>');
-        };
-        $success = $this->waitForNotificationToComplete($acquiaCloudClient, $notificationUuid, $spinnerMessage, $successCallback);
-        Loop::run();
+        $success = $this->waitForNotificationToComplete($acquiaCloudClient, $notificationUuid, $spinnerMessage, static function (): void {
+        });
         if (!$success) {
             throw new AcquiaCliException('Cloud API failed to create a backup');
         }
+        $this->output->writeln('');
+        $this->output->writeln('<info>Database backup is ready!</info>');
     }
 
     private function connectToLocalDatabase(string $dbHost, string $dbUser, string $dbName, string $dbPassword, ?callable $outputCallback = null): void

tests/phpunit/src/Commands/Pull/PullDatabaseCommandTest.php

@@ -434,10 +434,10 @@ public function testPullNode(): void
     public function testPullDatabasesWithCodebaseUuid(): void
     {
         $codebaseUuid = '11111111-041c-44c7-a486-7972ed2cafc8';
-        self::SetEnvVars(['AH_CODEBASE_UUID' =>  $codebaseUuid]);
+        self::SetEnvVars(['AH_CODEBASE_UUID' => $codebaseUuid]);
 
         // Mock the codebase returned from /codebases/{uuid}.
-        $codebase =  $this->getMockCodeBaseResponse();
+        $codebase = $this->getMockCodeBaseResponse();
         $this->clientProphecy->request('get', '/codebases/' . $codebaseUuid)
             ->willReturn($codebase);
 
@@ -562,10 +562,10 @@ public function testPullDatabasesKillsLogicalAndMutation(): void
     public function testPullDatabasesWithCodebaseUuidOnDemand(): void
     {
         $codebaseUuid = '11111111-041c-44c7-a486-7972ed2cafc8';
-        self::SetEnvVars(['AH_CODEBASE_UUID' =>  $codebaseUuid]);
+        self::SetEnvVars(['AH_CODEBASE_UUID' => $codebaseUuid]);
 
         // Mock the codebase returned from /codebases/{uuid}.
-        $codebase =  $this->getMockCodeBaseResponse();
+        $codebase = $this->getMockCodeBaseResponse();
         $this->clientProphecy->request('get', '/codebases/' . $codebaseUuid)
             ->willReturn($codebase);
 
@@ -600,7 +600,6 @@ public function testPullDatabasesWithCodebaseUuidOnDemand(): void
         $this->clientProphecy->request('post', '/site-instances/8979a8ac-80dc-4df8-b2f0-6be36554a370.3e8ecbec-ea7c-4260-8414-ef2938c859bc/database/backups')
             ->willReturn($createSiteInstanceDatabaseBackup)
             ->shouldBeCalled();
-        ;
         $siteInstanceDatabaseBackups = $this->getMockSiteInstanceDatabaseBackupsResponse();
         $this->clientProphecy->request('get', '/site-instances/8979a8ac-80dc-4df8-b2f0-6be36554a370.3e8ecbec-ea7c-4260-8414-ef2938c859bc/database/backups')
             ->willReturn($siteInstanceDatabaseBackups->_embedded->items)
@@ -647,6 +646,87 @@ public function testPullDatabasesWithCodebaseUuidOnDemand(): void
         self::unsetEnvVars(['AH_CODEBASE_UUID']);
     }
 
+    /**
+     * Tests that a codebase backup response without a download link
+     * throws AcquiaCliException before attempting any HTTP download.
+     */
+    public function testPullDatabasesCodebaseBackupMissingDownloadUrl(): void
+    {
+        $codebaseUuid = '11111111-041c-44c7-a486-7972ed2cafc8';
+        self::SetEnvVars(['AH_CODEBASE_UUID' => $codebaseUuid]);
+
+        $codebase = $this->getMockCodeBaseResponse();
+        $this->clientProphecy->request('get', '/codebases/' . $codebaseUuid)
+            ->willReturn($codebase);
+
+        $codebaseEnv = $this->getMockCodeBaseEnvironment();
+        $this->clientProphecy->request('get', '/codebases/' . $codebaseUuid . '/environments')
+            ->willReturn([$codebaseEnv])
+            ->shouldBeCalled();
+
+        $codeabaseSites = $this->getMockCodeBaseSites();
+        $this->clientProphecy->request('get', '/codebases/' . $codebaseUuid . '/sites')
+            ->willReturn($codeabaseSites);
+        $siteInstance = $this->getMockSiteInstanceResponse();
+
+        $this->clientProphecy->request('get', '/site-instances/8979a8ac-80dc-4df8-b2f0-6be36554a370.3e8ecbec-ea7c-4260-8414-ef2938c859bc')
+            ->willReturn($siteInstance)
+            ->shouldBeCalled();
+        $siteId = '8979a8ac-80dc-4df8-b2f0-6be36554a370';
+        $site = $this->getMockSite();
+        $this->clientProphecy->request('get', '/sites/' . $siteId)
+            ->willReturn($site)
+            ->shouldBeCalled();
+        $siteInstanceDatabase = $this->getMockSiteInstanceDatabaseResponse();
+        $this->clientProphecy->request('get', '/site-instances/8979a8ac-80dc-4df8-b2f0-6be36554a370.3e8ecbec-ea7c-4260-8414-ef2938c859bc/database')
+            ->willReturn($siteInstanceDatabase)
+            ->shouldBeCalled();
+        $siteInstanceDatabaseConnection = $this->getMockSiteInstanceDatabaseConnectionResponse();
+        $this->clientProphecy->request('get', '/site-instances/8979a8ac-80dc-4df8-b2f0-6be36554a370.3e8ecbec-ea7c-4260-8414-ef2938c859bc/database/connection')
+            ->willReturn($siteInstanceDatabaseConnection)
+            ->shouldBeCalled();
+        $createSiteInstanceDatabaseBackup = $this->getMockSiteInstanceDatabaseBackupsResponse('post', '201');
+        $this->clientProphecy->request('post', '/site-instances/8979a8ac-80dc-4df8-b2f0-6be36554a370.3e8ecbec-ea7c-4260-8414-ef2938c859bc/database/backups')
+            ->willReturn($createSiteInstanceDatabaseBackup)
+            ->shouldBeCalled();
+
+        // Return a backup response WITHOUT the download link.
+        $backupWithoutDownload = (object)[
+            'created_at' => '2025-04-01T13:01:06.603Z',
+            'database_id' => 'b0c9dff7-56b6-4c0d-bad0-0e6593f66cd3',
+            'id' => 'e0c9dff7-56b6-4c0d-bad0-0e6593f66cd3',
+            '_links' => (object)[
+                'self' => (object)[
+                    'href' => 'https://environment-service-php.acquia.com/api/site-instances/3e8ecbec-ea7c-4260-8414-ef2938c859bc.d3f7270e-c45f-4801-9308-5e8afe84a323/database/backups/a0c9dff7-56b6-4c0d-bad0-0e6593f66cd3',
+                ],
+            ],
+        ];
+        $this->clientProphecy->request('get', '/site-instances/8979a8ac-80dc-4df8-b2f0-6be36554a370.3e8ecbec-ea7c-4260-8414-ef2938c859bc/database/backups')
+            ->willReturn([$backupWithoutDownload])
+            ->shouldBeCalled();
+
+        // Mock the client options that are set before the download URL check.
+        $this->clientProphecy->addOption('sink', Argument::type('string'))->shouldBeCalled();
+        $this->clientProphecy->addOption('curl.options', Argument::type('array'))->shouldBeCalled();
+        $this->clientProphecy->addOption('progress', Argument::that(static fn($v) => is_callable($v)))->shouldBeCalled();
+        $this->clientProphecy->addOption('on_stats', Argument::that(static fn($v) => is_callable($v)))->shouldBeCalled();
+
+        $localMachineHelper = $this->mockLocalMachineHelper();
+        $this->mockExecuteMySqlConnect($localMachineHelper, true);
+
+        // No HTTP download should be attempted.
+        $this->httpClientProphecy->request('GET', Argument::any(), Argument::any())->shouldNotBeCalled();
+
+        $inputs = self::inputChooseEnvironment();
+
+        $this->expectException(AcquiaCliException::class);
+        $this->expectExceptionMessage('Cloud API failed to provide a valid backup download URL');
+        $this->executeCommand([
+            '--no-scripts' => true,
+            '--on-demand' => true,
+        ], $inputs);
+    }
+
     /**
      * Test catch block in getSiteInstanceDatabaseConnection method.
      * Covers the logger->debug() line when an exception is caught.