fix(backup): prevent notification failures from affecting backup status (#9162)

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
+                    // Send appropriate notification (wrapped in try-catch so notification
-                    if ($s3UploadError) {
+                    // failures never affect backup status — see GitHub issue #9088)
-                        $this->team->notify(new BackupSuccessWithS3Warning($this->backup, $this->database, $database, $s3UploadError));
+                    try {
-                    } else {
+                        if ($s3UploadError) {
-                        $this->team->notify(new BackupSuccess($this->backup, $this->database, $database));
+                            $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([
+            // Don't overwrite a successful backup status — a post-backup error
-                'status' => 'failed',
+            // (e.g. notification failure) should not retroactively mark the backup
-                'message' => 'Job permanently failed after '.$this->attempts().' attempts: '.($exception?->getMessage() ?? 'Unknown error'),
+            // as failed (see GitHub issue #9088)
-                'size' => 0,
+            if ($log->status !== 'success') {
-                'filename' => null,
+                $log->update([
-                'finished_at' => Carbon::now(),
+                    '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
+        // Notify team about permanent failure (only if backup didn't already succeed)
-        if ($this->team) {
+        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(),
+                ]);
+            }
         }
     }
 }

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();