From b526907e8dcb1ad4e308abc75a437534e5b169b3 Mon Sep 17 00:00:00 2001 From: Riddhesh Sanghvi Date: Mon, 29 Jun 2026 22:03:46 +0530 Subject: [PATCH 1/3] fix(backup): make EasyDash backup callback flow consistent The --dash-auth backup path could report success even when no backup remained, and could leave EasyDash with neither a success nor a failure callback. - Defer dash_backup_completed = true until after a confirmed success callback so the shutdown handler still fires the failure callback when the success callback fails and the upload is rolled back. - On success-callback failure, capture_error + EE::error after rollback so the exit code and message reflect that no backup remains, instead of falling through to EE::success. - Add a dash_callback_sent guard set at the actual send sites and checked by the shutdown handler so exactly one terminal callback (success XOR failure) is ever emitted. - Surface a failed rollback purge via capture_error + EE::error so the orphaned-remote-backup state propagates to the exit code and failure callback rather than being swallowed by a warning. --- src/helper/Site_Backup_Restore.php | 51 ++++++++++++++++++++++++------ 1 file changed, 41 insertions(+), 10 deletions(-) diff --git a/src/helper/Site_Backup_Restore.php b/src/helper/Site_Backup_Restore.php index a287b639..6cd18dcc 100644 --- a/src/helper/Site_Backup_Restore.php +++ b/src/helper/Site_Backup_Restore.php @@ -41,6 +41,7 @@ class Site_Backup_Restore { private $dash_api_url; private $dash_backup_metadata; private $dash_backup_completed = false; + private $dash_callback_sent = false; // Guard: exactly one terminal callback (success XOR failure) may be emitted private $dash_new_backup_path; // Track new backup path for potential rollback // Error tracking for EasyDash failure callbacks @@ -154,8 +155,6 @@ public function backup( $args, $assoc_args = [] ) { $this->fs->remove( EE_BACKUP_DIR . '/' . $this->site_data['site_url'] . '.lock' ); - // Mark backup as completed and send success callback - $this->dash_backup_completed = true; if ( $this->dash_auth_enabled ) { $api_success = $this->send_dash_success_callback( $this->dash_api_url, @@ -164,13 +163,27 @@ public function backup( $args, $assoc_args = [] ) { $this->dash_backup_metadata ); - // Only cleanup old backups if API callback succeeded - // If API failed, rollback the newly uploaded backup if ( $api_success ) { + // Backup is now registered with EasyDash; only then is it safe to + // mark complete (suppressing the shutdown failure callback) and prune. + $this->dash_backup_completed = true; $this->cleanup_old_backups(); } else { + // Success callback failed: the upload is orphaned (EasyDash never + // recorded it), so roll it back and report failure. dash_backup_completed + // stays false so the shutdown handler emits the single failure callback; + // EE::error() makes the exit code/message reflect that no backup remains. + $this->capture_error( + 'Backup uploaded but EasyDash success callback failed; rolled back the orphaned upload.', + self::ERROR_TYPE_NETWORK, + 4002 + ); $this->rollback_failed_backup(); + EE::error( 'EasyDash success callback failed; the uploaded backup was rolled back. No backup was created.' ); } + } else { + // Non-dash path: backup is done once the upload succeeds. + $this->dash_backup_completed = true; } // Release global backup lock (also released by shutdown handler as safety net) @@ -189,8 +202,9 @@ public function backup( $args, $assoc_args = [] ) { * explicitly captured during backup execution. */ public function dash_shutdown_handler() { - // Only send failure callback if dash auth was enabled and backup didn't complete - if ( $this->dash_auth_enabled && ! $this->dash_backup_completed ) { + // Only send a failure callback if dash auth was enabled, the backup didn't + // complete, and no terminal callback (success or failure) has been sent yet. + if ( $this->dash_auth_enabled && ! $this->dash_backup_completed && ! $this->dash_callback_sent ) { // If no error was captured yet, try to capture shutdown error if ( empty( $this->dash_error_message ) ) { @@ -1635,10 +1649,16 @@ private function rollback_failed_backup() { $result = EE::launch( sprintf( 'rclone purge %s', escapeshellarg( $this->dash_new_backup_path ) ) ); if ( $result->return_code ) { - EE::warning( sprintf( - 'Failed to delete backup from remote storage. Please manually delete: %s', + // Rollback purge failed: an untracked backup is now orphaned on the + // remote. Surface it so the exit code and failure callback reflect this + // state instead of silently continuing past a warning. capture_error keeps + // the root cause if one was already recorded by the caller. + $message = sprintf( + 'Failed to delete orphaned backup from remote storage. Please manually delete: %s', $this->dash_new_backup_path - ) ); + ); + $this->capture_error( $message, self::ERROR_TYPE_FILESYSTEM, 4003 ); + EE::error( $message ); } else { EE::success( 'Successfully removed unregistered backup from remote storage.' ); } @@ -1727,7 +1747,15 @@ private function send_dash_success_callback( $ed_api_url, $backup_id, $verify_to EE::debug( 'Payload being sent: ' . json_encode( $payload ) ); - return $this->send_dash_request( $endpoint, $payload ); + $success = $this->send_dash_request( $endpoint, $payload ); + + // A success that actually reached EasyDash is the terminal callback; a failed + // attempt is not, so the failure path can still emit the failure callback. + if ( $success ) { + $this->dash_callback_sent = true; + } + + return $success; } /** @@ -1756,6 +1784,9 @@ private function send_dash_failure_callback( $ed_api_url, $backup_id, $verify_to 'error_code' => $payload['error_code'], ] ) ); + // Failure is terminal: mark before sending so a concurrent shutdown pass + // can never emit a second (duplicate) terminal callback. + $this->dash_callback_sent = true; $this->send_dash_request( $endpoint, $payload ); } From b88166fd896cca327d065e76b3d05dce83c8ebd4 Mon Sep 17 00:00:00 2001 From: Riddhesh Sanghvi Date: Mon, 29 Jun 2026 22:18:50 +0530 Subject: [PATCH 2/3] fix(backup): correct EasyDash error code and orphan-purge failure payload Review follow-up to the dash-callback consistency fix. - Give the callback-failure rollback its own error_code 4004; 4002 was already in use for ERROR_TYPE_DATABASE ("Database backup failed"), and error_code is shipped to EasyDash as a machine-readable field. - When the rollback rclone purge also fails, force-overwrite the captured dash error before re-capturing so the accurate FILESYSTEM 4003 "manually delete " message wins over the caller's first-wins optimistic "rolled back" message; otherwise EasyDash was told the inverse of reality (rolled back) and never received the manual path. - Demote the rollback-success notice from EE::success to EE::log since it always precedes a terminal EE::error. --- src/helper/Site_Backup_Restore.php | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/helper/Site_Backup_Restore.php b/src/helper/Site_Backup_Restore.php index 6cd18dcc..c36635e9 100644 --- a/src/helper/Site_Backup_Restore.php +++ b/src/helper/Site_Backup_Restore.php @@ -176,7 +176,7 @@ public function backup( $args, $assoc_args = [] ) { $this->capture_error( 'Backup uploaded but EasyDash success callback failed; rolled back the orphaned upload.', self::ERROR_TYPE_NETWORK, - 4002 + 4004 ); $this->rollback_failed_backup(); EE::error( 'EasyDash success callback failed; the uploaded backup was rolled back. No backup was created.' ); @@ -1649,18 +1649,21 @@ private function rollback_failed_backup() { $result = EE::launch( sprintf( 'rclone purge %s', escapeshellarg( $this->dash_new_backup_path ) ) ); if ( $result->return_code ) { - // Rollback purge failed: an untracked backup is now orphaned on the - // remote. Surface it so the exit code and failure callback reflect this - // state instead of silently continuing past a warning. capture_error keeps - // the root cause if one was already recorded by the caller. + // Rollback purge failed: the untracked backup genuinely survives on the + // remote, so the operator must delete it manually. Force-overwrite any + // optimistic "rolled back" error captured by the caller (capture_error is + // first-wins) so EasyDash receives this accurate path, not the inverse. $message = sprintf( 'Failed to delete orphaned backup from remote storage. Please manually delete: %s', $this->dash_new_backup_path ); + $this->dash_error_message = ''; $this->capture_error( $message, self::ERROR_TYPE_FILESYSTEM, 4003 ); EE::error( $message ); } else { - EE::success( 'Successfully removed unregistered backup from remote storage.' ); + // Demoted from EE::success: this always precedes a terminal EE::error, so a + // green success line would be misleading on a failing command. + EE::log( 'Successfully removed unregistered backup from remote storage.' ); } } From 092e4aaf489f1a3feeae6cd30300ba8acef3db5d Mon Sep 17 00:00:00 2001 From: Riddhesh Sanghvi Date: Thu, 24 Sep 2026 10:39:24 +0000 Subject: [PATCH 3/3] fix(backup): release global lock before EasyDash callback-failure rollback The callback-failure path now exits via EE::error, so the explicit lock release is skipped and the global backup lock stays held through the shutdown failure callback, which can retry for up to 15 minutes when EasyDash is unreachable. Release it before the rollback so queued backups are not delayed further. --- src/helper/Site_Backup_Restore.php | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/helper/Site_Backup_Restore.php b/src/helper/Site_Backup_Restore.php index c36635e9..17de0e85 100644 --- a/src/helper/Site_Backup_Restore.php +++ b/src/helper/Site_Backup_Restore.php @@ -178,6 +178,8 @@ public function backup( $args, $assoc_args = [] ) { self::ERROR_TYPE_NETWORK, 4004 ); + // Don't hold the lock through the shutdown failure callback's retries. + $this->release_global_backup_lock(); $this->rollback_failed_backup(); EE::error( 'EasyDash success callback failed; the uploaded backup was rolled back. No backup was created.' ); }