Add ErrorController.

[ADmad]

This ensures settings done in AppController are used when generating error responses too.

I have seen numerous users on IRC/Slack struggle to figure out why the setting they have done in AppController don't affect their error pages since they don't realize that by default the core's ErrorController is used which extends Controller not AppController. Most go down the wrong path of either trying to use custom error handler or exception renderer. So perhaps the docs need improving too. Regardless having a ErrorController in app itself should save newbies some hair pulling.

[thinkingmedia]

    public function initialize()
    {
        $this->loadComponent('RequestHandler');
    }

That will be a problem for most people. Extending AppController but skipping initialization is going to trip up people, because most apps perform logic in the beforeFilter that assumes components were loaded.

I don't think this is a good practice, because the odds of a cascading failure to render the error page is more likely. The moment someone has an error in a component or the AppController the error rendering will fail.

What I recommend, is to add something to the Error controller in the core. That will introduce a warning if the developer uses custom error views. The warning would state

Warning: Do not use custom error views with the core ErrorController. Add your own class at App\Controller\ErrorController,

[markstory]

I don't think this is a good practice, because the odds of a cascading failure to render the error page is more likely. The moment someone has an error in a component or the AppController the error rendering will fail.

This is exactly the reason that the core ErrorController doesn't extend AppController. I suspect that with this change we'll have to start fielding questions about memory exhaustion, request timeouts, and cryptic fatal errors.

[ADmad]

Extending AppController but skipping initialization is going to trip up people, because most apps perform logic in the beforeFilter that assumes components were loaded.

Good point, I can add an empty beforeFilter() to avoid that. Skipping AppController's initialize() is necessary as components like Auth, Security would trip up error page generation.

I don't think this is a good practice, because the odds of a cascading failure to render the error page is more likely.

.

This is exactly the reason that the core ErrorController doesn't extend AppController. I suspect that with this change we'll have to start fielding questions about memory exhaustion, request timeouts, and cryptic fatal errors.

Yeah there is the potential for cascading failures but exception renderer can mitigate that. There are checks to use Controller instance if creating ErrorController instance fails. In 2.x CakeErrorController extends AppController and we don't seem so have too many problems with that. Seeing the ErrorController present would at least give an idea to users that they can modify it to suit their needs.

What I recommend, is to add something to the Error controller in the core. That will introduce a warning if the developer uses custom error views.

Then users would just create an "empty" ErrorController which extends Controller without skipping the initialization code again end up having problems. Since the solution is to have an ErrorController it's better we proper a most suitable one.

Adding an ErrorController in the skeleton might not be the perfect solution but I think it's still better than the current situtation. Seeing the ErrorController should at least give them an idea that they can modify it to suit their needs.

[josegonzalez]

If we add an empty beforeFilter, what settings are we trying to persist for the ErrorController?

[ADmad]

If we add an empty beforeFilter, what settings are we trying to persist for the ErrorController?

The two common issues I have seen people have with error pages are:

1. HTML error pages not getting styled similar similar rest of their site since for e.g. theme settings are lost.
2. Getting HTML instead of JSON responses for APIs.

I am hoping most do these settings in beforeRender() instead of beforeFilter().

If they are doing view/response related setting in beforeFilter() they are going to having problems.

Like I said above adding this ErrorController isn't a prefect solution but at least having it present should give users an idea when they should be doing changes. But if the consensus is otherwise this PR can be closed.

[thinkingmedia]

Inheritance might be backwards.

Instead of ErrorController > AppControler > Controller it should be AppController > ErrorController > Controller.

With, the ErrorController living in the app namespace. This way, the minimum requirements for a theme is placed in the ErrorController. AuthComponent and higher requirements are placed in AppController.

This would remove the need to overload methods just to silence AppController, and removes the risk that you missed something in beforeRender or what ever.

[inoas]

Instead of ErrorController > AppControler > Controller it should be AppController > ErrorController Controller.

What about AppController > BaseController > Controller or ActionController > BaseController > Controller

Inheriting from ErrorController sounds strange. Maybe ErrorHandlerController.

[markstory]

@ADmad I'm not against trying out this approach. We can always change our minds later if this proves to be as problematic or more later.

[inoas]

I am hoping most do these settings in beforeRender() instead of beforeFilter().

Looking at the PR now I really like it. To stop accidental behaviour, maybe afterFilter() should be empty as well? That will yield consistent and reliable behaviour, wouldn't it?

[chinpei215]

I like this change too. It would allow users to add custom HTTP status codes in their own ErrorController. As a result, I can solve cakephp/cakephp#8962 by using my original approach.