fix: PR #46 review — error handling, failure notifications, cleanup
Generic: Project CI / Tests (push) Blocked by required conditions
Generic: Project CI / Tests (pull_request) Blocked by required conditions
Joomla: Extension CI / Tests (PHP 8.2) (pull_request) Blocked by required conditions
Joomla: Extension CI / Tests (PHP 8.3) (pull_request) Blocked by required conditions
Joomla: Extension CI / PHPStan Analysis (pull_request) Blocked by required conditions
Joomla: Extension CI / Build RC Pre-Release (pull_request) Blocked by required conditions
Generic: Repo Health / Scripts governance (push) Blocked by required conditions
Generic: Repo Health / Repository health (push) Blocked by required conditions
Generic: Repo Health / Report Issues (push) Blocked by required conditions
Universal: PR Check / Build RC Package (pull_request) Blocked by required conditions
Universal: PR Check / Report Issues (pull_request) Blocked by required conditions
Generic: Repo Health / Scripts governance (pull_request) Blocked by required conditions
Generic: Repo Health / Repository health (pull_request) Blocked by required conditions
Generic: Repo Health / Report Issues (pull_request) Blocked by required conditions
Generic: Repo Health / Access control (push) Successful in 1s
Generic: Repo Health / Site Health (push) Has been skipped
Joomla: Extension CI / Release Readiness Check (pull_request) Failing after 7s
Generic: Project CI / Lint & Validate (push) Successful in 11s
Universal: Auto Version Bump / Version Bump (push) Successful in 4s
Universal: PR Check / Branch Policy (pull_request) Successful in 2s
Universal: Secret Scanning / Gitleaks Secret Scan (pull_request) Successful in 10s
Generic: Repo Health / Site Health (pull_request) Has been skipped
Generic: Repo Health / Access control (pull_request) Successful in 2s
Universal: Pre-Release / Build Pre-Release (${{ inputs.stability || github.ref_name }}) (push) Successful in 8s
Universal: PR Check / Validate PR (pull_request) Failing after 8s
Generic: Project CI / Lint & Validate (pull_request) Successful in 35s
Joomla: Extension CI / Lint & Validate (pull_request) Failing after 38s

Critical:
- Wrap cleanupOldBackups() in try-catch to prevent admin panel crash
- Add missing fields (total_size, files_count, etc.) to failure record
  so failure notifications actually send

High:
- Log unlink failures in deleteBackupRecord() instead of silent return
- Wrap DB delete in try-catch so one failed record doesn't abort loop
- Check for ext-curl before calling curl_init() in sendNtfy()

Medium:
- Change runPreActionBackup catch from \Exception to \Throwable
- Log warning for skipped files during archive encryption
- Truncate ntfy response body in error logs (200 chars max)
This commit is contained in:
Jonathan Miller
2026-06-18 10:26:44 -05:00
parent f47a99636b
commit 9656a2a92b
3 changed files with 44 additions and 19 deletions
@@ -305,15 +305,19 @@ class BackupEngine
$this->log('FATAL: ' . $e->getMessage());
$update = (object) [
'id' => $recordId,
'status' => 'fail',
'description' => $description ?: '',
'backup_type' => $profile->backup_type ?? 'full',
'origin' => $origin,
'archivename' => $archiveName,
'backupstart' => $now ?? date('Y-m-d H:i:s'),
'backupend' => date('Y-m-d H:i:s'),
'log' => implode("\n", $this->log),
'id' => $recordId,
'status' => 'fail',
'description' => $description ?: '',
'backup_type' => $profile->backup_type ?? 'full',
'origin' => $origin,
'archivename' => $archiveName,
'backupstart' => $now ?? date('Y-m-d H:i:s'),
'backupend' => date('Y-m-d H:i:s'),
'total_size' => 0,
'files_count' => 0,
'tables_count' => 0,
'remote_filename' => '',
'log' => implode("\n", $this->log),
];
$db->updateObject('#__mokosuitebackup_records', $update, 'id');
@@ -487,6 +491,7 @@ class BackupEngine
$name = $zip->getNameIndex($i);
if ($name === false) {
$this->log('WARNING: Could not read file at index ' . $i . ' during encryption — file may remain unencrypted');
continue;
}
@@ -169,6 +169,12 @@ class NotificationSender
return false;
}
if (!function_exists('curl_init')) {
error_log('MokoSuiteBackup: ntfy notifications require ext-curl');
return false;
}
try {
$config = Factory::getApplication()->getConfig();
$siteName = $config->get('sitename', 'Joomla Site');
@@ -219,7 +225,7 @@ class NotificationSender
}
if ($httpCode < 200 || $httpCode >= 300) {
error_log('MokoSuiteBackup: ntfy returned HTTP ' . $httpCode . ': ' . $response);
error_log('MokoSuiteBackup: ntfy returned HTTP ' . $httpCode . ': ' . substr((string) $response, 0, 200));
return false;
}
@@ -138,6 +138,15 @@ final class MokoSuiteBackup extends CMSPlugin implements SubscriberInterface
* A profile value of 0 means "use the global default".
*/
private function cleanupOldBackups(): void
{
try {
$this->doCleanup();
} catch (\Throwable $e) {
error_log('MokoSuiteBackup: cleanupOldBackups() failed: ' . $e->getMessage());
}
}
private function doCleanup(): void
{
$db = Factory::getDbo();
$globalMaxAge = (int) ComponentHelper::getParams('com_mokosuitebackup')->get('max_age_days', 30);
@@ -219,10 +228,11 @@ final class MokoSuiteBackup extends CMSPlugin implements SubscriberInterface
{
if (!empty($record->absolute_path) && is_file($record->absolute_path)) {
if (!@unlink($record->absolute_path)) {
return; // Don't delete DB record if file can't be removed
error_log('MokoSuiteBackup: Could not delete backup file (id=' . $record->id . '): ' . $record->absolute_path);
return;
}
// Also remove the log file if it exists alongside the archive
$logPath = preg_replace('/\.(zip|tar\.gz)$/i', '.log', $record->absolute_path);
if (is_file($logPath)) {
@@ -230,12 +240,16 @@ final class MokoSuiteBackup extends CMSPlugin implements SubscriberInterface
}
}
$db->setQuery(
$db->getQuery(true)
->delete($db->quoteName('#__mokosuitebackup_records'))
->where($db->quoteName('id') . ' = ' . (int) $record->id)
);
$db->execute();
try {
$db->setQuery(
$db->getQuery(true)
->delete($db->quoteName('#__mokosuitebackup_records'))
->where($db->quoteName('id') . ' = ' . (int) $record->id)
);
$db->execute();
} catch (\Exception $e) {
error_log('MokoSuiteBackup: Could not delete backup record ' . $record->id . ': ' . $e->getMessage());
}
}
/**
@@ -291,7 +305,7 @@ final class MokoSuiteBackup extends CMSPlugin implements SubscriberInterface
'warning'
);
}
} catch (\Exception $e) {
} catch (\Throwable $e) {
error_log('MokoSuiteBackup: ' . $description . ' failed: ' . $e->getMessage());
Factory::getApplication()->enqueueMessage(
'MokoSuiteBackup: ' . $description . ' failed — ' . $e->getMessage(),