Uh oh!
There was an error while loading. Please reload this page.
Allow for custom exception handlers. - #5675
Conversation
MGatner
commented
Feb 11, 2022
This is more then I can handle on mobile, but looks really good! On my "to do" list. |
iRedds
left a comment
There was a problem hiding this comment.
It seems to me that this PR is BC.
Services::exceptions() returns an instance of the CodeIgniter\Debug\Exceptions class, which can be extended.
Changing this class will cause breaking changes for the custom implementation.
| * if ($exception instanceOf PageNotFoundException) { | ||
| * return new \App\Libraries\MyExceptionHandler(); | ||
| * } |
There was a problem hiding this comment.
This PR implies handling of unhandled exceptions, but PageNotFoundException is handled separately.
Therefore, the mention of PageNotFoundException can be misleading.
| if ($exception instanceOf PageNotFoundException) { | ||
| return new \App\Libraries\MyExceptionHandler(); | ||
| } |
There was a problem hiding this comment.
This PR implies handling of unhandled exceptions, but PageNotFoundException is handled separately.
Therefore, the mention of PageNotFoundException can be misleading.
There was a problem hiding this comment.
PageNotFound is still handled in the ExceptionHandler, so if someone wants to override the handling on that one they still can, unless it's been overridden at the routing level, of course.
| $config = config('Exceptions'); | ||
| if (strpos($this->request->getHeaderLine('accept'), 'text/html') === false) { | ||
| $this->respond(ENVIRONMENT === 'development' ? $this->collectVars($exception, $statusCode) : '', $statusCode)->send(); | ||
| exit($exitCode); | ||
| } | ||
| if (! method_exists($config, 'handler')) { | ||
| exit('Config\Exception must have a handler() method.'); | ||
| } | ||
| $this->render($exception, $statusCode); | ||
| $handler = config('Exceptions')->handler($statusCode, $exception); |
There was a problem hiding this comment.
config('Exceptions') is called twice even though we already have the $config variable
There was a problem hiding this comment.
Good catch. Thanks. I forgot to go back and clean that up. Will do.
Uh oh!
There was an error while loading. Please reload this page.
Co-authored-by: Abdul Malik Ikhsan <samsonasik@gmail.com>
lonnieezell
commented
Feb 11, 2022
Yeah, you're correct. I have been sick the last couple of days but needed to do something other than sleep for a few hours and wasn't up to tackling the refactor of Tasks needed to get it integrated into the framework so I figured I could knock this out. Obviously wasn't thinking 100%. We can shelve it or start a 5.0 branch so it doesn't just clog up the PR lists, or something. |
| { | ||
| if (! is_cli()) { | ||
| $this->response->setStatusCode($this->statusCode); | ||
| header(sprintf('HTTP/%s %s %s', $this->request->getProtocolVersion(), $this->response->getStatusCode(), $this->response->getReasonPhrase()), true, $this->statusCode); |
There was a problem hiding this comment.
If headers have been sent, calling header() causes error.
| @@ -107,20 +85,25 @@ public function exceptionHandler(Throwable $exception) | |||
| ]); | |||
gphg
commented
Feb 27, 2023
Any update on this PR? |
I have updated this PR for 4.4, and it is waiting for reviews. |
kenjis
commented
Apr 9, 2023
Closed by #7087 |
Allows developers to easily create and integrate their own Exception handler classes, and limiting their usage
to only the exception or HTTP status code desired.
Supercedes #5355
Checklist: