fix(fallback): Throw RuntimeException class on asset/json I/O failures.#111
fix(fallback): Throw RuntimeException class on asset/json I/O failures.#111terabytesoftw merged 1 commit intomainfrom
RuntimeException class on asset/json I/O failures.#111Conversation
terabytesoftw
commented
Jan 22, 2026
| Q | A |
|---|---|
| Is bugfix? | ✔️ |
| New feature? | ❌ |
| Breaks BC? | ❌ |
📝 WalkthroughSummary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings. WalkthroughThis PR enhances error handling for file I/O operations in JSON and asset fallback classes by introducing RuntimeException throws on read/write/remove failures, alongside comprehensive test coverage using internal mocking infrastructure. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #111 +/- ##
===========================================
Coverage 100.00% 100.00%
- Complexity 343 347 +4
===========================================
Files 26 26
Lines 787 802 +15
===========================================
+ Hits 787 802 +15 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@tests/Fallback/AssetFallbackTest.php`:
- Line 98: The MockerState conditions for file I/O are using exact argument
arrays that don't match how AssetFallback calls file_get_contents and
file_put_contents; update the two MockerState::addCondition invocations (the one
referencing 'file_get_contents' and the one referencing 'file_put_contents') to
match actual signatures or use loose matchers: for file_get_contents use a
single-argument matcher corresponding to the path (e.g. match the first arg only
or a wildcard) and for file_put_contents match two arguments (path and data) or
use a matcher for the first two args instead of expecting flags/offset/null
values; adjust the conditions so they reflect AssetFallback's real calls to
file_get_contents($this->path) and file_put_contents($this->path,
$this->originalContent).
📜 Review details
Configuration used: Organization UI
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (7)
CHANGELOG.mdsrc/Fallback/AssetFallback.phpsrc/Json/JsonFile.phptests/Fallback/AssetFallbackTest.phptests/Json/JsonFileTest.phptests/Support/InternalMockerExtension.phptests/Support/internal-mocker-stubs.php
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: terabytesoftw
Repo: php-forge/support PR: 12
File: src/TestSupport.php:0-0
Timestamp: 2025-08-18T20:13:33.518Z
Learning: User terabytesoftw prefers clean trait-based designs over maintaining backward compatibility wrappers when refactoring utility classes in the php-forge/support project.
Learnt from: terabytesoftw
Repo: php-forge/actions PR: 65
File: .github/workflows/composer-require-checker.yml:80-83
Timestamp: 2025-09-28T15:12:48.345Z
Learning: The user terabytesoftw prefers using floating tags like v1 for third-party actions in GitHub workflows instead of pinning to specific commit SHAs, even when it's a security best practice to pin to immutable commits.
🧬 Code graph analysis (3)
tests/Fallback/AssetFallbackTest.php (1)
src/Exception/RuntimeException.php (1)
RuntimeException(21-23)
tests/Json/JsonFileTest.php (2)
src/Exception/RuntimeException.php (1)
RuntimeException(21-23)src/Json/JsonFormatter.php (1)
getArrayKeys(34-39)
src/Json/JsonFile.php (1)
src/Exception/RuntimeException.php (1)
RuntimeException(21-23)
🪛 PHPMD (2.15.0)
tests/Fallback/AssetFallbackTest.php
98-98: Avoid using static access to class '\Xepozz\InternalMocker\MockerState' in method 'testSaveThrowsWhenFileCannotBeRead'. (undefined)
(StaticAccess)
163-163: Avoid using static access to class '\Xepozz\InternalMocker\MockerState' in method 'testRestoreThrowsWhenWriteFails'. (undefined)
(StaticAccess)
182-182: Missing class import via use statement (line '182', column '38'). (undefined)
(MissingImport)
tests/Json/JsonFileTest.php
94-94: Avoid using static access to class '\Xepozz\InternalMocker\MockerState' in method 'testGetArrayKeysThrowsWhenFileCannotBeRead'. (undefined)
(StaticAccess)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: phpunit / PHP 8.4-windows-latest
- GitHub Check: phpunit / PHP 8.2-ubuntu-latest
🔇 Additional comments (13)
CHANGELOG.md (1)
14-14: LGTM!The changelog entry accurately documents the bug fix and follows the established format.
tests/Json/JsonFileTest.php (2)
16-20: LGTM!The new imports properly support the test for RuntimeException handling and mocking infrastructure.
87-102: LGTM!The test correctly verifies RuntimeException is thrown when file reading fails:
- Creates a valid file first to pass the
exists()check- Mocks
file_get_contentsto returnfalsefor the specific path- Sets exception expectations before calling
getArrayKeys()- Uses regex matcher to handle variable path in the message
The static analysis hint about
MockerState::addConditionis a false positive—this is the intended API for the xepozz/internal-mocker library.tests/Support/InternalMockerExtension.php (2)
58-77: LGTM!The new mock registrations properly cover all file I/O functions used in
JsonFileandAssetFallbackclasses, enabling the failure-path tests.
80-84: LGTM!The explicit path configuration for the Mocker aligns with the updated constructor signature and correctly references the custom stubs file.
src/Json/JsonFile.php (2)
16-17: LGTM!Import of the custom
RuntimeExceptionclass is correct.
91-105: LGTM!The guarded read flow properly handles both scenarios:
- File exists but unreadable → throws
RuntimeExceptionwith descriptive message- File doesn't exist → proceeds with empty content (graceful handling)
The implementation correctly uses inherited
exists()andgetPath()methods fromComposer\Json\JsonFile.tests/Support/internal-mocker-stubs.php (1)
1-21: LGTM!The stub overrides correctly match PHP's function signatures for
file_get_contentsandfile_put_contents, enabling precise argument matching in the mocked functions. The untyped$contextparameters are intentional as they acceptresource|null.src/Fallback/AssetFallback.php (3)
19-19: LGTM!Import of the custom
RuntimeExceptionclass is correct.
40-53: LGTM!The
save()method now properly validates file read operations and throws a descriptiveRuntimeExceptionon failure, ensuring errors surface early rather than silently proceeding with invalid state.
55-80: LGTM!The
restore()method improvements are well-implemented:
- Removal failures are caught and re-thrown with the original exception as cause (good for debugging)
- Write failures are explicitly checked and surfaced
One consideration: if
remove()succeeds butfile_put_contentsfails, the original file is lost. This is an acceptable trade-off since the exception clearly communicates the failure, allowing callers to handle it appropriately.tests/Fallback/AssetFallbackTest.php (2)
19-22: LGTM — required test imports added.
171-199: Good coverage of remove-failure wrapping.
Asserts the wrapped exception and preserves the previous error—nice.
✏️ Tip: You can disable this entire section by setting review_details to false in your review settings.