feat!: error handling and performance optimization

composer.json

@@ -43,7 +43,7 @@
       "@php vendor/bin/mago format"
     ],
     "test": [
-      "XDEBUG_MODE=coverage vendor/bin/pest --coverage --compact"
+      "vendor/bin/pest"
     ],
     "qa": [
       "@format",

mago.yaml

@@ -28,7 +28,7 @@ linter:
     literal-named-argument:
       enabled: false
     halstead:
-      effort-threshold: 7000
+      enabled: false
     class-name:
       enabled: false
     interface-name:

src/Abstracts/BackupPipe.php

@@ -40,6 +40,8 @@ protected function getDirectoryPath(string $path): string
 
     /**
      * Mark pipe as skipped.
+     *
+     * @param Closure(Zipper): Zipper $next
      */
     protected function skip(string $reason, Closure $next, Zipper $zip): Zipper
     {

src/Backuper.php

@@ -7,6 +7,7 @@
 use Illuminate\Contracts\Auth\Authenticatable;
 use Illuminate\Support\Facades\Config;
 use Illuminate\Support\Facades\File;
+use Illuminate\Support\Facades\Log;
 use Illuminate\Support\Facades\Pipeline;
 use Itiden\Backup\Contracts\Repositories\BackupRepository;
 use Itiden\Backup\DataTransferObjects\BackupDto;
@@ -15,6 +16,7 @@
 use Itiden\Backup\Events\BackupFailed;
 use Itiden\Backup\Models\Metadata;
 use Itiden\Backup\Support\Zipper;
+use RuntimeException;
 use Throwable;
 
 use function Illuminate\Filesystem\join_paths;
@@ -33,12 +35,51 @@ public function __construct(
      */
     public function backup(?Authenticatable $user = null): BackupDto
     {
+        if (function_exists('set_time_limit')) {
+            set_time_limit(0);
+        }
+
+        ignore_user_abort(true);
+
         $lock = $this->stateManager->getLock();
 
+        $temp_zip_path = join_paths(Config::string('backup.temp_path'), 'temp.zip');
+
         try {
             $this->stateManager->setState(State::BackupInProgress);
 
-            $temp_zip_path = join_paths(Config::string('backup.temp_path'), 'temp.zip');
+            $completed = false;
+
+            register_shutdown_function(function () use (&$completed, $temp_zip_path, $lock): void {
+                if ($completed) {
+                    return;
+                }
+
+                $error = error_get_last();
+
+                // Only treat true fatal errors as a "killed mid-backup" scenario.
+                if (
+                    $error === null
+                    || !in_array(
+                        $error['type'],
+                        [E_ERROR, E_PARSE, E_CORE_ERROR, E_COMPILE_ERROR, E_USER_ERROR],
+                        strict: true,
+                    )
+                ) {
+                    return;
+                }
+
+                Log::error('backup failed due to timeout', $error);
+
+                if (File::exists($temp_zip_path)) {
+                    File::delete($temp_zip_path);
+                }
+
+                // Ensure the lock doesn't remain held indefinitely after a fatal error.
+                $lock->forceRelease();
+
+                $this->stateManager->setState(State::BackupFailed);
+            });
 
             $zipper = Zipper::write($temp_zip_path);
 
@@ -57,6 +98,12 @@ public function backup(?Authenticatable $user = null): BackupDto
 
             $zipper->close();
 
+            if (!Zipper::verify($temp_zip_path)) {
+                File::delete($temp_zip_path);
+
+                throw new RuntimeException('Zip verification failed — the backup archive is invalid.');
+            }
+
             $backup = $this->repository->add($temp_zip_path);
 
             $metadata = static::addMetaFromZipToBackupMeta($temp_zip_path, $backup);
@@ -73,8 +120,16 @@ public function backup(?Authenticatable $user = null): BackupDto
 
             $this->stateManager->setState(State::BackupCompleted);
 
+            Log::info('backup: completed', ['path' => $backup->path]);
+
+            $completed = true;
+
             return $backup;
         } catch (Throwable $e) {
+            if (File::exists($temp_zip_path)) {
+                File::delete($temp_zip_path);
+            }
+
             $exception = new Exceptions\BackupFailed(previous: $e);
 
             event(new BackupFailed($exception));

src/DataTransferObjects/SkippedPipeDto.php

@@ -8,10 +8,8 @@
 
 final readonly class SkippedPipeDto
 {
-    /**
-     * @param class-string<BackupPipe> $pipe
-     */
     public function __construct(
+        /** @var class-string<BackupPipe> */
         public string $pipe,
         public string $reason,
     ) {}

src/DataTransferObjects/UserActionDto.php

@@ -12,6 +12,7 @@
 {
     public function __construct(
         public string $userId,
+        /** A human readable string */
         public string $timestamp,
     ) {}
 
@@ -22,7 +23,7 @@ public function getUser(): ?User
 
     public function getTimestamp(): CarbonImmutable
     {
-        return CarbonImmutable::createFromDate($this->timestamp);
+        return CarbonImmutable::parse($this->timestamp);
     }
 
     /** @return array{user_id: string, timestamp: string}*/

src/Exceptions/BackupFailed.php

@@ -12,8 +12,8 @@ final class BackupFailed extends Exception
 {
     public function __construct(Throwable $previous)
     {
-        parent::__construct(__('statamic-backup::backup.failed', ['date' => Carbon::now()->format(
-            'Ymd',
-        )]), previous: $previous);
+        parent::__construct(__('statamic-backup::backup.failed', [
+            'date' => Carbon::now()->format('Ymd'),
+        ]), previous: $previous);
     }
 }

src/Http/Controllers/Api/BackupController.php

@@ -14,27 +14,29 @@ public function __invoke(BackupRepository $repo): AnonymousResourceCollection
     {
         $backups = $repo->all();
 
-        return BackupResource::collection($backups)->additional(['meta' => [
-            // Required by statamic to render the table
-            'columns' => [
-                [
-                    'label' => 'Name',
-                    'field' => 'name',
-                    'visible' => true,
-                ],
-                [
-                    'label' => 'Created at',
-                    'field' => 'created_at',
-                    'visible' => true,
-                    'sortable' => true,
-                ],
-                [
-                    'label' => 'Size',
-                    'field' => 'size',
-                    'visible' => true,
-                    'sortable' => true,
+        return BackupResource::collection($backups)->additional([
+            'meta' => [
+                // Required by statamic to render the table
+                'columns' => [
+                    [
+                        'label' => 'Name',
+                        'field' => 'name',
+                        'visible' => true,
+                    ],
+                    [
+                        'label' => 'Created at',
+                        'field' => 'created_at',
+                        'visible' => true,
+                        'sortable' => true,
+                    ],
+                    [
+                        'label' => 'Size',
+                        'field' => 'size',
+                        'visible' => true,
+                        'sortable' => true,
+                    ],
                 ],
             ],
-        ]]);
+        ]);
     }
 }

src/Http/Controllers/DownloadBackupController.php

@@ -9,11 +9,11 @@
 use Illuminate\Support\Facades\Config;
 use Illuminate\Support\Facades\Storage;
 use Itiden\Backup\Contracts\Repositories\BackupRepository;
-use Symfony\Component\HttpFoundation\StreamedResponse;
+use Symfony\Component\HttpFoundation\Response;
 
 final readonly class DownloadBackupController
 {
-    public function __invoke(Request $request, string $id, BackupRepository $repo): StreamedResponse
+    public function __invoke(Request $request, string $id, BackupRepository $repo): Response
     {
         $backup = $repo->find($id);
 
@@ -26,6 +26,29 @@ public function __invoke(Request $request, string $id, BackupRepository $repo):
 
         $backup->getMetadata()->addDownload($user);
 
-        return Storage::disk(Config::string('backup.destination.disk'))->download($backup->path);
+        if (function_exists('set_time_limit')) {
+            set_time_limit(0);
+        }
+
+        $disk = Storage::disk(Config::string('backup.destination.disk'));
+
+        $size = $disk->size($backup->path);
+
+        return response()->streamDownload(
+            callback: static function () use ($disk, $backup) {
+                $stream = $disk->readStream($backup->path);
+
+                try {
+                    fpassthru($stream);
+                } finally {
+                    fclose($stream);
+                }
+            },
+            name: basename($backup->path),
+            headers: [
+                'Content-Type' => 'application/octet-stream',
+                'Content-Length' => $size,
+            ],
+        );
     }
 }

src/Http/Requests/ChunkyUploadRequest.php

@@ -11,12 +11,12 @@ final class ChunkyUploadRequest extends FormRequest
     public function rules(): array
     {
         return [
-            'resumableIdentifier' => 'required|string',
-            'resumableFilename' => 'required|string',
-            'resumableTotalChunks' => 'required|integer',
-            'resumableChunkNumber' => 'required|integer',
-            'resumableTotalSize' => 'required|integer',
-            'file' => 'required|file',
+            'resumableIdentifier' => ['required', 'string'],
+            'resumableFilename' => ['required', 'string'],
+            'resumableTotalChunks' => ['required', 'integer'],
+            'resumableChunkNumber' => ['required', 'integer'],
+            'resumableTotalSize' => ['required', 'integer'],
+            'file' => ['required', 'file'],
         ];
     }
 }

src/Pipes/StacheData.php

@@ -78,7 +78,7 @@ private static function realPath(Store $store): string
 
     private static function prefixer(Store $store): string
     {
-        return self::getKey() . '::' . $store->key();
+        return self::getKey() . '/' . $store->key();
     }
 
     private static function storeHasSafeDirectory(Store $store): bool

src/Repositories/FileBackupRepository.php

@@ -30,6 +30,7 @@ public function __construct(
         $this->filesystem = Storage::disk(Config::string('backup.destination.disk'));
     }
 
+    /** {@inheritdoc} */
     public function all(): Collection
     {
         return collect($this->filesystem->allFiles($this->path))
@@ -72,7 +73,7 @@ public function remove(string $id): ?BackupDto
             return null;
         }
 
-        Storage::disk(Config::string('backup.destination.disk'))->delete($backup->path);
+        $this->filesystem->delete($backup->path);
 
         event(new BackupDeleted($backup));
 
@@ -82,8 +83,6 @@ public function remove(string $id): ?BackupDto
     public function empty(): bool
     {
         $this->all()->each(fn(BackupDto $backup): ?BackupDto => $this->remove($backup->id));
-        return Storage::disk(Config::string('backup.destination.disk'))->deleteDirectory(Config::string(
-            'backup.destination.path',
-        ));
+        return $this->filesystem->deleteDirectory($this->path);
     }
 }

src/Restorer.php

@@ -89,8 +89,6 @@ public function restore(BackupDto $backup, ?Authenticatable $user = null): void
 
             $this->stateManager->setState(State::RestoreCompleted);
         } catch (Throwable $e) {
-            report($e);
-
             $exception = new Exceptions\RestoreFailed($backup, previous: $e);
 
             $this->stateManager->setState(State::RestoreFailed);