Skip to content
This repository has been archived by the owner on Jan 29, 2020. It is now read-only.

Allow opt-in to raising exceptions instead of error middleware #78

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
23 changes: 23 additions & 0 deletions doc/book/error-handlers.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,8 @@ You can typically handle these conditions via middleware itself.

## Handling 404 conditions

- Since 1.3.0

If no middleware is able to handle the incoming request, this is typically
representative of an HTTP 404 status. Stratigility provides a barebones
middleware that you may register in an innermost layer that will return a 404
Expand Down Expand Up @@ -64,6 +66,27 @@ class NotFoundMiddleware implements ServerMiddlewareInterface

## Handling PHP errors and exceptions

- Since 1.3.0

> ### Opting in to error middleware
>
> If you have upgraded from Expressive 1.0.0, you will have been using the
> `FinalHandler` implementation, and relying on the fact that, internally,
> dispatching wraps all middleware in `try/catch` blocks.
>
> Starting in 1.3.0, we provide a new way to handle errors via middleware.
>
> **To opt-in to the new system, you must call `raiseThrowables()` on your
> middleware pipeline:**
>
> ```php
> $pipeline = new MiddlewarePipe();
> $pipeline->raiseThrowables();
> ```
>
> (Starting in 2.0.0, this will no longer be necessary, but until then, this is
> how you opt-in to the system described below.)
`Zend\Stratigility\Middleware\ErrorHandler` is a middleware implementation to
register as the *outermost layer* of your application (or close to the outermost
layer). It does the following:
Expand Down
15 changes: 13 additions & 2 deletions doc/book/migration/to-v2.md
Original file line number Diff line number Diff line change
Expand Up @@ -79,8 +79,17 @@ These approaches, however, have several shortcomings:
Starting in 1.3, we are promoting using standard middleware layers as error
handlers, instead of using the existing error middleware/final handler system.

To achieve this, we have provided some new functionality, as well as augmented
existing functionality:
The first step is to opt-in to having throwables and exceptions raised by
middleware, instead of having the dispatcher catch them and then invoke
middleware. Do this via the `MiddlewarePipe::raiseThrowables()` method:

```php
$pipeline = new MiddlewarePipe();
$pipeline->raiseThrowables();
```

Once you have done that you may start using some of the new functionality, as
well as augmented existing functionality:

- [NotFoundHandler middleware](../error-handlers.md#handling-404-conditions)
- [ErrorHandler middleware](../error-handlers.md#handling-php-errors-and-exceptions)
Expand Down Expand Up @@ -133,6 +142,8 @@ request and a response, and be guaranteed to return a response instance.)

To summarize:

- Call the `raiseThrowables()` method of your `MiddlewarePipe` instance to
opt-in to the new error handling strategy.
- Use the new `Zend\Stratigility\Middleware\NotFoundHandler` as the innermost
layer of your application pipeline in order to provide 404 responses.
- Use the new `Zend\Stratigility\Middleware\ErrorHandler` middleware as the
Expand Down
36 changes: 35 additions & 1 deletion src/Dispatch.php
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@

namespace Zend\Stratigility;

use Interop\Http\Middleware\DelegateInterface;
use Interop\Http\Middleware\MiddlewareInterface as InteropMiddlewareInterface;
use Interop\Http\Middleware\ServerMiddlewareInterface;
use Throwable;
Expand All @@ -27,6 +26,14 @@
*/
class Dispatch
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Import Interop\Http\Middleware\DelegateInterface is not used. However it is mentioned as a comment in the docblock right before the process method, but I don't think that's a reason to import it.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, removed now.

{
/**
* Flag indicating whether or not to raise throwables during dispatch; when
* false, a try/catch block is used instead (default behavior).
*
* @var bool
*/
private $raiseThrowables = false;

/**
* @var ResponseInterface
*/
Expand Down Expand Up @@ -103,6 +110,17 @@ public function process(Route $route, RequestInterface $request, callable $next)
return $this->dispatchInteropMiddleware($route->handler, $next, $request);
}

/**
* Enables the "raise throwables", causing this instance to raise
* throwables instead of catch them.
*
* @return void
*/
public function raiseThrowables()
{
$this->raiseThrowables = true;
}

/**
* Set a response prototype to use when invoking callable middleware following http-interop middleware.
*
Expand Down Expand Up @@ -193,6 +211,18 @@ private function dispatchCallableMiddleware(
break;
}

if ($this->raiseThrowables) {
if ($hasError && $arity === 4) {
return $middleware($err, $request, $response, $next);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is not tested. Not sure if it's needed though.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably not, as it's the same workflow as what's in the try/catch block... and we're recommending against using error middleware starting in this version anyways.

}

if (! $hasError && $arity < 4) {
return $middleware($request, $response, $next);
}

return $next($request, $response, $err);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tests are missing for this.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Tests have now been added for both this path, and the one noted above.

}

try {
if ($hasError && $arity === 4) {
return $middleware($err, $request, $response, $next);
Expand Down Expand Up @@ -231,6 +261,10 @@ private function dispatchInteropMiddleware($middleware, callable $next, RequestI
$middleware->setResponsePrototype($this->responsePrototype);
}

if ($this->raiseThrowables) {
return $middleware->process($request, $next);
}

try {
return $middleware->process($request, $next);
} catch (Throwable $throwable) {
Expand Down
55 changes: 55 additions & 0 deletions src/Exception/MiddlewareException.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
<?php
/**
* @link http://github.com/zendframework/zend-stratigility for the canonical source repository
* @copyright Copyright (c) 2016 Zend Technologies USA Inc. (http://www.zend.com)
* @license http://framework.zend.com/license/new-bsd New BSD License
*/

namespace Zend\Stratigility\Exception;

use RuntimeException;

/**
* Exception raised when a string $err is provided and raise throwables is enabled.
*
* @todo Remove for 2.0.0.
*/
class MiddlewareException extends RuntimeException
{
/**
* Create an instance based on an error value.
*
* @param mixed $err
* @return self
*/
public static function fromErrorValue($err)
{
if (is_object($err)) {
return self::fromType(get_class($err));
}

if (is_array($err)) {
return self::fromType(gettype($err));
}

if (is_string($err)) {
throw new self($err);
}

return self::fromType(var_export($err, true));
}

/**
* Create an instance using a templated error string.
*
* @param string $value
* @return self
*/
private static function fromType($value)
{
return new self(sprintf(
'Middleware raised an error condition: %s',
$value
));
}
}
27 changes: 27 additions & 0 deletions src/MiddlewarePipe.php
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,14 @@ class MiddlewarePipe implements MiddlewareInterface, ServerMiddlewareInterface
*/
protected $pipeline;

/**
* Whether or not exceptions thrown by middleware or invocation of
* middleware using the $err argument should bubble up as exceptions.
*
* @var bool
*/
private $raiseThrowables = false;

/**
* @var Response
*/
Expand Down Expand Up @@ -73,6 +81,7 @@ public function __construct()
*
* @todo Make $out required for 2.0.0.
* @todo Remove trigger of deprecation notice when preparing for 2.0.0.
* @todo Remove raiseThrowables logic for 2.0.0.
* @param Request $request
* @param Response $response
* @param callable $out
Expand All @@ -96,6 +105,10 @@ public function __invoke(Request $request, Response $response, callable $out = n
$done = $out ?: new FinalHandler([], $response);
$next = new Next($this->pipeline, $done);
$next->setResponsePrototype($response);
if ($this->raiseThrowables) {
$next->raiseThrowables();
}

$result = $next($request, $response);

return ($result instanceof Response ? $result : $response);
Expand All @@ -119,6 +132,9 @@ public function process(Request $request, DelegateInterface $delegate)
if ($response) {
$next->setResponsePrototype($response);
}
if ($this->raiseThrowables) {
$next->raiseThrowables();
}

$result = $next->process($request);

Expand Down Expand Up @@ -179,6 +195,17 @@ public function pipe($path, $middleware = null)
return $this;
}

/**
* Enable the "raise throwables" flag.
*
* @todo Deprecate this starting in 2.0.0
* @return void
*/
public function raiseThrowables()
{
$this->raiseThrowables = true;
}

/**
* @param Response $prototype
* @return void
Expand Down
61 changes: 55 additions & 6 deletions src/Next.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
use Psr\Http\Message\ServerRequestInterface;
use RuntimeException;
use SplQueue;
use Throwable;

/**
* Iterate a queue of middlewares and execute them.
Expand All @@ -37,6 +38,15 @@ class Next implements DelegateInterface
*/
private $queue;

/**
* Flag indicating whether or not the dispatcher should raise throwables
* when encountered, and whether or not $err arguments should raise them;
* defaults false.
*
* @var bool
*/
private $raiseThrowables = false;

/**
* @var string
*/
Expand Down Expand Up @@ -105,13 +115,12 @@ public function __invoke(
ResponseInterface $response,
$err = null
) {
if ($err !== null && $this->raiseThrowables) {
$this->raiseThrowableFromError($err);
}

if (null !== $err) {
trigger_error(
'Usage of error middleware is deprecated as of 1.3.0, and will be removed in 2.0.0; '
. 'please see https://docs.zendframework.com/zend-stratigility/migration/to-v2/ '
. 'for details on how to update your application to remove this message.',
E_USER_DEPRECATED
);
$this->triggerErrorDeprecation();
}

if (! $this->responsePrototype) {
Expand Down Expand Up @@ -200,6 +209,17 @@ public function process(RequestInterface $request)
return $result;
}

/**
* Toggle the "raise throwables" flag on.
*
* @return void
*/
public function raiseThrowables()
{
$this->raiseThrowables = true;
$this->dispatch->raiseThrowables();
}

/**
* @param ResponseInterface $prototype
* @return void
Expand Down Expand Up @@ -384,4 +404,33 @@ private function dispatchNextDelegate(
$this->validateServerRequest($request);
return $nextDelegate($request, $response, $err);
}

/**
* @param mixed $err
* @throws Throwable|\Exception
*/
private function raiseThrowableFromError($err)
{
if ($err instanceof Throwable
|| $err instanceof \Exception
) {
throw $err;
}

$this->triggerErrorDeprecation();
throw Exception\MiddlewareException::fromErrorValue($err);
}

/**
* @todo Remove for 2.0.0
*/
private function triggerErrorDeprecation()
{
trigger_error(
'Usage of error middleware is deprecated as of 1.3.0, and will be removed in 2.0.0; '
. 'please see https://docs.zendframework.com/zend-stratigility/migration/to-v2/ '
. 'for details on how to update your application to remove this message.',
E_USER_DEPRECATED
);
}
}
Loading