From b5d34b2adb3703db2bb1a5c5c6e47f859449a2d8 Mon Sep 17 00:00:00 2001 From: Matthew Weier O'Phinney Date: Wed, 7 Dec 2016 12:04:36 -0600 Subject: [PATCH 1/3] Make PHP error handler operate in a stack Per #400, when we register a new error handler to swallow deprecation notices in `Application::__invoke()`, if any global error handlers were registered previously, they now disappear. This patch adds a test covering that scenario, and updates the `Application` class as follows: - The `set_error_handler()` call in `Application::__invoke()` has been replaced with a call to a new method, `swallowDeprecationNotices()`. - `swallowDeprecationNotices()` now captures the return value of calling `set_error_handler()`; if that value is non-empty, it creates a new error handler closing over its own error handler and the previous which invokes the previous if our own returns false. --- src/Application.php | 33 ++++++++++++++++++++++++++++++--- test/IntegrationTest.php | 39 +++++++++++++++++++++++++++++++++++++++ 2 files changed, 69 insertions(+), 3 deletions(-) diff --git a/src/Application.php b/src/Application.php index 1af00bc6..6e4590da 100644 --- a/src/Application.php +++ b/src/Application.php @@ -139,9 +139,7 @@ public function __construct( */ public function __invoke(ServerRequestInterface $request, ResponseInterface $response, callable $out = null) { - set_error_handler(function ($errno, $errstr) { - return false !== strstr($errstr, 'error middleware is deprecated'); - }, E_USER_DEPRECATED); + $this->swallowDeprecationNotices(); if (! $out && (null === ($out = $this->getFinalHandler($response)))) { $response = $response instanceof StratigilityResponse @@ -671,4 +669,33 @@ private function checkForDuplicateRoute($path, $methods = null) ); } } + + /** + * Register an error handler to swallow deprecation notices due to error middleware usage. + * + * @return void + */ + private function swallowDeprecationNotices() + { + $handler = function ($errno, $errstr) { + if ($errno !== E_USER_DEPRECATED) { + return false; + } + return false !== strstr($errstr, 'error middleware is deprecated'); + }; + + $previous = set_error_handler($handler); + if (! $previous) { + return; + } + + $composite = function ($errno, $errstr, $errfile, $errline, $errcontext) use ($handler, $previous) { + if ($handler($errno, $errstr)) { + return true; + } + + return $previous($errno, $errstr, $errfile, $errline, $errcontext); + }; + set_error_handler($composite); + } } diff --git a/test/IntegrationTest.php b/test/IntegrationTest.php index 0a5004df..35d49b65 100644 --- a/test/IntegrationTest.php +++ b/test/IntegrationTest.php @@ -21,11 +21,21 @@ class IntegrationTest extends TestCase { + public $errorHandler; public $response; public function setUp() { $this->response = null; + $this->errorHandler = null; + } + + public function tearDown() + { + if ($this->errorHandler) { + set_error_handler($this->errorHandler); + $this->errorHandler = null; + } } public function getEmitter() @@ -67,4 +77,33 @@ public function testInjectedFinalHandlerCanEmitA404WhenNoMiddlewareMatches() $this->assertInstanceOf(ResponseInterface::class, $this->response); $this->assertEquals(404, $this->response->getStatusCode()); } + + /** + * @todo Remove for version 2.0, as that version will remove error middleware support + * @group 400 + */ + public function testErrorMiddlewareDeprecationErrorHandlerWillNotOverridePreviouslyRegisteredErrorHandler() + { + $triggered = false; + $this->errorHandler = set_error_handler(function ($errno, $errstr) use (&$triggered) { + $triggered = true; + return true; + }); + + $expected = new Response(); + $middleware = function ($request, $response, $next) use ($expected) { + trigger_error('Triggered', E_USER_NOTICE); + return $expected; + }; + + $app = new Application(new FastRouteRouter(), null, null, $this->getEmitter()); + $app->pipe($middleware); + + $request = new ServerRequest([], [], 'https://example.com/foo', 'GET'); + $response = clone $expected; + + $app->run($request, $response); + + $this->assertTrue($triggered); + } } From 4578f4be36dcbd5966b4b6e1b1826922f8e23c5e Mon Sep 17 00:00:00 2001 From: Matthew Weier O'Phinney Date: Wed, 7 Dec 2016 12:15:23 -0600 Subject: [PATCH 2/3] Minor cleanup of code and `@todo` annotations - added `@todo` annotation to `swallowDeprecationNotices()` to ensure we remove it for 2.0.0. - updated `@todo` annotation for `__invoke()` to reference `swallowDeprecationNotices()` invocation - tightened up logic in `swallowDeprecationNotices()` to remove temporary variables. --- src/Application.php | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/Application.php b/src/Application.php index 6e4590da..86a837b1 100644 --- a/src/Application.php +++ b/src/Application.php @@ -129,9 +129,8 @@ public function __construct( * * If $out is not provided, uses the result of `getFinalHandler()`. * - * @todo Remove logic for creating final handler for version 2.0. - * @todo Remove error handler for deprecation notice due to triggering - * error middleware for version 2.0.0. + * @todo Remove logic for creating final handler for version 2.0.0. + * @todo Remove swallowDeprecationNotices() invocation for version 2.0.0. * @param ServerRequestInterface $request * @param ResponseInterface $response * @param callable|null $out @@ -673,6 +672,7 @@ private function checkForDuplicateRoute($path, $methods = null) /** * Register an error handler to swallow deprecation notices due to error middleware usage. * + * @todo Remove method for version 2.0.0. * @return void */ private function swallowDeprecationNotices() @@ -689,13 +689,12 @@ private function swallowDeprecationNotices() return; } - $composite = function ($errno, $errstr, $errfile, $errline, $errcontext) use ($handler, $previous) { + set_error_handler(function ($errno, $errstr, $errfile, $errline, $errcontext) use ($handler, $previous) { if ($handler($errno, $errstr)) { return true; } return $previous($errno, $errstr, $errfile, $errline, $errcontext); - }; - set_error_handler($composite); + }); } } From ee14fe3eaeb413e907fa3adfc7ed7add3a17e114 Mon Sep 17 00:00:00 2001 From: Matthew Weier O'Phinney Date: Wed, 7 Dec 2016 12:28:18 -0600 Subject: [PATCH 3/3] Added CHANGELOG for #402 --- CHANGELOG.md | 22 ++++++++++++++++++++++ 1 file changed, 22 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 21a4ba28..989e5364 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,28 @@ All notable changes to this project will be documented in this file, in reverse chronological order by release. +## 1.0.4 - 2016-12-07 + +### Added + +- Nothing. + +### Deprecated + +- Nothing. + +### Removed + +- Nothing. + +### Fixed + +- [#402](https://github.com/zendframework/zend-expressive/pull/402) fixes how + `Application::__invoke()` registers the error handler designed to swallow + deprecation notices, as introduced in 1.0.3. It now checks to see if another + error handler was previously registered, and, if so, creates a composite + handler that will delegate to the previous for all other errors. + ## 1.0.3 - 2016-11-11 ### Added