From 23d1cbf51d0451e39621dee1a8485870981716cf Mon Sep 17 00:00:00 2001 From: Wilmer Arambula Date: Tue, 19 Aug 2025 17:45:18 -0400 Subject: [PATCH 1/3] refactor: Rename `templateResponse` to `response` in `ErrorHandler` and update related methods for clarity. --- src/http/ErrorHandler.php | 18 +++++----- tests/http/ErrorHandlerTest.php | 62 ++++++++++++++++++++++++++------- 2 files changed, 59 insertions(+), 21 deletions(-) diff --git a/src/http/ErrorHandler.php b/src/http/ErrorHandler.php index 6fc407b5..39f08f60 100644 --- a/src/http/ErrorHandler.php +++ b/src/http/ErrorHandler.php @@ -48,19 +48,19 @@ final class ErrorHandler extends \yii\web\ErrorHandler ]; /** - * Template Response instance for error handling. + * Response instance for error handling. * * When set, this Response instance will be used as a base for error responses, preserving configured format, * formatters, and other settings. */ - private Response|null $templateResponse = null; + private Response|null $response = null; /** * Clears all output buffers above the minimum required level. * * Iterates through all active output buffers and cleans them, ensuring that only the minimum buffer level remains. * - * This method is used to discard any existing output before rendering an error response, maintaining a clean output + * This method is used to discard any existing output before rendering an error Response, maintaining a clean output * state while preserving compatibility with the testing framework. * * **PHPUnit Compatibility.** @@ -134,16 +134,16 @@ public function handleException($exception): Response } /** - * Sets the template Response for error handling. + * Sets the Response for error handling. * - * The provided Response will be used as a template for error responses, preserving configuration such as format, - * charset, and formatters. The Response will be cleared of any existing data before use. + * The provided Response will be used for error responses, preserving configuration such as format, charset, and + * formatters. The Response will be cleared of any existing data before use. * * @param Response $response Template response with desired configuration. */ public function setResponse(Response $response): void { - $this->templateResponse = $response; + $this->response = $response; } /** @@ -249,13 +249,13 @@ protected function renderException($exception): Response /** * Creates a Response instance for error handling. * - * Uses the template Response if available, otherwise creates a new instance with default configuration. + * Uses the Response if available, otherwise creates a new instance with default configuration. * * @return Response Clean Response instance ready for error content. */ private function createErrorResponse(): Response { - $response = $this->templateResponse ?? new Response($this->defaultResponseConfig); + $response = $this->response ?? new Response($this->defaultResponseConfig); $response->clear(); diff --git a/tests/http/ErrorHandlerTest.php b/tests/http/ErrorHandlerTest.php index 9e4f3f65..892bac8d 100644 --- a/tests/http/ErrorHandlerTest.php +++ b/tests/http/ErrorHandlerTest.php @@ -60,6 +60,44 @@ public function testClearOutputCleansAllBuffersInNonTestEnvironment(): void } } + public function testCreateErrorResponseClearsExistingResponseData(): void + { + $errorHandler = new ErrorHandler(); + + $errorHandler->discardExistingOutput = false; + + $response = new Response(); + $response->data = 'Pre-existing data that should be cleared'; + + $response->setStatusCode(201); + $response->getHeaders()->set('X-Another-Header', 'another-value'); + $response->getHeaders()->set('X-Custom-Header', 'custom-value'); + + $errorHandler->setResponse($response); + + $response = $errorHandler->handleException(new Exception('Test exception for response clearing')); + + self::assertSame( + 500, + $response->getStatusCode(), + "Should set status code to '500' after clearing existing response.", + ); + self::assertNotSame( + 'Pre-existing data that should be cleared', + $response->data, + "Response data should be 'Pre-existing data that should be cleared' when reusing existing response for " . + 'error handling.', + ); + self::assertFalse( + $response->getHeaders()->has('X-Another-Header'), + 'All custom headers should be cleared when reusing existing response for error handling.', + ); + self::assertFalse( + $response->getHeaders()->has('X-Custom-Header'), + 'Custom headers should be cleared when reusing existing response for error handling.', + ); + } + public function testHandleExceptionCallsClearOutputWhenEnabled(): void { $errorHandler = new ErrorHandler(); @@ -170,7 +208,7 @@ public function testHandleExceptionWithComplexMessage(): void ); self::assertIsString( $response->data, - 'Should set response data as string for complex exception.', + 'Should set Response data as string for complex exception.', ); } @@ -191,7 +229,7 @@ public function testHandleExceptionWithEmptyMessage(): void ); self::assertNotNull( $response->data, - "Should set response data even for 'Exception' with empty message.", + "Should set Response data even for 'Exception' with empty message.", ); } @@ -212,7 +250,7 @@ public function testHandleExceptionWithGenericException(): void ); self::assertNotEmpty( $response->data, - 'Should set response data with exception information.', + 'Should set Response data with exception information.', ); } @@ -235,7 +273,7 @@ public function testHandleExceptionWithLongMessage(): void ); self::assertNotEmpty( $response->data, - "Should set response data for 'Exception' with long message.", + "Should set Response data for 'Exception' with long message.", ); } @@ -270,7 +308,7 @@ public function testHandleExceptionWithMultipleDifferentExceptions(): void } self::assertNotEmpty( $response->data, - "Should set response data for exceptions {$index}.", + "Should set Response data for exceptions {$index}.", ); } } @@ -293,7 +331,7 @@ public function testHandleExceptionWithNestedExceptions(): void ); self::assertNotEmpty( $response->data, - 'Should set response data for nested exceptions.', + 'Should set Response data for nested exceptions.', ); } @@ -314,7 +352,7 @@ public function testHandleExceptionWithRuntimeException(): void ); self::assertNotEmpty( $response->data, - 'Should set response data for runtime exception.', + 'Should set Response data for runtime exception.', ); } @@ -336,7 +374,7 @@ public function testHandleExceptionWithSpecialCharactersInTrace(): void ); self::assertIsString( $response->data, - "Should set response data as string for 'Exception' with special trace.", + "Should set Response data as string for 'Exception' with special trace.", ); } } @@ -358,7 +396,7 @@ public function testHandleExceptionWithUserException(): void ); self::assertNotEmpty( $response->data, - 'Should set response data for user exception.', + 'Should set Response data for user exception.', ); } @@ -379,7 +417,7 @@ public function testHandleExceptionWithZeroCode(): void ); self::assertNotEmpty( $response->data, - "Should set response data for 'Exception' with zero code.", + "Should set Response data for 'Exception' with zero code.", ); } @@ -395,11 +433,11 @@ public function testResponseDataIsNotEmpty(): void self::assertNotEmpty( $response->data, - 'Should always set non-empty response data.', + 'Should always set non-empty Response data.', ); self::assertIsString( $response->data, - "'Response' data should be string.", + 'Response data should be string.', ); } From c17c01db22805f91000da1094b49fdefacb34aeb Mon Sep 17 00:00:00 2001 From: Wilmer Arambula Date: Tue, 19 Aug 2025 17:50:36 -0400 Subject: [PATCH 2/3] test: Update `Response` instantiation in `ErrorHandlerTest` to include charset configuration. --- tests/http/ErrorHandlerTest.php | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/tests/http/ErrorHandlerTest.php b/tests/http/ErrorHandlerTest.php index 892bac8d..3884635f 100644 --- a/tests/http/ErrorHandlerTest.php +++ b/tests/http/ErrorHandlerTest.php @@ -66,7 +66,8 @@ public function testCreateErrorResponseClearsExistingResponseData(): void $errorHandler->discardExistingOutput = false; - $response = new Response(); + $response = new Response(['charset' => 'UTF-8']); + $response->data = 'Pre-existing data that should be cleared'; $response->setStatusCode(201); From a868b3ded612a48a40e5538b1a1b38bc4bc7511d Mon Sep 17 00:00:00 2001 From: Wilmer Arambula Date: Tue, 19 Aug 2025 17:59:00 -0400 Subject: [PATCH 3/3] Apply fixed review coderabbitai nitpick comments. --- src/http/ErrorHandler.php | 2 +- tests/http/ErrorHandlerTest.php | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/http/ErrorHandler.php b/src/http/ErrorHandler.php index 39f08f60..a85f5518 100644 --- a/src/http/ErrorHandler.php +++ b/src/http/ErrorHandler.php @@ -139,7 +139,7 @@ public function handleException($exception): Response * The provided Response will be used for error responses, preserving configuration such as format, charset, and * formatters. The Response will be cleared of any existing data before use. * - * @param Response $response Template response with desired configuration. + * @param Response $response Response instance with desired configuration. */ public function setResponse(Response $response): void { diff --git a/tests/http/ErrorHandlerTest.php b/tests/http/ErrorHandlerTest.php index 3884635f..f0aea969 100644 --- a/tests/http/ErrorHandlerTest.php +++ b/tests/http/ErrorHandlerTest.php @@ -86,8 +86,8 @@ public function testCreateErrorResponseClearsExistingResponseData(): void self::assertNotSame( 'Pre-existing data that should be cleared', $response->data, - "Response data should be 'Pre-existing data that should be cleared' when reusing existing response for " . - 'error handling.', + "Response data should not be 'Pre-existing data that should be cleared' when reusing existing response " . + 'for error handling.', ); self::assertFalse( $response->getHeaders()->has('X-Another-Header'),