Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ViewException does not provide dependencies to the wrapped report method #36108

Closed
imjoehaines opened this issue Feb 1, 2021 · 3 comments · Fixed by #36110
Closed

ViewException does not provide dependencies to the wrapped report method #36108

imjoehaines opened this issue Feb 1, 2021 · 3 comments · Fixed by #36110
Labels

Comments

@imjoehaines
Copy link

  • Laravel Version: 6.20.15 / 8.25.0 / 9.x (8076d86)
  • PHP Version: N/A
  • Database Driver & Version: N/A

Description:

The ViewException class introduced in #36032 does not pass dependencies to the wrapped exception's report method. This causes a fatal error when an exception thrown in a view has a dependency

See the docs:

You may type-hint any required dependencies of the report method and they will automatically be injected into the method by Laravel's service container.
https://laravel.com/docs/8.x/errors#renderable-exceptions

Steps To Reproduce:

  1. Create a new Laravel project
  2. Remove Ignition & Collision (composer remove facade/ignition nunomaduro/collision) [6.x & 8.x only]
  3. Create a custom exception class with a report method that requires dependencies, e.g.:
    namespace App\Exceptions;
    
    class MyException extends \Exception
    {
        public function report(\Illuminate\Contracts\Auth\Guard $guard)
        {
            return false;
        }
    }
  4. Throw an instance of this exception in a view
  5. An ArgumentCountError is thrown:

    Too few arguments to function App\Exceptions\MyException::report()

@driesvints
Copy link
Member

Urgh, I didn't take that into account. I'll get that fixed 👍

@driesvints driesvints added the bug label Feb 1, 2021
@driesvints
Copy link
Member

@imjoehaines can you try modifying the ViewException class to the following below and see if that works?

<?php

namespace Illuminate\View;

use ErrorException;
use Illuminate\Container\Container;
use Illuminate\Support\Reflector;

class ViewException extends ErrorException
{
    /**
     * Report the exception.
     *
     * @return void
     */
    public function report()
    {
        $exception = $this->getPrevious();

        if (Reflector::isCallable($reportCallable = [$exception, 'report'])) {
            Container::getInstance()->call($reportCallable);
        }
    }

    /**
     * Render the exception into an HTTP response.
     *
     * @param  \Illuminate\Http\Request  $request
     * @return \Illuminate\Http\Response
     */
    public function render($request)
    {
        $exception = $this->getPrevious();

        if ($exception && method_exists($exception, 'render')) {
            return $exception->render($request);
        }
    }
}

@imjoehaines
Copy link
Author

@imjoehaines can you try modifying the ViewException class to the following below and see if that works?

Yeah, that works 🙂

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants