feat: error handling and performance optimization#78
Open
andreasbergqvist wants to merge 5 commits into
Open
Conversation
Contributor
There was a problem hiding this comment.
Pull request overview
This PR hardens the backup/zip creation flow to avoid producing and persisting corrupt archives on low-performance / I/O-limited hosts by adding ZipArchive error handling, selective compression, integrity verification, safer directory iteration, and additional logging around backup lifecycle events.
Changes:
- Adds ZipArchive return-value checks, selective CM_STORE vs CM_DEFLATE compression, and a
Zipper::verify()helper. - Improves backup robustness with temp cleanup, post-close verification, and a shutdown handler to recover from fatal/timeout termination.
- Extends unit coverage for compression selection and verification behavior.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
| tests/Unit/ZipperTest.php | Adds unit tests for compression method selection and zip verification behavior. |
| src/Support/Zipper.php | Introduces selective compression, adds error handling for several ZipArchive operations, adds verify(), and switches addDirectory() to lazy iteration with progress logging. |
| src/Backuper.php | Adds lifecycle logging, verification before repository persistence, temp zip cleanup on failure, and a shutdown handler intended to recover from fatal termination mid-backup. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Prevent invalid backups on low-performance and I/O-limited shared hosting
Problem
On lower-performance systems (e.g., shared hosting with 512MB RAM and 20MB/s I/O limits like Oderland), backups were producing invalid/corrupt zip files. The root causes were:
Zero error checking on
ZipArchiveoperations —open(),close(),addFile(), andaddFromString()all had their return values ignored. Whenclose()failed (disk full, memory exhaustion, I/O timeout), the code proceeded to move a half-written corrupt zip to the repository as if it succeeded.No zip integrity verification — corrupt zips were moved to the repository without ever being validated.
Re-compressing already-compressed files — Statamic sites store PNG, JPG, PDF, MP4, etc. These formats are already fully compressed. Attempting deflate compression on them wastes CPU cycles, increases I/O bandwidth (read + compress + write instead of just read + write), and makes
close()significantly slower — increasing the window formax_execution_timekills.addDirectory()loaded all file paths into memory —collect(File::allFiles($path))->each()created a Collection of everySplFileInfobefore iterating. For sites with many assets, this could exhaust PHP'smemory_limitbeforeclose()even runs.No safety net for process kills — if PHP was killed by
max_execution_timeduringclose()(a fatal error, not catchable bytry/catch), the temp zip was left on disk, the state was stuck atbackup_in_progress, and no error was logged.No logging — impossible to diagnose failures on remote systems.
Solution
Selective compression (
Zipper::addFile)Media and archive extensions (png, jpg, mp4, pdf, zip, etc.) now use
ZipArchive::CM_STOREinstead ofCM_DEFLATE. This packages pre-compressed assets at raw write speed with zero CPU processing and zero compression buffer overhead. Text-based files (YAML, markdown, config) continue to useCM_DEFLATEfor size reduction.This reduces I/O peak by up to 90% on asset-heavy sites and makes
close()dramatically faster — directly addressing the 20MB/s I/O throttling issue on shared hosting.Error checking on all
ZipArchiveoperations (Zipper)Every
ZipArchivemethod that returns a success/failure indicator now has its return value checked. Failures throwRuntimeExceptionwith descriptive messages instead of silently producing corrupt zips. This is the primary fix for invalid zips —close()returningfalseis now a loud error, not a silent corruption.Zip verification (
Zipper::verify,Backuper)After
close()succeeds, the zip is re-opened to verify integrity (numFiles > 0). If verification fails, the temp file is deleted and an exception is thrown — corrupt zips never reach the repository.Lazy file iteration (
Zipper::addDirectory)Replaced
collect(File::allFiles($path))->each()with a lazySymfony Finderforeachloop. Files are processed one at a time instead of loading all paths into memory first. Progress is logged every 500 files.Process kill safety net (
Backuper)set_time_limit(0)— removes PHP's execution time limit so slow I/O doesn't cause a timeout kill. Guarded withfunction_exists()for hosts that disable it.ignore_user_abort(true)— prevents HTTP client disconnects from killing the process.register_shutdown_function()— runs after fatal errors (includingmax_execution_timekills). If the backup didn't complete, it cleans up the temp file, sets state toBackupFailed, and logs the error. This catches the exact scenario where PHP dies mid-close().Temp file cleanup (
Backuper)The
catchblock now deletes the temp zip if it exists. Previously, failed backups left orphanedtemp.zipfiles consuming disk space.Logging (
Backuper,Zipper)Added
Log::info/Log::errorcalls at key points: backup start, zip close (with size), verification, completion, failure (with error message), and per-directory progress. Uses Laravel's default log channel — no config changes needed.Files changed
src/Support/Zipper.phpZipArchiveops, lazyFinderiteration,verify()method, progress loggingsrc/Backuper.phpset_time_limit/ignore_user_abort/register_shutdown_function, loggingtests/Unit/ZipperTest.phpCompatibility
Zipperinterface is identical)setCompressionNameandsetEncryptionIndexare independent)