[RTM] Rework the exception listeners #214
Conversation
| ]; | ||
|
|
||
| /** | ||
| * Adds the referer ID to the request. |
There was a problem hiding this comment.
Maps known exceptions to HTTP exceptions?
There was a problem hiding this comment.
Not sure if my "?" was not clear enough: the method description is wrong ;-)
There was a problem hiding this comment.
I know and I suggested to use "Maps known exceptions to HTTP exceptions" instead. You like?
| tags: | ||
| - { name: kernel.event_listener, event: kernel.exception, method: onKernelException } | ||
| - { name: kernel.event_listener, event: kernel.exception, method: onKernelException, priority: 64 } |
There was a problem hiding this comment.
Why do we need to set priorities at all?
There was a problem hiding this comment.
Because it is vital that the exception converter listener comes before the pretty error screen listener?
There was a problem hiding this comment.
I don't think so, as we are running the same chain again (and not pass our converted exception to the next listener).
| @@ -325,12 +325,12 @@ private function validateInstallation(Request $request = null) | |||
|
|
|||
| // Show the "insecure document root" message | |||
| if (!in_array($request->getClientIp(), ['127.0.0.1', 'fe80::1', '::1']) && '/web' === substr($request->getBasePath(), -4)) { | |||
| throw new InsecureInstallationHttpException(null, 'Your installation is not secure. Please set the document root to the <code>/web</code> subfolder.'); | |||
| throw new InsecureInstallationException('Your installation is not secure. Please set the document root to the <code>/web</code> subfolder.'); | |||
There was a problem hiding this comment.
We now also should remove the <code> tag from the message as it is not an http exception anymore.
| { | ||
| $this->response = $response; | ||
|
|
||
| parent::__construct($response->getContent(), 0, $previous); |
There was a problem hiding this comment.
This is bad as the content of the response may be changed but the message not.
leofeyer
force-pushed
the
feature/exception-listener
branch
from
cbf52b9
to
01812bf
Compare
leofeyer
changed the title
|
I have updated the status to RTM. |
This is the PR regarding our exception listener discussion yesterday. There are now three exception listeners:
ResponseExceptionListener: takes care of all response exceptions.ExceptionConverterListener: converts runtime exceptions into HTTP exceptions.PrettyErrorScreenListener: converts HTTP exceptions into pretty error screens.