Issue #3046243 by tim.plunkett, jibran, dpi: Regression: Optional context values may throw exceptions if unsatisfied

core/lib/Drupal/Core/Plugin/Context/ContextHandler.php

@@ -5,6 +5,7 @@ namespace Drupal\Core\Plugin\Context;
 use Drupal\Component\Plugin\Definition\ContextAwarePluginDefinitionInterface;
 use Drupal\Component\Plugin\Exception\ContextException;
 use Drupal\Component\Plugin\Exception\MissingValueContextException;
+use Drupal\Component\Plugin\Exception\PluginException;
 use Drupal\Core\Cache\CacheableDependencyInterface;
 use Drupal\Core\Plugin\ContextAwarePluginInterface;
 
@@ -110,20 +111,37 @@ class ContextHandler implements ContextHandlerInterface {
           // Collect required contexts that exist but are missing a value.
           $missing_value[] = $plugin_context_id;
         }
+
+        // Proceed to the next definition.
+        continue;
       }
-      elseif (($context = $plugin->getContext($context_id)) && $context->hasContextValue()) {
+
+      try {
+        $context = $plugin->getContext($context_id);
+      }
+      catch (ContextException $e) {
+        $context = NULL;
+      }
+      // @todo Remove in https://www.drupal.org/project/drupal/issues/3046342.
+      catch (PluginException $e) {
+        $context = NULL;
+      }
+
+      if ($context && $context->hasContextValue()) {
         // Ignore mappings if the plugin has a value for a missing context.
         unset($mappings[$plugin_context_id]);
+        continue;
       }
-      elseif ($plugin_context_definition->isRequired()) {
+
+      if ($plugin_context_definition->isRequired()) {
         // Collect required contexts that are missing.
         $missing_value[] = $plugin_context_id;
+        continue;
       }
-      else {
+
       // Ignore mappings for optional missing context.
       unset($mappings[$plugin_context_id]);
     }
-    }
 
     // If there are any mappings that were not satisfied, throw an exception.
     // This is a more severe problem than missing values, so check and throw

core/tests/Drupal/Tests/Core/Plugin/ContextHandlerTest.php

@@ -18,7 +18,6 @@ use Drupal\Core\Cache\NullBackend;
 use Drupal\Core\DependencyInjection\ClassResolverInterface;
 use Drupal\Core\DependencyInjection\ContainerBuilder;
 use Drupal\Core\Extension\ModuleHandlerInterface;
-use Drupal\Core\Plugin\Context\Context;
 use Drupal\Core\Plugin\Context\ContextDefinition;
 use Drupal\Core\Plugin\Context\ContextHandler;
 use Drupal\Core\Plugin\ContextAwarePluginInterface;
@@ -379,9 +378,9 @@ class ContextHandlerTest extends UnitTestCase {
       ->method('setContext');
 
     // No context, so no cacheability metadata can be passed along.
-    $plugin->expects($this->once())
+    $plugin->expects($this->any())
       ->method('getContext')
-      ->willReturn(new Context($context_definition));
+      ->willThrowException(new ContextException());
 
     $this->setExpectedException(MissingValueContextException::class, 'Required contexts without a value: hit');
     $this->contextHandler->applyContextMapping($plugin, $contexts);
@@ -415,9 +414,9 @@ class ContextHandlerTest extends UnitTestCase {
       ->method('setContext');
 
     // No context, so no cacheability metadata can be passed along.
-    $plugin->expects($this->once())
+    $plugin->expects($this->any())
       ->method('getContext')
-      ->willReturn(new Context($context_definition));
+      ->willThrowException(new ContextException());
 
     $this->contextHandler->applyContextMapping($plugin, $contexts);
   }