fix: prevent cleanup exceptions from marking successful deployments as failed

Fixes #7439 where successful deployments were being marked as FAILED due to exceptions during old container cleanup.

Root cause: Commit 97550f406 wrapped stop_running_container() in try-catch that re-throws ALL exceptions as DeploymentException. When old containers are already removed (a common scenario), the "No such container" error propagates and marks successful deployments as failed.

Solution: Check if deployment has already succeeded (newVersionIsHealthy || force) before re-throwing exceptions from cleanup operations. Cleanup failures are logged but don't fail the deployment.

- Add conditional handling in stop_running_container() catch block
- Log cleanup warnings with hidden: true to avoid UI clutter
- Only re-throw exceptions if deployment hasn't succeeded yet
- Preserves backward compatibility and expected behavior

🤖 Generated with [Claude Code](https://claude.com/claude-code)

Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
Andras Bacsai 2025-12-02 17:28:48 +01:00
parent afb19114cf
commit cfea11f189

View file

@ -3187,6 +3187,19 @@ private function stop_running_container(bool $force = false)
$this->graceful_shutdown_container($this->container_name);
}
} catch (Exception $e) {
// If new version is healthy, this is just cleanup - don't fail the deployment
if ($this->newVersionIsHealthy || $force) {
$this->application_deployment_queue->addLogEntry(
"Warning: Could not remove old container: {$e->getMessage()}",
'stderr',
hidden: true
);
\Log::warning("Failed to stop running container {$this->container_name}: {$e->getMessage()}");
return; // Don't re-throw - cleanup failures shouldn't fail successful deployments
}
// Only re-throw if deployment hasn't succeeded yet
throw new DeploymentException("Failed to stop running container: {$e->getMessage()}", $e->getCode(), $e);
}
}