Closed (outdated)
Project:
Drupal core
Version:
11.x-dev
Component:
plugin system
Priority:
Normal
Category:
Task
Assigned:
Unassigned
Reporter:
Created:
24 Jan 2013 at 16:21 UTC
Updated:
18 Jul 2024 at 05:57 UTC
Jump to comment: Most recent, Most recent file
Comments
Comment #1
xjmYep.
Comment #2
eclipsegc commentedFirst pass at a patch.
As a side note, we cannot provide the Manager class that called this reliably because of: #1851706: Make all core plugin managers be consistent as to whether $this or $this->discovery is passed to the factory and apparently no amount of discussion will solve that. We also probably shouldn't spit out plugin definitions in the exceptions either. I don't know of a way that could be a security issue, but hey... you never know. It's also really hard to read. This patch at least makes a difference between "Your plugin didn't specify a class" and "Your plugin doesn't exist". That's probably the best we can do for the time being short of putting catch statements in the methods on the plugin manager to catch the messages and add the manager's name to them (which is possible, but also likely to be problematic with regard to #1851706: Make all core plugin managers be consistent as to whether $this or $this->discovery is passed to the factory and I can hear people screaming already about catching an exception just to throw it again).
Not sure what else we can do to improve this.
Eclipse
Comment #3
eclipsegc commentedComment #4
xjmI told @EclipseGc to say in the issue that he asked me about this in IRC to make it clear he wasn't ignoring issue etiquette, but he apparently forgot. ;)
#2 is approximately what I was doing, plus this.
Comment #5
webchickSigh. :( I stupidly accidentally blew away my site from the debug issue and now can't replicate the problem. :(
I manually adjusted getPluginClass() with a
if ($plugin_id == 'ckeditor') { $plugin_definition = NULL; }and now the error back is:I'm not sure if CKEditor is just a weird one-off here, but CacheDecorator doesn't seem to be important/helpful at all to figuring out where the problem is.
I erased that debugging line and just put an unconditional $plugin_definition = NULL for everything and reload /node. That gives me:
Here, FormatterPluginManager does seem to be a lot more relevant.
OTOH, if I go to user/1:
Not sure about that one; seems kind of irrelevant again.
This is definitely a step in the right direction, but not sure I feel qualified to RTBC. The discovery seems to be hit and miss in terms of usefulness to debugging. I love the idea of putting *something* there, but I wonder if there's another property that might be more helpful?
Comment #6
webchickOh wait. That's FormatterPluginManager for both, because both are fields. For some reason (probably because it's stupid o'clock), I read the first as FormatPluginManager.
Yeah, I dunno. To me that discovery being there would just send me down a wild goose chase, as it already did in #1897364: How the bleepity bleep do you debug plugin-related errors? :), but am I missing something valuable?
Comment #7
neclimdulrelated #1846070: Improve exception messages when plugins cannot be found
Comment #8
jibran#4: plugin_error-1897762-4.patch queued for re-testing.
Comment #10
rbayliss commentedRerolled. No interdiff because I couldn't apply the patch from #4. We no longer have access to the discovery in this method, so I guess any arguments for or against printing the discovery mechanisms are a moot point now.
Comment #12
clemens.tolboomXREF #1846070: Improve exception messages when plugins cannot be found
Comment #13
xjmxjm does not know why she still has this assigned to herself.
Comment #27
quietone commentedThis was fixed in #2485513: DefaultFactory cannot deal with objects as plugin definitions