πŸ‡¦πŸ‡ΊAustralia @david.houlder

Account created on 2 September 2009, over 15 years ago
#

Recent comments

πŸ‡¦πŸ‡ΊAustralia david.houlder

For what it's worth, those problematic paths are URL aliases for taxonomy terms. e.g. in /taxonomy/term/260/edit I see

Name: Calls for Papers
URL alias: /category/article-content/calls-papers

πŸ‡¦πŸ‡ΊAustralia david.houlder

Sure, no worries.

easy_breadcrumb/src/TitleResolver.php

  public function getTitle(Request $request, Route $route) {
    $url = Url::fromUri("internal:" . $request->getRequestUri());
    $entity = NULL;
    try {
      $route_parts = explode(".", $url->getRouteName());
      $params = $url->getRouteParameters();
      if ($route_parts[0] === 'entity' && $route_parts[2] === 'canonical') {
        $entity_type = $route_parts[1];
        if (isset($params[$entity_type])) {
          $entity = $this->entityTypeManager->getStorage($entity_type)->load($params[$entity_type]);
        }
      }
    } catch (\UnexpectedValueException $e) {
    }

and the rest is unchanged.

πŸ‡¦πŸ‡ΊAustralia david.houlder

Don't want it white screening a website ever.

Sure, but in that case, why isn't it wrapped in a try…catch already? I would argue that if the code is so poorly behaved or understood that you have to do that, you've got some significant problems with code quality.

Besides, I'm using the code after my try…catch to set $title appropriately, and on top of that, keeping the try block small makes the code easier to understand, as it limits the scope of calls that are likely to throw the exception. It also means you don't need a comment in the catch block many lines distant from the function call describing what's going on.

As I said before, I have spent way too much time on this already, and I am unlikely to be the only one who runs into this problem. I am going ahead with my patched version of 2.0.5 or 2.x-dev. You guys know this code way better than me, and are in a much better position to diagnose any underlying issue.

Thanks for your time anyway.
cheers
David

πŸ‡¦πŸ‡ΊAustralia david.houlder

Hrm, this looks like a core bug, but I am not sure which one. Can you try this patch to see if it gives us a better error to go off?

https://www.drupal.org/project/drupal/issues/3308418 πŸ› Improve error message on URI with no corresponding route Needs work

That's already done in
https://git.drupalcode.org/project/drupal/-/blob/9.5.x/core/lib/Drupal/C...
and the offending url is already reported in the error string: UnexpectedValueException: base:category/article-content/growing has no corresponding route. in Drupal\Core\Url->getRouteName() (line 567 of core/lib/Drupal/Core/Url.php).

1. Wrap the entire function block in try catch, this makes it so the catch is at the end of the function and not the middle.

I think that's bad idea. You want the try to wrap as little code as possible because we're only interested in the UnexpectedValueException from getRouteName(). Wrapping the entire block in a try would catch the exception from everything, which would obscure unanticipated UnexpectedValueExceptions from other things which should be debugged and fixed.

πŸ‡¦πŸ‡ΊAustralia david.houlder

Hi Greg. With 2.x-dev I still get

The website encountered an unexpected error. Please try again later.

UnexpectedValueException: base:category/article-content/growing has no corresponding route. in Drupal\Core\Url->getRouteName() (line 567 of core/lib/Drupal/Core/Url.php).

My use case is not particularly weird. The URLs that cause problems are generated by a taxonomy view block. There are bound to be other sites affected by this.

See the live site https://xyonline.net/ (running easy_breadcrumb 2.0.2). There's a Topics menu in the left sidebar, which is provided by a taxonomy view block. The URLs in that topic list are the ones that cause problems in easy_breadcrumb > 2.0.3.

To make this work I've patched 2.x-dev (see below). There's arguably a bit too much code in the try block, but this gets me out of trouble. I don't know if you want to pursue this further, but I've already spent way too much time on this, and I'm going to use my patched version to get the site to a state where I can migrate to Drupal 10, unless you have a better idea.

cheers
David


diff -Naur /home/david/Downloads/drupal/easy_breadcrumb-2.x-dev/easy_breadcrumb/src/TitleResolver.php easy_breadcrumb/src/TitleResolver.php
--- /home/david/Downloads/drupal/easy_breadcrumb-2.x-dev/easy_breadcrumb/src/TitleResolver.php	2024-01-04 04:09:27.000000000 +1100
+++ easy_breadcrumb/src/TitleResolver.php	2024-01-26 09:27:15.951298503 +1100
@@ -70,14 +70,17 @@
    */
   public function getTitle(Request $request, Route $route) {
     $url = Url::fromUri("internal:" . $request->getRequestUri());
-    $route_parts = explode(".", $url->getRouteName());
     $entity = NULL;
-    $params = $url->getRouteParameters();
-    if ($route_parts[0] === 'entity' && $route_parts[2] === 'canonical') {
-      $entity_type = $route_parts[1];
-      if (isset($params[$entity_type])) {
-        $entity = $this->entityTypeManager->getStorage($entity_type)->load($params[$entity_type]);
+    try {
+      $route_parts = explode(".", $url->getRouteName());
+      $params = $url->getRouteParameters();
+      if ($route_parts[0] === 'entity' && $route_parts[2] === 'canonical') {
+        $entity_type = $route_parts[1];
+        if (isset($params[$entity_type])) {
+          $entity = $this->entityTypeManager->getStorage($entity_type)->load($params[$entity_type]);
+        }
       }
+    } catch (\UnexpectedValueException $e) {
     }
     if ($entity !== NULL) {
       $current_langcode = $this->languageManager->getCurrentLanguage()->getId();

πŸ‡¦πŸ‡ΊAustralia david.houlder

That's the line of code that gets the title of the page for use in the crumb. Without it, you're getting the fallback title or entity label for each route.

I don't think so. Have a look at the code. $title is overwritten by $entity->label(); in all cases. The result of getTitleString() is never used here, and none of its arguments are passed by reference.

πŸ‡¦πŸ‡ΊAustralia david.houlder

See
https://git.drupalcode.org/project/easy_breadcrumb/-/blob/2.0.4/src/Easy...

                   $title = $this->normalizeText($this->getTitleString($route_request, $route_match, $replacedTitles));
                    // Add this entity's cacheability metadata.
                    $breadcrumb->addCacheableDependency($entity);
                    $title = (string) $entity->label();

I commented out the first $title assignment

                   // $title = $this->normalizeText($this->getTitleString($route_request, $route_match, $replacedTitles));

and it works now. Since that value of $title is never used, I suspect this may be a fix, at least for my case. I'm not familiar enough with the Drupal API to guess what was intended here.

Production build 0.71.5 2024