After last update my site was crashed. Route '/api/menu_items/main' responded with 500 error.

UnexpectedValueException: External URLs do not have an internal route name. in Drupal\Core\Url->getRouteName() (line 555 of /var/www/html/web/core/lib/Drupal/Core/Url.php).

Unrouted links are useful for headless sites - you can place into menu internal link to pages that generated on frontend only.

Source of problem is here /rest_menu_items/src/Plugin/rest/resource/RestMenuItemsResource.php at line 261

if (!$url->isExternal() && $url->getRouteName() === '<nolink>' && in_array($key, $itemsToRemoveWhenNoLink)) {

I suppose, we should also add here one more condition: $url->isRouted()

CommentFileSizeAuthor
#11 3034663-unrouted-error-11.patch1.46 KBainarend
#9 3034663-unrouted-error-8.patch1.06 KBainarend
#9 with-patch.png67.41 KBainarend
#9 500-error.png21.81 KBainarend
#9 menu-item.png34.33 KBainarend
#7 interdiff-2-7.txt677 bytesainarend
#7 3034663-unrouted-error-7.patch741 bytesainarend
#2 3034663-unrouted-error-2.patch759 bytesgun_dose

Comments

gun_dose created an issue. See original summary.

gun_dose’s picture

gun_dose commented
StatusFileSize
new 3034663-unrouted-error-2.patch759 bytes

And here is patch to fix this.

andysipple’s picture

andysipple commented

Ran into the same problem, reverted back to 2.5 for the time being. I am thinking it was because we had a couple menu links using # at the end of an internal link. Example /some-internal-link#hello-world
I'll try the patch and report back.

gun_dose’s picture

gun_dose commented
Status: Active » Needs review
supermoos’s picture

supermoos commented

Having the same problem.

ainarend’s picture

ainarend commented
Status: Needs review » Needs work

Can confirm that the patch in #2 resolves the issue.

But there are two small improvements we can make to the patch:

+    if (!$url->isExternal() && $url->isRouted() && $url->getRouteName() === '<nolink>' && in_array($key, $itemsToRemoveWhenNoLink))

In there we are calling the $url object to check if the url is not external and whether it's routed. But both of those calls are already stored to a variable.

Drupal\rest_menu_items\Plugin\rest\resource\RestMenuItemsResource:

  protected function getElementValue(array &$returnArray, $key, MenuLinkInterface $link, Url $url) {
    $external = $url->isExternal();
    $routed = $url->isRouted();
    $existing = TRUE;
    $value = NULL;

    // Check if the url is a <nolink> and do not do anything for some keys.
    $itemsToRemoveWhenNoLink = ['uri', 'alias', 'absolute', 'relative'];
    if (!$url->isExternal() && $url->getRouteName() === '<nolink>' && in_array($key, $itemsToRemoveWhenNoLink)) {
      return;
    }

So we could simply do

+    if (!$external && $routed && $url->getRouteName() === '<nolink>' && in_array($key, $itemsToRemoveWhenNoLink))

________

Other than that, the bug causes a WSOD, but this patch fixes it.

ainarend’s picture

ainarend commented
Status: Needs work » Needs review
StatusFileSize
new 3034663-unrouted-error-7.patch741 bytes
new interdiff-2-7.txt677 bytes

Here is a new patch with the mentioned small improvements.

Status: Needs review » Needs work

The last submitted patch, 7: 3034663-unrouted-error-7.patch, failed testing. View results
- codesniffer_fixes.patch Interdiff of automated coding standards fixes only.

ainarend’s picture

ainarend commented
Status: Needs work » Needs review
StatusFileSize
new menu-item.png34.33 KB
new 500-error.png21.81 KB
new with-patch.png67.41 KB
new 3034663-unrouted-error-8.patch1.06 KB

Adding screenshots that show that the patch fixes the 500 error.

Created an unrouted menu item:
Unrouted menu item

500 error on rest menu items api route for main menu:
500 error on the api route

The same route with the last patch:
No error with the patch for api route

____

Okay, in the mean time the testbot has finished and found a coding standard issue.

The test failure was introduced in #3025723 with this commit:
https://git.drupalcode.org/project/rest_menu_items/commit/c4b09e4

Uploaded a new patch with the CS fix.

Status: Needs review » Needs work

The last submitted patch, 9: 3034663-unrouted-error-8.patch, failed testing. View results

ainarend’s picture

ainarend commented
Status: Needs work » Needs review
StatusFileSize
new 3034663-unrouted-error-11.patch1.46 KB

Oops, missed 1 additional CS issue. Should be all good now.

Status: Needs review » Needs work

The last submitted patch, 11: 3034663-unrouted-error-11.patch, failed testing. View results

ainarend’s picture

ainarend commented
Status: Needs work » Reviewed & tested by the community

Ok, don't get it why the testbot says that the patch failed testing now - build is successful and no CS errors. Maybe because of the module doesn't have tests?

Either way, I manually tested the solution and it fixes the issue.

fabianderijk’s picture

fabianderijk
him/his
Primary language Dutch
Location Alphen aan den Rijn
commented
Assigned: Unassigned » fabianderijk

Thanks for the patch, I'll add it in the upcoming days

fabianderijk’s picture

fabianderijk
him/his
Primary language Dutch
Location Alphen aan den Rijn
commented
Status: Reviewed & tested by the community » Fixed

The patch has been applied and will be available in the dev release. The 8.x-2.8 release will be available soon.

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.