fix: address PR review findings — error handling and safety

Fixes from code review and silent failure audit:

- SnapshotRestoreEngine: catch only duplicate key errors (MySQL 1062)
  in merge mode, re-throw all other exceptions instead of swallowing
- SnapshotRestoreEngine: add json_last_error() check for better error
  messages on corrupt snapshot files
- SnapshotRestoreEngine: log warnings when set_time_limit/ini_set fail
- SnapshotEngine: use strlen($json) instead of filesize() to avoid
  race conditions; catch \Exception instead of \Throwable
- SnapshotsController: remove @unlink suppression, add try-catch
  around delete loop with partial failure reporting
- script.php: add user-facing warning when webcron secret generation
  fails (was silently swallowed, inconsistent with other catch blocks)

source/packages/com_mokosuitebackup/language/en-GB/com_mokosuitebackup.ini

@@ -333,6 +333,7 @@ COM_MOKOJOOMBACKUP_SNAPSHOT_RESTORE_TYPES="Types to restore"
 COM_MOKOJOOMBACKUP_SNAPSHOT_REPLACE_WARNING="Replace mode will delete all current content of the selected types before restoring from the snapshot. This cannot be undone."
 COM_MOKOJOOMBACKUP_SNAPSHOTS_N_DELETED="%d snapshot(s) deleted."
 COM_MOKOJOOMBACKUP_SNAPSHOTS_1_DELETED="1 snapshot deleted."
+COM_MOKOJOOMBACKUP_SNAPSHOTS_DELETE_ERRORS="Failed to delete snapshot(s): %s"
 
 ; Snapshot ACL
 COM_MOKOSUITEBACKUP_ACTION_SNAPSHOT_MANAGE="Manage Snapshots"

source/packages/com_mokosuitebackup/src/Controller/SnapshotsController.php

@@ -131,33 +131,49 @@ class SnapshotsController extends AdminController
 
 		$db      = Factory::getDbo();
 		$deleted = 0;
+		$errors  = [];
 
 		foreach ($cid as $id) {
 			$id = (int) $id;
 
-			// Load record to get file path
-			$query = $db->getQuery(true)
-				->select($db->quoteName('data_file'))
-				->from($db->quoteName('#__mokosuitebackup_snapshots'))
-				->where($db->quoteName('id') . ' = ' . $id);
-			$db->setQuery($query);
-			$dataFile = $db->loadResult();
+			try {
+				// Load record to get file path
+				$query = $db->getQuery(true)
+					->select($db->quoteName('data_file'))
+					->from($db->quoteName('#__mokosuitebackup_snapshots'))
+					->where($db->quoteName('id') . ' = ' . $id);
+				$db->setQuery($query);
+				$dataFile = $db->loadResult();
 
-			// Delete data file
-			if ($dataFile && is_file($dataFile)) {
-				@unlink($dataFile);
+				// Delete data file
+				if ($dataFile && is_file($dataFile)) {
+					if (!unlink($dataFile)) {
+						error_log('MokoSuiteBackup: Failed to delete snapshot file: ' . $dataFile);
+					}
+				}
+
+				// Delete record
+				$query = $db->getQuery(true)
+					->delete($db->quoteName('#__mokosuitebackup_snapshots'))
+					->where($db->quoteName('id') . ' = ' . $id);
+				$db->setQuery($query);
+				$db->execute();
+				$deleted++;
+			} catch (\Exception $e) {
+				error_log('MokoSuiteBackup: Failed to delete snapshot ' . $id . ': ' . $e->getMessage());
+				$errors[] = $id;
 			}
-
-			// Delete record
-			$query = $db->getQuery(true)
-				->delete($db->quoteName('#__mokosuitebackup_snapshots'))
-				->where($db->quoteName('id') . ' = ' . $id);
-			$db->setQuery($query);
-			$db->execute();
-			$deleted++;
 		}
 
-		$this->setMessage(Text::plural('COM_MOKOJOOMBACKUP_SNAPSHOTS_N_DELETED', $deleted));
+		if (!empty($errors)) {
+			$this->setMessage(
+				Text::plural('COM_MOKOJOOMBACKUP_SNAPSHOTS_N_DELETED', $deleted)
+				. ' ' . Text::sprintf('COM_MOKOJOOMBACKUP_SNAPSHOTS_DELETE_ERRORS', implode(', ', $errors)),
+				'warning'
+			);
+		} else {
+			$this->setMessage(Text::plural('COM_MOKOJOOMBACKUP_SNAPSHOTS_N_DELETED', $deleted));
+		}
 		$this->setRedirect(Route::_('index.php?option=com_mokosuitebackup&view=snapshots', false));
 	}
 }

source/packages/com_mokosuitebackup/src/Engine/SnapshotEngine.php

@@ -139,7 +139,7 @@ class SnapshotEngine
 				throw new \RuntimeException('Failed to write snapshot file: ' . $filePath);
 			}
 
-			$fileSize = filesize($filePath);
+			$fileSize = strlen($json);
 			$this->log('Snapshot saved: ' . $filename . ' (' . number_format($fileSize) . ' bytes)');
 
 			// Create database record
@@ -174,7 +174,7 @@ class SnapshotEngine
 				),
 				'id' => $record->id,
 			];
-		} catch (\Throwable $e) {
+		} catch (\Exception $e) {
 			$this->log('FATAL: ' . $e->getMessage());
 
 			return [

source/packages/com_mokosuitebackup/src/Engine/SnapshotRestoreEngine.php

@@ -46,8 +46,13 @@ class SnapshotRestoreEngine
 	 */
 	public function restore(int $snapshotId, string $mode = 'replace', array $contentTypes = []): array
 	{
-		@set_time_limit(0);
-		@ini_set('memory_limit', '512M');
+		if (!@set_time_limit(0)) {
+			$this->log('WARNING: Could not disable time limit — large restores may timeout');
+		}
+
+		if (!@ini_set('memory_limit', '512M')) {
+			$this->log('WARNING: Could not increase memory limit to 512M');
+		}
 
 		if (!in_array($mode, ['replace', 'merge'])) {
 			return ['success' => false, 'message' => 'Invalid restore mode: ' . $mode];
@@ -85,8 +90,12 @@ class SnapshotRestoreEngine
 
 		$data = json_decode($json, true);
 
+		if (json_last_error() !== JSON_ERROR_NONE) {
+			return ['success' => false, 'message' => 'Snapshot file contains invalid JSON: ' . json_last_error_msg()];
+		}
+
 		if (!is_array($data) || empty($data['tables'])) {
-			return ['success' => false, 'message' => 'Invalid snapshot data format'];
+			return ['success' => false, 'message' => 'Invalid snapshot data format: missing tables key'];
 		}
 
 		$snapshotTypes = $data['content_types'] ?? [];
@@ -206,11 +215,16 @@ class SnapshotRestoreEngine
 					$db->insertObject($realTable, $obj);
 				}
 			} else {
-				// Composite key tables — just insert (for merge, we skip duplicates)
+				// Composite key tables — insert, skip genuine duplicates
 				try {
 					$db->insertObject($realTable, $obj);
 				} catch (\Exception $e) {
-					// Duplicate key — skip silently in merge mode
+					if (str_contains($e->getMessage(), 'Duplicate entry') || $e->getCode() === 1062) {
+						$this->log('  Skipped duplicate in ' . $abstractTable);
+						continue;
+					}
+
+					throw $e;
 				}
 			}
 

source/script.php

@@ -232,6 +232,11 @@ class Pkg_MokoSuiteBackupInstallerScript
 			$db->execute();
 		} catch (\Exception $e) {
 			error_log('MokoSuiteBackup: generateWebcronSecret() failed: ' . $e->getMessage());
+			Factory::getApplication()->enqueueMessage(
+				'MokoSuiteBackup could not generate a random webcron secret. '
+				. 'Please set one manually in the component options to secure the webcron endpoint.',
+				'warning'
+			);
 		}
 	}