fix: ensure deployment failure notifications are sent reliably
**Problem:** Deployment failure notifications were not being sent due to two bugs: 1. **Timing Issue in next() function:** - When failed() called next(FAILED), the database still had status "in_progress" - The notification check looked for ALREADY failed status (not found yet) - Status was updated AFTER the check, losing the notification 2. **Direct Status Update:** - Healthcheck failures directly updated status to FAILED - Bypassed next() entirely, no notification sent **Solution:** Refactored status transition logic with clear separation of concerns: - Moved notification logic AFTER status update (not before) - Created transitionToStatus() as single source of truth - Added completeDeployment() and failDeployment() for clarity - Extracted status-specific side effects into dedicated methods - Updated healthcheck failure to use failDeployment() **Benefits:** - ✅ Notifications sent for ALL failure scenarios - ✅ Clear, self-documenting method names - ✅ Single responsibility per method - ✅ Type-safe using enum instead of strings - ✅ Harder to bypass notification logic accidentally - ✅ Backward compatible (old next() preserved) **Changed:** - app/Jobs/ApplicationDeploymentJob.php (+101/-21 lines) Fixes #6911 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
parent
2c8c9a5e3c
commit
42f916dce2
1 changed files with 103 additions and 23 deletions
|
|
@ -459,7 +459,7 @@ private function decide_what_to_do()
|
|||
private function post_deployment()
|
||||
{
|
||||
GetContainersStatus::dispatch($this->server);
|
||||
$this->next(ApplicationDeploymentStatus::FINISHED->value);
|
||||
$this->completeDeployment();
|
||||
if ($this->pull_request_id !== 0) {
|
||||
if ($this->application->is_github_based()) {
|
||||
ApplicationPullRequestUpdateJob::dispatch(application: $this->application, preview: $this->preview, deployment_uuid: $this->deployment_uuid, status: ProcessStatus::FINISHED);
|
||||
|
|
@ -1008,7 +1008,7 @@ private function just_restart()
|
|||
$this->generate_image_names();
|
||||
$this->check_image_locally_or_remotely();
|
||||
$this->should_skip_build();
|
||||
$this->next(ApplicationDeploymentStatus::FINISHED->value);
|
||||
$this->completeDeployment();
|
||||
}
|
||||
|
||||
private function should_skip_build()
|
||||
|
|
@ -3023,9 +3023,7 @@ private function stop_running_container(bool $force = false)
|
|||
$this->application_deployment_queue->addLogEntry('----------------------------------------');
|
||||
}
|
||||
$this->application_deployment_queue->addLogEntry('New container is not healthy, rolling back to the old container.');
|
||||
$this->application_deployment_queue->update([
|
||||
'status' => ApplicationDeploymentStatus::FAILED->value,
|
||||
]);
|
||||
$this->failDeployment();
|
||||
$this->graceful_shutdown_container($this->container_name);
|
||||
}
|
||||
}
|
||||
|
|
@ -3659,42 +3657,124 @@ private function checkForCancellation(): void
|
|||
}
|
||||
}
|
||||
|
||||
private function next(string $status)
|
||||
/**
|
||||
* Transition deployment to a new status with proper validation and side effects.
|
||||
* This is the single source of truth for status transitions.
|
||||
*/
|
||||
private function transitionToStatus(ApplicationDeploymentStatus $status): void
|
||||
{
|
||||
// Refresh to get latest status
|
||||
$this->application_deployment_queue->refresh();
|
||||
|
||||
// Never allow changing status from FAILED or CANCELLED_BY_USER to anything else
|
||||
if ($this->application_deployment_queue->status === ApplicationDeploymentStatus::FAILED->value) {
|
||||
$this->application->environment->project->team?->notify(new DeploymentFailed($this->application, $this->deployment_uuid, $this->preview));
|
||||
|
||||
if ($this->isInTerminalState()) {
|
||||
return;
|
||||
}
|
||||
|
||||
$this->updateDeploymentStatus($status);
|
||||
$this->handleStatusTransition($status);
|
||||
queue_next_deployment($this->application);
|
||||
}
|
||||
|
||||
/**
|
||||
* Check if deployment is in a terminal state (FAILED or CANCELLED).
|
||||
* Terminal states cannot be changed.
|
||||
*/
|
||||
private function isInTerminalState(): bool
|
||||
{
|
||||
$this->application_deployment_queue->refresh();
|
||||
|
||||
if ($this->application_deployment_queue->status === ApplicationDeploymentStatus::FAILED->value) {
|
||||
return true;
|
||||
}
|
||||
|
||||
if ($this->application_deployment_queue->status === ApplicationDeploymentStatus::CANCELLED_BY_USER->value) {
|
||||
// Job was cancelled, stop execution
|
||||
$this->application_deployment_queue->addLogEntry('Deployment cancelled by user, stopping execution.');
|
||||
throw new \RuntimeException('Deployment cancelled by user', 69420);
|
||||
}
|
||||
|
||||
return false;
|
||||
}
|
||||
|
||||
/**
|
||||
* Update the deployment status in the database.
|
||||
*/
|
||||
private function updateDeploymentStatus(ApplicationDeploymentStatus $status): void
|
||||
{
|
||||
$this->application_deployment_queue->update([
|
||||
'status' => $status,
|
||||
'status' => $status->value,
|
||||
]);
|
||||
}
|
||||
|
||||
queue_next_deployment($this->application);
|
||||
/**
|
||||
* Execute status-specific side effects (events, notifications, additional deployments).
|
||||
*/
|
||||
private function handleStatusTransition(ApplicationDeploymentStatus $status): void
|
||||
{
|
||||
match ($status) {
|
||||
ApplicationDeploymentStatus::FINISHED => $this->handleSuccessfulDeployment(),
|
||||
ApplicationDeploymentStatus::FAILED => $this->handleFailedDeployment(),
|
||||
default => null,
|
||||
};
|
||||
}
|
||||
|
||||
if ($status === ApplicationDeploymentStatus::FINISHED->value) {
|
||||
event(new ApplicationConfigurationChanged($this->application->team()->id));
|
||||
/**
|
||||
* Handle side effects when deployment succeeds.
|
||||
*/
|
||||
private function handleSuccessfulDeployment(): void
|
||||
{
|
||||
event(new ApplicationConfigurationChanged($this->application->team()->id));
|
||||
|
||||
if (! $this->only_this_server) {
|
||||
$this->deploy_to_additional_destinations();
|
||||
}
|
||||
$this->application->environment->project->team?->notify(new DeploymentSuccess($this->application, $this->deployment_uuid, $this->preview));
|
||||
if (! $this->only_this_server) {
|
||||
$this->deploy_to_additional_destinations();
|
||||
}
|
||||
|
||||
$this->sendDeploymentNotification(DeploymentSuccess::class);
|
||||
}
|
||||
|
||||
/**
|
||||
* Handle side effects when deployment fails.
|
||||
*/
|
||||
private function handleFailedDeployment(): void
|
||||
{
|
||||
$this->sendDeploymentNotification(DeploymentFailed::class);
|
||||
}
|
||||
|
||||
/**
|
||||
* Send deployment status notification to the team.
|
||||
*/
|
||||
private function sendDeploymentNotification(string $notificationClass): void
|
||||
{
|
||||
$this->application->environment->project->team?->notify(
|
||||
new $notificationClass($this->application, $this->deployment_uuid, $this->preview)
|
||||
);
|
||||
}
|
||||
|
||||
/**
|
||||
* Complete deployment successfully.
|
||||
* Sends success notification and triggers additional deployments if needed.
|
||||
*/
|
||||
private function completeDeployment(): void
|
||||
{
|
||||
$this->transitionToStatus(ApplicationDeploymentStatus::FINISHED);
|
||||
}
|
||||
|
||||
/**
|
||||
* Fail the deployment.
|
||||
* Sends failure notification and queues next deployment.
|
||||
*/
|
||||
private function failDeployment(): void
|
||||
{
|
||||
$this->transitionToStatus(ApplicationDeploymentStatus::FAILED);
|
||||
}
|
||||
|
||||
/**
|
||||
* @deprecated Use transitionToStatus(), completeDeployment(), or failDeployment() instead.
|
||||
*/
|
||||
private function next(string $status): void
|
||||
{
|
||||
$this->transitionToStatus(ApplicationDeploymentStatus::from($status));
|
||||
}
|
||||
|
||||
public function failed(Throwable $exception): void
|
||||
{
|
||||
$this->next(ApplicationDeploymentStatus::FAILED->value);
|
||||
$this->failDeployment();
|
||||
$this->application_deployment_queue->addLogEntry('Oops something is not okay, are you okay? 😢', 'stderr');
|
||||
if (str($exception->getMessage())->isNotEmpty()) {
|
||||
$this->application_deployment_queue->addLogEntry($exception->getMessage(), 'stderr');
|
||||
|
|
|
|||
Loading…
Reference in a new issue