Skip to content

Conversation

@seriquynh
Copy link
Contributor

What is the problem?

For years, a bunch of features such as render check and Responsable check have been added to the ExcepionHandler render method. The latest thing is render callbacks registration. Because of them, the ExcepionHandler render method becomes giant and messy. I think it's time to refactor this method, so it will be easier to read and maintain.

What do I do?

In this MR, I've done three things:

  1. Remove a few useless else statements.
  2. Replace a few checks by match expressions.
  3. Separate logic into smaller methods.

How is it improved?

The ExcepionHandler render method now has a clear logic in this order:

  1. Return if the given exception defines render method that returns a not null result.
  2. Return if the given exception implements Responsable interface.
  3. Convert the given exception into our expected exception before continue the rest logic.
  4. Return if developers handle the given exception inside their render callbacks.
  5. Handle other cases such as authentication and validation or return the default response if any.

This is a quick overview of the final method:

class Handler
{
    public function render($request, Throwable $e)
    {
        if (method_exists($e, 'render') && $response = $e->render($request)) {
            return Router::toResponse($request, $response);
        }

        if ($e instanceof Responsable) {
            return $e->toResponse($request);
        }

        $e = $this->prepareException($this->mapException($e));

        if ($response = $this->renderViaCallbacks($request, $e)) {
            return $response;
        }

        return match(true) {
            $e instanceof HttpResponseException => $e->getResponse(),
            $e instanceof AuthenticationException => $this->unauthenticated($request, $e),
            $e instanceof ValidationException => $this->convertValidationExceptionToResponse($e, $request),
            default => $this->renderExceptionResponse($request, $e),
        };
    }
}

@seriquynh seriquynh changed the title Refactor handler render [9.x] Refactor ExceptionHandler render method Dec 8, 2021
@seriquynh
Copy link
Contributor Author

1) Illuminate\Tests\Support\SupportHelpersTest::testRetryWithPassingSleepCallback
Failed asserting that 0.3255300521850586 matches expected 0.3.

D:\a\framework\framework\tests\Support\SupportHelpersTest.php:586

This failure isn't related to this MR. There is a problem with the retry function and windows platform. I test it on my windows laptop. It often passes but seldom fails. For example, run this test will result 9 passes and 1 failure.

restry_test

@taylorotwell taylorotwell merged commit 1873d2a into laravel:master Dec 8, 2021
@taylorotwell
Copy link
Member

Thanks for contributing to Laravel! ❤️

@seriquynh seriquynh deleted the refactor-handler-render branch December 8, 2021 13:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants