Due to some problems it could happen that a plugin which is requested from
the plugin manager does not exist anymore.
#1879774: Catch plugin exceptions for invalid views display plugins#1881096: Regression: Site broken when disabling module without removing block instances.#1822048: Consider improving/removing the concept of "broken" handlers#2063403: Fatal when editing a nonexistent image effect#2007248: When CKEditor is disabled, editor config entities that refer to it cause a Twig exception
One example what views is doing: It replaces the plugin instance by some stub implementation which does nothing.
So the current approach in #1881096: Regression: Site broken when disabling module without removing block instances. is to allow plugins belonging to disabled modules to be silently ignored, but to still throw exceptions for all other plugin problems for debugging.
I'm hesitant about the approach in #1879774: Catch plugin exceptions for invalid views display plugins. On the one hand, as dawehner pointed out, it allows you to use the Views admin page even if the View has a broken display (presumably so that you can remove it). However, it also allows broken Views to continue to function on the frontend, but then goes around and displays a warning to end users anyway. Also, I'd like to consider how to handle this for plugins in general before introducing more Viewsisms.
I've always considered the debug() from Views' missing/broken handlers to be less than ideal as well, but I'm not sure what the correct answer is, either.
One thing that is probably important is why the plugin is invalid:
Another thing to consider is what happens when you disable a module that's providing plugins. E.g., if a user tries to disable my_block_types.module or my_views_displays.module and there are block instances or views using the plugins provided by the module, the user should realize that they're disabling those configured blocks and those Views displays.
Oh, I keep forgetting to mention: The one downside I can think of for silently skipping disabled modules' data is that module_exists() doesn't distinguish between disabled and uninstalled, so we might end up with crufty data if a module doesn't properly delete its data on uninstall, and not know about it without inspecting the data manually. Maybe we could add a report entry that lists plugin data belonging to modules that are not currently installed...
Another reason why the plugin could be invalid: Let's assume we have some kind of views plugin
which uses derivatives and so once per entity type.
I agree with xjm that we should do our best and warn people if they disable a certain module,
but this could be hard to detect all the times where it's used. See the disabled configuration entities
discussion #1826602: Allow all configuration entities to be enabled/disabled which does not fit 100% into here, but I think you get the point if you look at
the text formats example.
When you disable node, the plugin itself might not belong to the node module, as the code is all in views.
Then you get a plugin id like 'entity:node' and boom this plugin ID is simply unknown.
Do we really want to break the site based on that? This seems to be not answerable on a global scale but would have to be decided per plugin type.
To clarify, I don't think there's one universal solution; I'd mostly like to decide best practices for some typical cases and maybe some API. Blocks and views are different -- each block depends on just one plugin, but views are built from multiple plugins.
I'd also like to consider how and when we should report the issue to the user. Views right now can't decide if we should use drupal_set_message() or watchdog() or debug() or re-throw exceptions or fail silently or add a report to the reports page... :) Edit: there's also a case for displaying messages when administering views, but not when viewing them; and the question of when a handler is "required" for a view to function and when it simply adds functionality, which is going to depend on the specific view.
Mmm... why are we duplicating the discussion from #1822048: Consider improving/removing the concept of "broken" handlers and #1881096: Regression: Site broken when disabling module without removing block instances. into yet another issue?
The arguments and considerations that have been raised so far are almost 100% identical to existing comments in those issues, so we're only repeating ourselves. Personally, I assumed that any kind of clues + consensus on those issues would inherently formulate the common and general direction for all other related issues.
I guess the intention is to have this issue as a meta issue. Re-titling and tagging accordingly.
#1862600: Entity type names/IDs are not properly namespaced by owner (e.g., taxonomy.term vs. taxonomy_term) is missing in the list of related issues, but is closely related, since it focuses on
1) the question of what exactly the current plugin IDs are good for; or more specifically:
- Who owns a certain 'plugin_id' and who's responsible for it?
- Where's the CRUD logic that is required to handle changes in plugin definitions?
- Why do we always need a rather massive array definition just to be able to reference a certain plugin, instead of a simple string?
- (Almost?) All plugin IDs contain the providing module as prefix anyway already, so why duplicate it in 'module'?
2) a too easily reachable disparity between 'plugin_id' and 'module' properties where a plugin is referenced.
- Is the plugin_id 'foo_bar', formerly provided by foo.module, still the same as the plugin_id 'foo_bar', now provided by baz.module?
- Is it even legit and/or technically possible to simply swap it out that way in the first place?
- If the answer is "Yes", then we need a clear definition of under which exact circumstances that swapping is able to work.
- If the answer is "No", then baz.module's 'foo_bar' plugin has to be 'baz.bar' instead of 'foo.bar'. (KISS)
Once we have answers to these questions, the topic isn't actually resolved yet — Derivative Plugins apparently add a whole 3rd dimension on top of the simple two dimensions that involve plugin manager vs. owner/provider, since individual derivatives are not necessarily owned/provided by the plugin provider, so the dependency chain becomes 3D.
following on from #3, there's another case that is likely to be common in D8 - plugins that are broken because of a partial import.
we're going to allow imports to pull in lots of different types of configurable thingies that could reference missing plugins, so we'd better add that to the list of ways we're going to hit this case.
If I'm reading xjm's comment in #6 I think I'm in agreement. The factory and discovery should fail in such a way that the calling code can handle it in a manner appropriate to the context. If you're in the admin interface you can display "X not found" if you're in page deliver maybe you just through some empty text or don't render. Its not really up to the plugin system I don't think to handle the error but to pass it off gracefully.
To that end, I know @sdboyer had a vision of doing a once over and making plugins through better exceptions and I /think/ this would be that sort of place.
There are currently two type of exceptions provided by the plugin system:
but additional when we have the 'module' key specified in each plugin type why not also throw an exception with the module does not exist/is not enabled?
As each of these exceptions are logically different we should better go with separated exception types.
Yeah, that's possible and probably necessary. Merlin actually asked for arbitrary plugin dependencies and it didn't happen for various reasons. If views where still in contrib, this would be how it would do things like provide handlers for forum module and have them only available and functional when the forum module was enabled. Maybe now is the time to do that. Module's enabled/disabled status are a specific common case for having a plugin dependency but there could be others so we should at least consider a more general solution like a callback though.
Marked #2055813: [meta] add exception handling for each plugin type and check upgrade tests as a dupe
Updated issue summary.
Added #2063403: Fatal when editing a nonexistent image effect to summary and we need to fix upgrade path first for image #2049465: Upgrade of image styles and effects broken
Marked #2007248: When CKEditor is disabled, editor config entities that refer to it cause a Twig exception as a duplicate of this issue. See comment #15 (in that issue) by David_Rothstein in particular.
Raising priority because we cannot ship Drupal 8 without a clear answer to this problem; it appears this problem is being fixed in ad hoc ways instead of the same consistent way everywhere.
it appears this problem is being fixed in ad hoc ways instead of the same consistent way everywhere.
We might not have any choice about this in some circumstances. E.g. views will likely want to deal with missing plugins (using broken handlers for example) than maybe image effects do? We also have (and have had) a few issues pertaining to block derivatives etc... also.
This is closely related to #2080823: Create API to discover content or config entities soft dependencies and use this to present a confirm form on module uninstall.
#15: True :) But I still think a "general rule of thumb" for Drupal contrib developers is a must-have. Core modules less complex than Views (e.g. CKEditor) could follow that too.
Drupal is a registered trademark of Dries Buytaert.