using html format throws error on dump() directives not inside a controller

[mattlibera]

At first I thought this was an overall problem with the HTML output, but after digging a bit discovered that this occurs only if running dump() directives when not inside a Controller (e.g. from within a closure inside of your routes/web.php file).

Sample case I used was the one from the README file. Running php artisan dump-server worked fine, but when running php artisan dump-server --format=html the result is a FatalThrowableError:

   Symfony\Component\Debug\Exception\FatalThrowableError  : Argument 1 passed to Symfony\Component\VarDumper\Dumper\HtmlDumper::dump() must be an instance of Symfony\Component\VarDumper\Cloner\Data, null given, called in /var/www/html/test-app/vendor/symfony/var-dumper/Command/Descriptor/HtmlDescriptor.php on line 47

  at /var/www/html/test-app/vendor/symfony/var-dumper/Dumper/HtmlDumper.php:111
    107| 
    108|     /**
    109|      * {@inheritdoc}
    110|      */
  > 111|     public function dump(Data $data, $output = null, array $extraDisplayOptions = array())
    112|     {
    113|         $this->extraDisplayOptions = $extraDisplayOptions;
    114|         $result = parent::dump($data, $output);
    115|         $this->dumpId = 'sf-dump-'.mt_rand();

  Exception trace:

  1   Symfony\Component\VarDumper\Dumper\HtmlDumper::dump([])
      /var/www/html/test-app/vendor/symfony/var-dumper/Command/Descriptor/HtmlDescriptor.php:47

  2   Symfony\Component\VarDumper\Command\Descriptor\HtmlDescriptor::describe(Object(Symfony\Component\Console\Style\SymfonyStyle), Object(Symfony\Component\VarDumper\Cloner\Data), [])
      /var/www/html/test-app/vendor/beyondcode/laravel-dump-server/src/DumpServerCommand.php:70

  3   BeyondCode\DumpServer\DumpServerCommand::BeyondCode\DumpServer\{closure}(Object(Symfony\Component\VarDumper\Cloner\Data), [])
      /var/www/html/test-app/vendor/symfony/var-dumper/Server/DumpServer.php:76

  4   Symfony\Component\VarDumper\Server\DumpServer::listen(Object(Closure))
      /var/www/html/test-app/vendor/beyondcode/laravel-dump-server/src/DumpServerCommand.php:71

  5   BeyondCode\DumpServer\DumpServerCommand::handle()
      /var/www/html/test-app/vendor/laravel/framework/src/Illuminate/Container/BoundMethod.php:29

  6   call_user_func_array([])
      /var/www/html/test-app/vendor/laravel/framework/src/Illuminate/Container/BoundMethod.php:29

  7   Illuminate\Container\BoundMethod::Illuminate\Container\{closure}()
      /var/www/html/test-app/vendor/laravel/framework/src/Illuminate/Container/BoundMethod.php:87

  8   Illuminate\Container\BoundMethod::callBoundMethod(Object(Illuminate\Foundation\Application), Object(Closure))
      /var/www/html/test-app/vendor/laravel/framework/src/Illuminate/Container/BoundMethod.php:31

  9   Illuminate\Container\BoundMethod::call(Object(Illuminate\Foundation\Application), [])
      /var/www/html/test-app/vendor/laravel/framework/src/Illuminate/Container/Container.php:564

  10  Illuminate\Container\Container::call()
      /var/www/html/test-app/vendor/laravel/framework/src/Illuminate/Console/Command.php:184

  11  Illuminate\Console\Command::execute(Object(Symfony\Component\Console\Input\ArgvInput), Object(Illuminate\Console\OutputStyle))
      /var/www/html/test-app/vendor/symfony/console/Command/Command.php:251

  12  Symfony\Component\Console\Command\Command::run(Object(Symfony\Component\Console\Input\ArgvInput), Object(Illuminate\Console\OutputStyle))
      /var/www/html/test-app/vendor/laravel/framework/src/Illuminate/Console/Command.php:171

  13  Illuminate\Console\Command::run(Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
      /var/www/html/test-app/vendor/symfony/console/Application.php:886

  14  Symfony\Component\Console\Application::doRunCommand(Object(BeyondCode\DumpServer\DumpServerCommand), Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
      /var/www/html/test-app/vendor/symfony/console/Application.php:262

  15  Symfony\Component\Console\Application::doRun(Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
      /var/www/html/test-app/vendor/symfony/console/Application.php:145

  16  Symfony\Component\Console\Application::run(Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
      /var/www/html/test-app/vendor/laravel/framework/src/Illuminate/Console/Application.php:89

  17  Illuminate\Console\Application::run(Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
      /var/www/html/test-app/vendor/laravel/framework/src/Illuminate/Foundation/Console/Kernel.php:122

  18  Illuminate\Foundation\Console\Kernel::handle(Object(Symfony\Component\Console\Input\ArgvInput), Object(Symfony\Component\Console\Output\ConsoleOutput))
      /var/www/html/test-app/artisan:37

After looking into some of the code, it seems that this is an inconsistency in the describe() method between CliDescriptor and HtmlDescriptor. See below - CliDescriptor checks to see if the $request contains a controller index before invoking the dump() method to output the controller name, whereas HtmlDescriptor does not:

// from Symfony\Component\VarDumper\Command\Descriptor\CliDescriptor, line 51-53
if ($controller = $request['controller']) {
    $rows[] = array('controller', rtrim($this->dumper->dump($controller, true), "\n"));
}
// from Symfony\Component\VarDumper\Command\Descriptor\HtmlDescriptor, line 47
$controller = "<span class='dumped-tag'>{$this->dumper->dump($request['controller'], true, array('maxDepth' => 0))}</span>";

I was able to get around this in a very hacky way by modifying DumpServerCommand, lines 69-71 to be as follows:

$this->server->listen(function (Data $data, array $context, int $clientId) use ($descriptor, $io) {
    // workaround for dumping html content when not within a controller
    if (is_null($context['request']['controller'])) {
        $context['request']['controller'] = new Data(["-"]);
    }
    $descriptor->describe($io, $data, $context, $clientId);
 });

To me this is probably something that should be addressed within the Symfony var-dumper package, but I wanted to check with @mpociot to see if this workaround is something worth adding to this package, if temporarily.

Thoughts? Is there a different way to accomplish this that is a bit more elegant? Or, does it even matter - how many people will be dump()-ing things to HTML format from their routes file?

[mpociot]

Maybe we should just extend Symfonys HtmlDescriptor and create our own that works with this too?
Since you PRd this, @paulredmond do you want to look into it?

[paulredmond]

@mpociot I’d be happy to check it out this week probably Monday or Tuesday

[mattlibera]

If you have not already looked into it, I tinkered a bit with it today. Here is the result: https://github.com/mattlibera/laravel-dump-server/tree/implement-htmldescriptor

To extend Symfony's HtmlDescriptor requires us to re-implement most everything in the class, as much of it is private. It also may require us to pull in our own version of their Resources (js and css). That is what I have done in the fork above. We could also refer to Symfony's copies of those files (__DIR__ . '/../../../symfony/var-dumper/Resources' would be the path) - that could work since it's reasonable to assume that the var-dumper package path will be present (since it is a requirement)... but feels a bit odd to me to directly refer to files in another package like this.

@mpociot I just wanted to run this by you, to see how you feel about having to maintain basically your own copy of the HtmlDescriptor files, not just the PHP class, but also the assets. Feel free to comment and I can tweak things, or if @paulredmond is already working on his own fix, I can stand down :)

[paulredmond]

I am wondering your thoughts on this...

Closure routes aren't a good idea if you want to support php artisan route:cache, so perhaps we can leave as-is for now @mpociot and in the meantime send a PR to the Symfony team that checks for $request['controller'] in the HtmlDescriptor.

This is the only line that will need to change:

if ($request['controller']) {
    $controller = "<span class='dumped-tag'>{$this->dumper->dump($request['controller'], true, array('maxDepth' => 0))}</span>";
}

And later in the HtmlDescriptor::describe() there's a check for $controller:

$tags = array_filter(array(
    'controller' => $controller ?? null,
    'project dir' => $projectDir ?? null,
));

[paulredmond]

After more digging I think we can work through this by updating the BeyondCode\DumpServer\RequestContextProvider file. Right now, it passes on null if the controller isn't set, but we can get the closure from the route.

I'll open another PR and we can work through it style-wise @mpociot so you're happy with the method in which we check if it's a closure.

[mattlibera]

Yes, this solution feels much more "right" to me. I hadn't even looked at that file!

Thanks for taking time to look at this, even though in practice it probably won't be encountered too much (I agree re: closure routes). I just know I got tripped up when the HTML output blew up with the example from the README.

And, thanks to you both for your innumerable contributions to the Laravel community!

[paulredmond]

Awesome, I’m glad we could figure it out with a simple tweak 👍