Exception when rendering breadcrumbs on Laravel 404 error page

[lewislarsen]

When overriding the default 404 template with a template that uses breadcrumbs in the view, and navigating to a implictly bound route that 404's you encounter the following error:

Diglactic\Breadcrumbs\ServiceProvider::{closure}(): Argument #2 ($directory) must be of type App\Models\Directory, string given

Here's the relevant breadcrumbs

Breadcrumbs::for('directory.show', function (BreadcrumbTrail $trail, Directory $directory) {
    $trail->parent('directory.index');
    $trail->push($directory->title, route('directory.show', ['directory' => $directory]));
});
// 404
Breadcrumbs::for('errors.404', function (BreadcrumbTrail $trail) {
    $trail->push('Page Not Found');
});

Would you happen to know the best way to resolve this? Going to look into it myself too. :)

[shengslogar]

Hmm... it's not immediately clear why this would be happening, I'll need to try and recreate this.

The obvious issue is somehow the breadcrumb for directory.show is getting called when only the errors.404 breadcrumb should be, hence the type exception. You might try replacing the Directory $directory type with an anonymous $directory to see what's being passed through.

Can you show me how you're calling Breadcrumbs::render in your Blade templates?

[shengslogar]

I'm not able to repro this. You'll need to post additional source code.

[lewislarsen]

Here's some more, hope this helps!

app.blade.php (which is extended on the show blade template & 404 template)

<!-- Breadcrumb -->
{{ Breadcrumbs::render() }}

web.php section

Route::namespace('directory')->name('directory.')->prefix('directory')->group(function () {
    Route::get('/', [IndexController::class, '__invoke'])->name('index');
    Route::get('/view/{directory}', [ShowDirectoryController::class, '__invoke'])->name('show');
    });

The model's default route key name is a slug, it probably doesn't matter but I wanted to mention it anyway!

Using Laravel version 8.7.5 & breadcrumbs version 7.0.1. Let me know if there's anything specific you want to see, it is doing this on every page that has route parameters that 404s looking at my failing tests. (I don't have any routes with parameters that don't use implicit binding)

[shengslogar]

Thanks. Was able to repro. Will need to look into this.

Edit:

Issue specifically relates to 404s on named routes.

laravel-breadcrumbs/src/Manager.php

Lines 241 to 243 in 0a14e87

	if ($route === null) {
	return ['errors.404', []];
	}

[shengslogar]

Ok, after thinking over this for awhile, I've decided to close out this issue without implementing a fix. Technically, the package is doing the correct behavior. If your controller were to fire with the same parameters being passed breadcrumbs, it, too, would throw a similar exception.

// breadcrumbs.php
Breadcrumbs::for('users.show', function(BreadcrumbTrail $trail, User $user){
   // ...
});

// UserController.php

public function show (User $user) { // This never fires on 404; if it did, there would be a type mismatch
   // ...
}

While messy, it seems to me that this variability should be handled at the breadcrumb definition level.

Breadcrumbs::for('users.show', function(BreadcrumbsTrail $trail, $user) {
    if ($user instanceof User) {
          $trail->push($user->name);
    } else {
          $trail->push('User not found');
    }
});

There's a reason why the default Laravel error pages don't have any app-related data on them because that just introduces additional points of failure.

There is a workaround that could help you avoid this. Whenever Laravel renders an error page, it passes an $exception variable to your Blade templates. That allows you to write:

@unless (isset($exception))
     {{ Breadcrumbs::render() }}
@endunless

Now, breadcrumbs will be entirely missing whenever an error is rendered. This makes sense to me, since breadcrumbs often pull data from models and the database which could cause a stack overflow.

Please voice any counterarguments, but for now I'm considering this an anti-pattern to the package.