[2.1] Cleanup ErrorHandler

[yiisoft]

[2.1] Cleanup ErrorHandler

[klimov-paul]

Yii may introduce its onw 'application end' callbacks list to be used instead of register_shutdown_function() and will be invoked at the application run ending. Thus saving session, writing logs and so on should be performed through it.
But in case developer uses some 3rd party library, which relies on register_shutdown_function() those callbacks will drop out of Yii error handling or logging.
Also developer will be stripped of ability to terminate script flow using exit or die operators, as thier invocations will prevent Yii ending callbacks to be executed, meaning no session will be written no logs flushed and so on.

It is a side effect that changes global state.

If you concern about about error handler remains registered after application running is complete, you can always invoke ErrorHandler::unregister() after application run:

$config = require(__DIR__ . '/../config/frontend.php');
(new yii\web\Application($config))->run();
Yii::$app->errorHandler->unregister();
//script continues

[SamMousa]

I was just looking into this, some more, what is your opinion on removing the global exception handler after application initialization?

Currently the global registration via set_exception_handler is a side effect.
While there might be cases where we need it (during init or bootstrap), as soon as we enter run():

try {
            $this->state = self::STATE_BEFORE_REQUEST;
            $this->trigger(self::EVENT_BEFORE_REQUEST);

            $this->state = self::STATE_HANDLING_REQUEST;
            $response = $this->handleRequest($this->getRequest());

            $this->state = self::STATE_AFTER_REQUEST;
            $this->trigger(self::EVENT_AFTER_REQUEST);

            $this->state = self::STATE_SENDING_RESPONSE;
            $response->send();

            $this->state = self::STATE_END;

            return $response->exitStatus;
        } catch (ExitException $e) {
            $this->end($e->statusCode, isset($response) ? $response : null);
            return $e->statusCode;
        }

We could add another catch statement from which we invoke the error handler.

[samdark]

1. I see no pros compared to handling exceptions via set_exception_handler.
2. There are still exceptions outside run() to be handled.

[SamMousa]

It is a side effect that changes global state.

[samdark]

Alright. How would you deal with exceptions outside of run() method then?

[SamMousa]

Haven't said I have it all figured out ;).

We have 4 phases

• before construction
• construct / init
• run
• after run

The first and last one are technically out of scope of the app; currently we can't handle 1 but we do handle 4.

We need to solve step 2, 4 is easy. One simple solution is to register the exception handler and unregister it at the end of the init phase.

Mostly I feel having special components is a code smell. And I'd like to gather some more opinions on this case.

[samdark]

@yiisoft/core-developers, @yiisoft/reviewers ?

[klimov-paul]

There are also 'shutdown' functions. which are run on script termination outside of the application scope.
They may trigger extra errors. Having error handler attached all the time is an insurance every error including ones causes by application destruction (e.g. at Application::__destruct() and destructor of any internal object) or at shutdown function will be at least logged.

[SamMousa]

That's a good point; destruction matters as well.

[klimov-paul]

Wrapping Application::run() into a try...catch is unlikely to suffice.
At the present state error handler is registered during Application instance creation, e.g. at Application::__construct(). It is registered before other components and other application initialization code, thus it is able to handle any error or exception causes by application internal components processing. Thus global try...catch should cover application construction as well, which demands creation of another static method for application run:

class Application extends Module
{
    public static function runApp($config)
    {
        try {
            $app = new static($config);
            $app->run();
        } catch (\Throwable $e) {
            if (isset($app) && isset($app->errorHandler)) { // error may occur before application created or error handler initialized
                $app->errorHandler->handleException($e);
            } else {
                throw $e;
            }
        }
    }
}

// file `index.php`
$config = require(__DIR__ . '/../config/frontend.php');
yii\web\Application::runApp($config);

For my taste this looks a little ugly.

[samdark]

Updated https://github.com/yiisoft/yii2/blob/master/docs/internals/design-decisions.md since, because of things mentioned by @klimov-paul, handler is unlikely to be done in another way.

[SamMousa]

Can this be moved to yiisoft/yii-core?

I would like to revisit this for 3.0.

While I understand the arguments given for the current implementation I feel we can do better.

Even if we decide that error handler needs to be attached early and stay alive after application finishes, we can improve:

• The error handler should not be a component, it doesn't make sense to have multiple error handlers; also a component
• Handling application errors by attempting to run a different route is questionable at best; we do not know the state of our application, so running actions is probably not the best approach. (yii\web\ErrorHandler::$errorAction)
• We should offer the user the option to not have side effects when using the error handler register_shutdown_function.
• The errorHandler is currently registered in Application::run() based on the value of a constant. If we are using errorHandler as a component then we should instead support disabling it via configuration.
• The error handler should be able to be used without register_shutdown_function.
• There should be a setting that unregisters the error handler after run().
• Code should not be using register_shutdown_function() but instead use Yii application events.

[samdark]

Why closing?

[SamMousa]

Hmm, was writing a post and then decided not to post it; clicked close instead of closing the page :|

Could you move this to core?

[samdark]

Done.

[samdark]

Re-checked everything and now consider it done: https://forum.yiiframework.com/t/error-handler-is-back/126667