feat(installer): let a migration report that it did not finish - #519
Merged
Conversation
A migration that depends on something outside the shop, such as an API call, can fail for a reason that will clear up on its own. Throwing is a poor fit: it aborts the upgrade, and because the installer never records a migration that throws, the shop retries a fatal on every load. Completing quietly is no better, since the work is then never attempted again. Timestamped migrations can now call markFailed() instead. The reason is logged as an error, the migration is left unrecorded so it is picked up on the next load, and the remaining migrations still run — one that could not finish should not hold up the rest of the upgrade. Added to TimestampedMigrationInterface rather than a separate contract: AbstractTimestampedMigration is its only implementer, so nothing outside it has to change. Part of INT-1695 Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #519 +/- ##
============================================
+ Coverage 94.30% 94.33% +0.02%
- Complexity 2400 2413 +13
============================================
Files 352 352
Lines 7696 7726 +30
============================================
+ Hits 7258 7288 +30
Misses 438 438 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR extends the installer’s timestamped migration system so a migration can signal “ran but didn’t finish” (without throwing), allowing the upgrade pass to continue while keeping that migration out of applied_migrations for a later retry.
Changes:
- Adds
hasFailed()toTimestampedMigrationInterfaceand amarkFailed()mechanism toAbstractTimestampedMigration(with error logging). - Updates
InstallerService::runUpMigrations()to skip recording failed timestamped migrations while continuing with subsequent migrations. - Adds unit tests and a new mock timestamped migration that marks itself as failed.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Unit/App/Installer/Service/InstallerServiceTest.php | Adds coverage for leaving failed migrations unrecorded and continuing with later migrations. |
| tests/Bootstrap/MockFailingTimestampedMigration.php | Introduces a mock timestamped migration that calls markFailed() for test scenarios. |
| src/App/Installer/Service/InstallerService.php | Skips markMigrationApplied() when a timestamped migration reports failure. |
| src/App/Installer/Migration/AbstractTimestampedMigration.php | Adds failure state tracking and markFailed() logging helper. |
| src/App/Installer/Contract/TimestampedMigrationInterface.php | Extends the contract with hasFailed() to report incomplete runs. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
NabDevs
approved these changes
Aug 6, 2026
joerivanveen
approved these changes
Aug 6, 2026
myparcel-bot Bot
added a commit
that referenced
this pull request
Aug 6, 2026
## [4.7.0](v4.6.0...v4.7.0) (2026-08-06) ### ✨ New Features * **installer:** let a migration report that it did not finish ([#519](#519)) ([6d622e2](6d622e2))
Contributor
|
🎉 This PR is included in version 4.7.0 🎉 The release is available on GitHub release Your semantic-release bot 📦🚀 |
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.
lets a timestamped migration report that it ran but could not finish, so the installer leaves it unrecorded and picks it up again next time.
A migration that depends on something outside the shop, an API call for instance, can fail for a reason that clears up on its own. Neither of the options we had was any good. Throwing aborts the upgrade, and because the installer never records a migration that throws, the shop hits the same fatal on every load. Returning quietly gets the migration marked as done, so the work is never attempted again.
Migrations can now call markFailed() with a reason instead. The reason is logged as an error, the migration stays out of applied_migrations so it runs again on the next load, and the migrations after it still run: one that could not finish should not hold up the rest of the upgrade. The installer reads the result back through hasFailed().
hasFailed() sits on TimestampedMigrationInterface rather than a separate contract. AbstractTimestampedMigration is its only implementer and provides both methods, so no existing migration has to change.
This was written in #514 but never used there. Splitting it out so the carrier-refresh migrations in myparcelnl/prestashop#624 and myparcelnl/woocommerce#1748 can use it without waiting on the no-tracking work.
Part of INT-1695
🤖 Generated with Claude Code