From 14a7f8646cad266d567035a7beb9cf6cfd90d54c Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Wed, 25 Mar 2026 12:43:47 +0100 Subject: [PATCH 1/2] fix(backup): prevent notification failures from affecting backup status - Wrap notification calls in try-catch blocks to log failures instead - Prevent failed() method from overwriting successful backup status - Skip failure notifications if backup already completed successfully - Ensures post-backup errors (e.g. notification failures) never retroactively mark successful backups as failed Fixes #9088 --- app/Jobs/DatabaseBackupJob.php | 61 ++++++++++++++------ tests/Feature/DatabaseBackupJobTest.php | 76 +++++++++++++++++++++++++ 2 files changed, 121 insertions(+), 16 deletions(-) diff --git a/app/Jobs/DatabaseBackupJob.php b/app/Jobs/DatabaseBackupJob.php index b55c324be..041d31bad 100644 --- a/app/Jobs/DatabaseBackupJob.php +++ b/app/Jobs/DatabaseBackupJob.php @@ -399,7 +399,15 @@ public function handle(): void 's3_uploaded' => null, ]); } - $this->team?->notify(new BackupFailed($this->backup, $this->database, $this->error_output ?? $this->backup_output ?? $e->getMessage(), $database)); + try { + $this->team?->notify(new BackupFailed($this->backup, $this->database, $this->error_output ?? $this->backup_output ?? $e->getMessage(), $database)); + } catch (\Throwable $notifyException) { + Log::channel('scheduled-errors')->warning('Failed to send backup failure notification', [ + 'backup_id' => $this->backup->uuid, + 'database' => $database, + 'error' => $notifyException->getMessage(), + ]); + } continue; } @@ -439,11 +447,20 @@ public function handle(): void 'local_storage_deleted' => $localStorageDeleted, ]); - // Send appropriate notification - if ($s3UploadError) { - $this->team->notify(new BackupSuccessWithS3Warning($this->backup, $this->database, $database, $s3UploadError)); - } else { - $this->team->notify(new BackupSuccess($this->backup, $this->database, $database)); + // Send appropriate notification (wrapped in try-catch so notification + // failures never affect backup status — see GitHub issue #9088) + try { + if ($s3UploadError) { + $this->team->notify(new BackupSuccessWithS3Warning($this->backup, $this->database, $database, $s3UploadError)); + } else { + $this->team->notify(new BackupSuccess($this->backup, $this->database, $database)); + } + } catch (\Throwable $e) { + Log::channel('scheduled-errors')->warning('Failed to send backup success notification', [ + 'backup_id' => $this->backup->uuid, + 'database' => $database, + 'error' => $e->getMessage(), + ]); } } } @@ -710,20 +727,32 @@ public function failed(?Throwable $exception): void $log = ScheduledDatabaseBackupExecution::where('uuid', $this->backup_log_uuid)->first(); if ($log) { - $log->update([ - 'status' => 'failed', - 'message' => 'Job permanently failed after '.$this->attempts().' attempts: '.($exception?->getMessage() ?? 'Unknown error'), - 'size' => 0, - 'filename' => null, - 'finished_at' => Carbon::now(), - ]); + // Don't overwrite a successful backup status — a post-backup error + // (e.g. notification failure) should not retroactively mark the backup + // as failed (see GitHub issue #9088) + if ($log->status !== 'success') { + $log->update([ + 'status' => 'failed', + 'message' => 'Job permanently failed after '.$this->attempts().' attempts: '.($exception?->getMessage() ?? 'Unknown error'), + 'size' => 0, + 'filename' => null, + 'finished_at' => Carbon::now(), + ]); + } } - // Notify team about permanent failure - if ($this->team) { + // Notify team about permanent failure (only if backup didn't already succeed) + if ($this->team && $log?->status !== 'success') { $databaseName = $log?->database_name ?? 'unknown'; $output = $this->backup_output ?? $exception?->getMessage() ?? 'Unknown error'; - $this->team->notify(new BackupFailed($this->backup, $this->database, $output, $databaseName)); + try { + $this->team->notify(new BackupFailed($this->backup, $this->database, $output, $databaseName)); + } catch (\Throwable $e) { + Log::channel('scheduled-errors')->warning('Failed to send backup permanent failure notification', [ + 'backup_id' => $this->backup->uuid, + 'error' => $e->getMessage(), + ]); + } } } } diff --git a/tests/Feature/DatabaseBackupJobTest.php b/tests/Feature/DatabaseBackupJobTest.php index 37c377dab..05cb21f12 100644 --- a/tests/Feature/DatabaseBackupJobTest.php +++ b/tests/Feature/DatabaseBackupJobTest.php @@ -120,6 +120,82 @@ expect($unrelatedBackup->save_s3)->toBeTruthy(); }); +test('failed method does not overwrite successful backup status', function () { + $team = Team::factory()->create(); + + $backup = ScheduledDatabaseBackup::create([ + 'frequency' => '0 0 * * *', + 'save_s3' => false, + 'database_type' => 'App\Models\StandalonePostgresql', + 'database_id' => 1, + 'team_id' => $team->id, + ]); + + $log = ScheduledDatabaseBackupExecution::create([ + 'uuid' => 'test-uuid-success-guard', + 'database_name' => 'test_db', + 'filename' => '/backup/test.dmp', + 'scheduled_database_backup_id' => $backup->id, + 'status' => 'success', + 'message' => 'Backup completed successfully', + 'size' => 1024, + ]); + + $job = new DatabaseBackupJob($backup); + + $reflection = new ReflectionClass($job); + + $teamProp = $reflection->getProperty('team'); + $teamProp->setValue($job, $team); + + $logUuidProp = $reflection->getProperty('backup_log_uuid'); + $logUuidProp->setValue($job, 'test-uuid-success-guard'); + + // Simulate a post-backup failure (e.g. notification error) + $job->failed(new Exception('Request to the Resend API failed')); + + $log->refresh(); + expect($log->status)->toBe('success'); + expect($log->message)->toBe('Backup completed successfully'); + expect($log->size)->toBe(1024); +}); + +test('failed method updates status when backup was not successful', function () { + $team = Team::factory()->create(); + + $backup = ScheduledDatabaseBackup::create([ + 'frequency' => '0 0 * * *', + 'save_s3' => false, + 'database_type' => 'App\Models\StandalonePostgresql', + 'database_id' => 1, + 'team_id' => $team->id, + ]); + + $log = ScheduledDatabaseBackupExecution::create([ + 'uuid' => 'test-uuid-pending-guard', + 'database_name' => 'test_db', + 'filename' => '/backup/test.dmp', + 'scheduled_database_backup_id' => $backup->id, + 'status' => 'pending', + ]); + + $job = new DatabaseBackupJob($backup); + + $reflection = new ReflectionClass($job); + + $teamProp = $reflection->getProperty('team'); + $teamProp->setValue($job, $team); + + $logUuidProp = $reflection->getProperty('backup_log_uuid'); + $logUuidProp->setValue($job, 'test-uuid-pending-guard'); + + $job->failed(new Exception('Some real failure')); + + $log->refresh(); + expect($log->status)->toBe('failed'); + expect($log->message)->toContain('Some real failure'); +}); + test('s3 storage has scheduled backups relationship', function () { $team = Team::factory()->create(); From 3034e89edb3c01c82468af52ae51c60fdcb23395 Mon Sep 17 00:00:00 2001 From: Andras Bacsai <5845193+andrasbacsai@users.noreply.github.com> Date: Wed, 25 Mar 2026 13:26:50 +0100 Subject: [PATCH 2/2] feat(preview-env): add production variable fallback for docker-compose When preview environment variables are configured, fall back to production variables for keys not overridden by preview values. This ensures variables like DB_PASSWORD that exist only in production are available in the preview .env file, enabling proper ${VAR} interpolation in docker-compose YAML. Fallback only applies when preview variables are configured, preventing unintended leakage of production values when previews aren't in use. Also improves UI by hiding the Domains section when only database services are present, and simplifies the logs view by removing status checks. --- app/Jobs/ApplicationDeploymentJob.php | 16 ++ app/Models/EnvironmentVariable.php | 5 + .../components/applications/links.blade.php | 2 +- .../project/application/general.blade.php | 36 ++- .../livewire/project/shared/logs.blade.php | 58 ++-- tests/Feature/PreviewEnvVarFallbackTest.php | 247 ++++++++++++++++++ 6 files changed, 318 insertions(+), 46 deletions(-) create mode 100644 tests/Feature/PreviewEnvVarFallbackTest.php diff --git a/app/Jobs/ApplicationDeploymentJob.php b/app/Jobs/ApplicationDeploymentJob.php index 9d927d10c..2af380a45 100644 --- a/app/Jobs/ApplicationDeploymentJob.php +++ b/app/Jobs/ApplicationDeploymentJob.php @@ -1333,6 +1333,22 @@ private function generate_runtime_environment_variables() foreach ($runtime_environment_variables_preview as $env) { $envs->push($env->key.'='.$env->real_value); } + + // Fall back to production env vars for keys not overridden by preview vars, + // but only when preview vars are configured. This ensures variables like + // DB_PASSWORD that are only set for production will be available in the + // preview .env file (fixing ${VAR} interpolation in docker-compose YAML), + // while avoiding leaking production values when previews aren't configured. + if ($runtime_environment_variables_preview->isNotEmpty()) { + $previewKeys = $runtime_environment_variables_preview->pluck('key')->toArray(); + $fallback_production_vars = $sorted_environment_variables->filter(function ($env) use ($previewKeys) { + return $env->is_runtime && ! in_array($env->key, $previewKeys); + }); + foreach ($fallback_production_vars as $env) { + $envs->push($env->key.'='.$env->real_value); + } + } + // Add PORT if not exists, use the first port as default if ($this->build_pack !== 'dockercompose') { if ($this->application->environment_variables_preview->where('key', 'PORT')->isEmpty()) { diff --git a/app/Models/EnvironmentVariable.php b/app/Models/EnvironmentVariable.php index cf60d5ab5..5acd4c1e4 100644 --- a/app/Models/EnvironmentVariable.php +++ b/app/Models/EnvironmentVariable.php @@ -32,6 +32,11 @@ )] class EnvironmentVariable extends BaseModel { + protected $attributes = [ + 'is_runtime' => true, + 'is_buildtime' => true, + ]; + protected $fillable = [ // Core identification 'key', diff --git a/resources/views/components/applications/links.blade.php b/resources/views/components/applications/links.blade.php index 26b1cedf5..85e8f7431 100644 --- a/resources/views/components/applications/links.blade.php +++ b/resources/views/components/applications/links.blade.php @@ -4,7 +4,7 @@ @if ( (data_get($application, 'fqdn') || - collect(json_decode($this->application->docker_compose_domains))->count() > 0 || + collect(json_decode($this->application->docker_compose_domains))->contains(fn($fqdn) => !empty(data_get($fqdn, 'domain'))) || data_get($application, 'previews', collect([]))->count() > 0 || data_get($application, 'ports_mappings_array')) && data_get($application, 'settings.is_raw_compose_deployment_enabled') !== true) diff --git a/resources/views/livewire/project/application/general.blade.php b/resources/views/livewire/project/application/general.blade.php index e27eda8b6..d743e346e 100644 --- a/resources/views/livewire/project/application/general.blade.php +++ b/resources/views/livewire/project/application/general.blade.php @@ -49,7 +49,13 @@ !is_null($parsedServices) && count($parsedServices) > 0 && !$application->settings->is_raw_compose_deployment_enabled) -