Closed (fixed)
Project:
Drupal core
Version:
8.7.x-dev
Component:
plugin system
Priority:
Critical
Category:
Bug report
Assigned:
Unassigned
Issue tags:
Reporter:
Created:
5 Apr 2019 at 21:38 UTC
Updated:
22 Apr 2019 at 05:29 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #4
tim.plunkettComment #5
jibranThanks, for creating the issue setting it RTBC as per original issue.
Comment #7
alexpott@larowlan asked in slack if
we can catch a more specific context. @tim.plunkett can we catch
\Drupal\Component\Plugin\Exception\PluginExceptioninstead?Comment #8
pandaski commentedQuick thought. why not update the getContext method then we can catch the '\Drupal\Component\Plugin\Exception\PluginException' from there.
Comment #9
pandaski commentedBased on thoughts in #8, check the $context is an instance of ContextInterface, otherwise, it is NULL
Comment #11
pandaski commentedComment #12
pandaski commented'getContext' always return an instance of ContextInerface or NULL instead of any exception.
Comment #13
tim.plunkett@Joseph Zhao that's not true. And that's actually the mistake I made in the original issue.
#8 shows the code. If it is not set, it creates one and returns it. Therefore it always returns a context object... However
\Drupal\Component\Plugin\ContextAwarePluginBase::getContextDefinition()can throw the exception.It says it will throw
\Drupal\Component\Plugin\Exception\PluginExceptionBut it actually throws
\Drupal\Component\Plugin\Exception\ContextExceptionIt switched from PluginException to ContextException in #2281635: \Drupal\Core\Plugin\Context\Context should use a ContextDefinition class instead of arrays.
I opened #3046342: \Drupal\Component\Plugin\ContextAwarePluginInterface has incorrect @throws docs to fix the docs.
In the meantime, we should catch both (even though one is only going to happen).
Interdiff is from my last patch in #4.
Comment #14
alexpott@tim.plunkett++ thanks the new patch looks much better than catching \Exception - if it is not rtbc tomorrow I'll do that otherwise I'll commit it if it is :)
Comment #15
jibranI agree patch looks much better now. Thanks!
Comment #18
catchCross-linking #2677162: Context::getContextValue() violates ContextInterface by throwing an unexpected exception and #2620126: No way to handle 'optional' contexts without try/catch.
Committed/pushed to 8.8.x and cherry-picked to 8.7.x, thanks!
Comment #19
jibranCorrecting the tag.