refactor(application): Improve response handling and reset request state in StatelessApplication.
#89
Conversation
…ate in `StatelessApplication`.
Walkthroughhandle() now captures terminate(...) result in a local variable and returns once; reset() now calls request->reset() after syncing the error handler response; terminate() no longer resets the request. No public signatures changed. (44 words) Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant StatelessApplication
participant ErrorHandler
participant Response
Client->>StatelessApplication: handle(request)
activate StatelessApplication
alt success
StatelessApplication->>StatelessApplication: process request
StatelessApplication->>Response: build response
else error
StatelessApplication->>ErrorHandler: handle throwable
ErrorHandler-->>StatelessApplication: error response
end
StatelessApplication->>StatelessApplication: terminate(response)
StatelessApplication-->>Client: return response
deactivate StatelessApplication
Note over StatelessApplication: On next request reset():<br/>- errorHandler->setResponse(response)<br/>- request->reset()
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (3)📓 Common learnings📚 Learning: 2025-08-03T16:24:09.241ZApplied to files:
📚 Learning: 2025-08-03T16:24:09.241ZApplied to files:
🧬 Code Graph Analysis (1)src/http/StatelessApplication.php (1)
⏰ 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). (8)
🔇 Additional comments (5)
✨ 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. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #89 +/- ##
===========================================
Coverage 100.00% 100.00%
Complexity 319 319
===========================================
Files 12 12
Lines 801 802 +1
===========================================
+ Hits 801 802 +1 ☔ View full report in Codecov by Sentry. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (4)
src/http/StatelessApplication.php (4)
278-284: Avoid type-hopping on $response (Response → ResponseInterface)$response is a Yii Response until Line 278, then becomes a ResponseInterface. This implicit type change can confuse readers and static analyzers. Prefer distinct variables.
Example refactor keeping the single-exit pattern:
try { $this->reset($request); $this->state = self::STATE_BEFORE_REQUEST; $this->trigger(self::EVENT_BEFORE_REQUEST); $this->state = self::STATE_HANDLING_REQUEST; /** @phpstan-var Response $yiiResponse */ $yiiResponse = $this->handleRequest($this->request); $this->state = self::STATE_AFTER_REQUEST; $this->trigger(self::EVENT_AFTER_REQUEST); $this->state = self::STATE_END; $psrResponse = $this->terminate($yiiResponse); } catch (Throwable $e) { $psrResponse = $this->terminate($this->handleError($e)); } return $psrResponse;
279-281: Verify catch-path assumptions: errorHandler availability before initialization failuresIf reset() throws before parent::__construct finishes (e.g., bad config), $this->errorHandler may not be initialized. Then handleError($e) could throw and skip termination/cleanup.
Please verify that, in all environments, errorHandler is always available in this catch branch. If not guaranteed, consider a guarded fallback (minimal 500 Response) in case handleError() itself fails.
427-433: Docblock is outdated: terminate() no longer resets the requestThe comment still claims terminate() “resets the request state,” but that responsibility moved to reset().
Apply this doc-only diff:
- * Cleans up registered events, resets uploaded files, flushes the logger, and resets the request state. + * Cleans up registered events, resets uploaded files, and flushes the logger. + * Note: Request state is reset in reset(), not here.
374-386: Clarify that reset() now resets the Request stateConsider explicitly mentioning the request reset in the method doc, since that responsibility moved here.
Suggested doc tweak:
- * Performs a full reinitialization of the application state, including event tracking, error handler cleanup, - * session management, and PSR-7 request injection. + * Performs a full reinitialization of the application state, including event tracking, error handler cleanup, + * request adapter reset, session management, and PSR-7 request injection.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/http/StatelessApplication.php(2 hunks)
🧰 Additional context used
🧠 Learnings (3)
📓 Common learnings
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#53
File: src/http/ErrorHandler.php:258-272
Timestamp: 2025-08-03T16:24:09.241Z
Learning: In yii2-extensions/psr-bridge, the StatelessApplication creates a new Response instance for each request in the reset() method, then passes it to ErrorHandler::setResponse(). This means the template response is not shared across requests, so calling clear() on it in createErrorResponse() is safe and doesn't cause side effects.
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#53
File: src/http/ErrorHandler.php:258-272
Timestamp: 2025-08-03T16:24:09.241Z
Learning: In yii2-extensions/psr-bridge, the StatelessApplication creates a new Response instance for each request in the reset() method, then passes it to ErrorHandler::setResponse(). This means the template response is not shared across requests, so calling clear() on it in createErrorResponse() is safe and doesn't cause side effects.
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#53
File: src/http/ErrorHandler.php:258-272
Timestamp: 2025-08-03T16:24:09.241Z
Learning: In yii2-extensions/psr-bridge, the StatelessApplication creates a new Response instance for each request in the reset() method (line 408: `$this->response = new Response($this->components['response'] ?? []);`), then passes it to ErrorHandler::setResponse(). This means the template response is not shared across requests, so calling clear() on it in createErrorResponse() is safe and doesn't cause side effects.
📚 Learning: 2025-08-03T16:24:09.241Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#53
File: src/http/ErrorHandler.php:258-272
Timestamp: 2025-08-03T16:24:09.241Z
Learning: In yii2-extensions/psr-bridge, the StatelessApplication creates a new Response instance for each request in the reset() method, then passes it to ErrorHandler::setResponse(). This means the template response is not shared across requests, so calling clear() on it in createErrorResponse() is safe and doesn't cause side effects.
Applied to files:
src/http/StatelessApplication.php
📚 Learning: 2025-08-03T16:24:09.241Z
Learnt from: terabytesoftw
PR: yii2-extensions/psr-bridge#53
File: src/http/ErrorHandler.php:258-272
Timestamp: 2025-08-03T16:24:09.241Z
Learning: In yii2-extensions/psr-bridge, the StatelessApplication creates a new Response instance for each request in the reset() method (line 408: `$this->response = new Response($this->components['response'] ?? []);`), then passes it to ErrorHandler::setResponse(). This means the template response is not shared across requests, so calling clear() on it in createErrorResponse() is safe and doesn't cause side effects.
Applied to files:
src/http/StatelessApplication.php
🧬 Code Graph Analysis (1)
src/http/StatelessApplication.php (1)
src/http/Request.php (1)
reset(748-751)
⏰ 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). (3)
- GitHub Check: phpunit / PHP 8.2-windows-latest
- GitHub Check: mutation / PHP 8.3-ubuntu-latest
- GitHub Check: mutation / PHP 8.3-ubuntu-latest
🔇 Additional comments (2)
src/http/StatelessApplication.php (2)
278-284: Single exit and consistent termination — good changeUsing a single return and ensuring terminate() runs in both success and error paths improves readability and guarantees cleanup. Looks good.
411-411: Resetting Request here is correct and aligns with stateless reuseCalling $this->request->reset() before setPsr7Request() prevents stale adapters leaking across requests. This matches prior learnings about per-request state and is the right place to do it.
Summary by CodeRabbit
Bug Fixes
Refactor
Documentation