Problem/Motivation
Trying to just administer the "Content" navigation (/admin/structure/menu/manage/content) from the new Core Navigation module the page didn't load and I got:
Symfony\Component\Routing\Exception\RouteNotFoundException: Route "" does not exist. in Drupal\Core\Routing\RouteProvider->getRouteByName()
Stacktrace - as you can see it won't help much::
Symfony\Component\Routing\Exception\RouteNotFoundException: Route "" does not exist. in Drupal\Core\Routing\RouteProvider->getRouteByName() (line 242 of core/lib/Drupal/Core/Routing/RouteProvider.php).
Drupal\Core\Routing\UrlGenerator->getRoute() (Line: 281)
Drupal\Core\Routing\UrlGenerator->generateFromRoute() (Line: 105)
Drupal\Core\Render\MetadataBubblingUrlGenerator->generateFromRoute() (Line: 776)
Drupal\Core\Url->toString() (Line: 176)
Drupal\Core\Utility\LinkGenerator->generate() (Line: 104)
Drupal\Core\Render\Element\Link::preRenderLink()
call_user_func_array() (Line: 107)
Drupal\Core\Render\Renderer->doTrustedCallback() (Line: 914)
Drupal\Core\Render\Renderer->doCallback() (Line: 447)
Drupal\Core\Render\Renderer->doRender() (Line: 517)
Drupal\Core\Render\Renderer->doRender() (Line: 230)
Drupal\Core\Render\Renderer->render() (Line: 501)
Drupal\Core\Template\TwigExtension->escapeFilter() (Line: 202)
__TwigTemplate_6fe6abbf39fffb97a26f20c0b7a9317c->doDisplay() (Line: 411)
Twig\Template->yield() (Line: 394)
Twig\Template->render() (Line: 51)
Twig\TemplateWrapper->render() (Line: 35)
Drupal\Core\Template\TwigThemeEngine->renderTemplate() (Line: 428)
Drupal\Core\Theme\ThemeManager->render() (Line: 504)
Drupal\Core\Render\Renderer->doRender() (Line: 517)
Drupal\Core\Render\Renderer->doRender() (Line: 517)
Drupal\Core\Render\Renderer->doRender() (Line: 230)
Drupal\Core\Render\Renderer->render() (Line: 242)
Drupal\Core\Render\MainContent\HtmlRenderer->{closure:Drupal\Core\Render\MainContent\HtmlRenderer::prepare():235}() (Line: 638)
Drupal\Core\Render\Renderer::{closure:Drupal\Core\Render\Renderer::executeInRenderContext():638}()
Fiber->resume() (Line: 653)
Drupal\Core\Render\Renderer->executeInRenderContext() (Line: 235)
Drupal\Core\Render\MainContent\HtmlRenderer->prepare() (Line: 131)
Drupal\Core\Render\MainContent\HtmlRenderer->renderResponse() (Line: 69)
Drupal\Core\EventSubscriber\MainContentViewSubscriber->onViewRenderArray() (Line: 246)
Symfony\Component\EventDispatcher\EventDispatcher::{closure:Symfony\Component\EventDispatcher\EventDispatcher::optimizeListeners():241}() (Line: 206)
Symfony\Component\EventDispatcher\EventDispatcher->callListeners() (Line: 56)
Symfony\Component\EventDispatcher\EventDispatcher->dispatch() (Line: 188)
Symfony\Component\HttpKernel\HttpKernel->handleRaw() (Line: 76)
Symfony\Component\HttpKernel\HttpKernel->handle() (Line: 53)
Drupal\Core\StackMiddleware\Session->handle() (Line: 30)
Drupal\Core\StackMiddleware\KernelPreHandle->handle() (Line: 28)
Drupal\Core\StackMiddleware\ContentLength->handle() (Line: 32)
Drupal\big_pipe\StackMiddleware\ContentLength->handle() (Line: 118)
Drupal\page_cache\StackMiddleware\PageCache->pass() (Line: 92)
Drupal\page_cache\StackMiddleware\PageCache->handle() (Line: 263)
Drupal\shield\ShieldMiddleware->bypass() (Line: 162)
Drupal\shield\ShieldMiddleware->handle() (Line: 62)
Drupal\crowdsec\Middleware->handle() (Line: 53)
Drupal\ban\BanMiddleware->handle() (Line: 48)
Drupal\Core\StackMiddleware\ReverseProxyMiddleware->handle() (Line: 51)
Drupal\Core\StackMiddleware\NegotiationMiddleware->handle() (Line: 61)
Drupal\Core\StackMiddleware\AjaxPageState->handle() (Line: 49)
Drupal\remove_http_headers\StackMiddleware\RemoveHttpHeadersMiddleware->handle() (Line: 54)
Drupal\Core\StackMiddleware\StackedHttpKernel->handle() (Line: 753)
Drupal\Core\DrupalKernel->handle() (Line: 34)
Symfony\Component\Runtime\Runner\Symfony\HttpKernelRunner->run() (Line: 32)thrown by this check:
$routes = $this->getRoutesByNames([$name]);
if (empty($routes)) {
throw new RouteNotFoundException(sprintf('Route "%s" does not exist.', $name));
}
Debugging RouteProvider::getRouteByName() I found out that the $name parameter was an empty string ("") - so the exception thrown was even a bit too late and not very specific. Empty string routes are never valid IMHO.
Tracking down the problem further for hours I found a database entry in the menu_tree table with an empty string ("") route was the root cause. The ID was: navigation.create.user.
Deleting that row made the error disappear, and I was able to edit the navigation again.
I don't think it's worth to find out how this entry was created - maybe by an error when installing the navigation module - but I think we should better handle this disallowed case by throwing helpful exceptions early.
Steps to reproduce
See above
Proposed resolution
1. Add another check for empty string in RouteProvider::getRouteByName() which throws an exception just like for empty routes in the line below.
A route name should never be an empty string.
This already allows tracking down the problem a bit more specific
2. If possible the menu_tree database table should not allow empty strings. As I don't think that's possible for a VARCHAR field in database (and NULL is allowed), maybe we could throw an exception when reading from the table, building the links, for example?
3. Add tests for empty routes to ensure helpful exceptions in these cases.
Remaining tasks
User interface changes
Introduced terminology
API changes
Data model changes
Release notes snippet
Issue fork drupal-3614874
Show commands
Start within a Git clone of the project using the version control instructions.
Or, if you do not have SSH keys set up on git.drupalcode.org:
Comments
Comment #2
anybodyComment #4
anybodyI think it's time for feedback on the check and regarding the second point - early checking that menu_tree never returns an empty string as route name.
Thanks!
Comment #5
anybodyComment #6
smustgrave commentedOops left 1 comment on the MR yesterday but forgot to change status.
Comment #7
anybodyThanks @smustgrave - should I also change the unrelated lines where the try / catch is used or only the new ones?
Do you generally agree with the approach?
Any ideas (anyone) regarding menu_tree hardening (2)?
Comment #8
longwaveSurely an invalid route name will have the same problem? Wouldn't it be better to put a try/catch around the point where it is used, so we can gracefully recover from any invalid route?
Comment #9
anybody@longwave thanks, yes. I think we should do both?
My focus was documenting this issue first, so I don't forget about the case.
I'd vote to add the exception here, but also fix it where it happens - in this case the menu system.
Comment #12
longwaveMR !16715 fixes the problem in menu UI instead, by catching the exception and displaying the link as text.
I don't think we should special case empty strings; there are lots of equally invalid route names and they should all be handled the same way.
Comment #13
anybodyThanks @longwave for finding the spot! I left a comment.
Regarding my MR: Yes I can agree on that. Maybe it was also the message that confused me and didn't clarify good enough that an empty = wrong route
$nameparameter value was given.What do you think about changing the exception text a bit:
I still think the empty route name is especially confusing, but maybe it's just my personal view.
Comment #14
anybodyComment #15
longwaveNot really sure whether to log something or not here, logging it in menu UI itself seems like it won't be hit very often (or maybe never), but logging it e.g. at render time would be too noisy. If the menu link doesn't work that should already be a clue to the site owner that something is wrong with the link.
We can improve the exception message, I'm fine with that.
Comment #16
anybodyI think for me the problem would be not knowing why it doesn't work. And that might be hard to find, especially in cases where it's code-defined?
So I'd personally prefer too much noise (which I can stop then by fixing the root cause) - with all the information required to do so - instead of noticing the broken menu item for a while maybe.
For example, I think our case happened after an update.
Anyway I'm totally happy with having a fix at all! Thank you so much!
Comment #17
longwaveWhat if we displayed the exception message in the UI after the plaintext menu link title?
Comment #18
anybodyMhm unsure if that's good or not. Do we have someone else who could decide on these points? I'm totally unsure what's the best approach, but IMHO we shouldn't just mute it.