refactor(ErrorHandler): Enhance error handling with template response and default configuration. #53
Conversation
… default configuration.
WalkthroughThe changes introduce a reusable template response mechanism for error handling in the Changes
Sequence Diagram(s)sequenceDiagram
participant App as StatelessApplication
participant EH as ErrorHandler
participant Resp as Response
App->>EH: setResponse(Response)
App->>EH: handle error
EH->>EH: createErrorResponse()
alt Template Response set
EH->>Resp: clone and clear template Response
else No template Response
EH->>Resp: create new Response with default config
end
EH->>App: return error Response
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~15 minutes Possibly related PRs
Poem
✨ Finishing Touches
🧪 Generate unit tests
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
composer.json(1 hunks)src/http/ErrorHandler.php(5 hunks)src/http/StatelessApplication.php(1 hunks)tests/TestCase.php(0 hunks)tests/http/StatelessApplicationTest.php(7 hunks)tests/support/stub/SiteController.php(0 hunks)
💤 Files with no reviewable changes (2)
- tests/support/stub/SiteController.php
- tests/TestCase.php
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#21
File: tests/http/PSR7ResponseTest.php:0-0
Timestamp: 2025-07-22T00:50:26.546Z
Learning: In yii2-extensions/psr-bridge, the ResponseAdapter::formatCookieHeader() method uses `$expire !== 1` to skip validation for Yii2's special deletion cookies, but this should be extended to handle all expired cookies, not just the special case where expire=1.
🧬 Code Graph Analysis (2)
src/http/StatelessApplication.php (1)
src/http/ErrorHandler.php (1)
setResponse(157-160)
src/http/ErrorHandler.php (2)
src/http/Response.php (1)
Response(38-102)tests/http/ErrorHandlerTest.php (1)
ErrorHandlerTest(15-393)
⏰ 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: mutation / PHP 8.3-ubuntu-latest
- GitHub Check: mutation / PHP 8.3-ubuntu-latest
🔇 Additional comments (14)
composer.json (1)
23-23: LGTM!The reordering of the infection dependency is fine and doesn't affect functionality.
src/http/StatelessApplication.php (1)
408-408: LGTM! Excellent integration with the enhanced ErrorHandler.This line properly integrates with the new template response mechanism in ErrorHandler, ensuring that error responses inherit the application's response configuration (format, charset, etc.). The placement is correct - after application reset but before request processing.
tests/http/StatelessApplicationTest.php (7)
8-8: LGTM!The import of
PHPForge\Support\Assertis correctly added to support theAssert::equalsWithoutLEmethod used in the updated test assertions.
433-433: LGTM!Adding the
handle()call ensures the application processes a request before checking container definitions, which is necessary for the container to be properly initialized.
702-702: LGTM!Adding the
handle()call ensures the application processes a request before memory limit recalculation tests, which is necessary for proper initialization.
753-768: LGTM!The change from
self::assertStringContainsStringtoAssert::equalsWithoutLEimproves test precision by asserting exact string equality while handling cross-platform line ending differences.
809-824: LGTM!The change from
self::assertStringContainsStringtoAssert::equalsWithoutLEimproves test precision by asserting exact string equality while handling cross-platform line ending differences.
862-877: LGTM!The change from
self::assertStringContainsStringtoAssert::equalsWithoutLEimproves test precision by asserting exact string equality while handling cross-platform line ending differences.
880-936: LGTM!The new test method
testUseErrorViewLogicWithNonHtmlFormatprovides comprehensive coverage for error handling with JSON response format. It correctly verifies that:
- The status code is 500 for exceptions
- The content-type is properly set to JSON
- HTML error view content is not included in JSON responses
- The JSON response structure contains the expected message key
This test validates the enhanced error handling logic introduced in the ErrorHandler changes.
src/http/ErrorHandler.php (5)
44-51: LGTM!The
$defaultResponseConfigproperty provides sensible default configuration for fallback Response instances with proper typing and documentation.
53-59: LGTM!The
$templateResponseproperty is well-designed with proper nullable typing and clear documentation explaining its purpose for reusing configured Response instances.
149-160: LGTM!The
setResponse()method is well-designed with clear documentation and proper type hints. It enables the template response pattern for consistent error handling configuration.
178-178: LGTM!The change to use
createErrorResponse()ensures consistent response creation using the template response or default configuration.
223-223: LGTM!The change to use
createErrorResponse()ensures consistent response creation using the template response or default configuration.
Summary by CodeRabbit
New Features
Bug Fixes
Chores