Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
That's all I see when trying to access /node/add
.
Dumping $plugin_id and $plugin_definition yields NULL and NULL.
Such errors should not be possible.
Related issues
Comment | File | Size | Author |
---|---|---|---|
#25 | drupal_1864014_25.patch | 3.75 KB | Xano |
#25 | interdiff.txt | 1.17 KB | Xano |
Comments
Comment #1
sunA
debug_backtrace()
reveals that Field API's Widget Plugin Factory tries to instantiate a widget for this URL alias field instance:Comment #2
yched CreditAttribution: yched commentedMh, I guess the WidgetFactory is at fault here, should provide a better message :-/.
Just for context, can you tell me :
what is the 'path_default' widget type ? where is it provided ?
What's the field type of field_url_alias ? what's its 'default_widget' ?
Comment #3
sunThe path_default widget comes from #1751210: Convert URL alias form element into a field and field widget
I surely have to amend + admit that this plugin only exists in one of my development git branches and not on 8.x, on which I encountered the error message.
In other words: The entire field widget plugin vanished and does not exist in the code-base.
What I don't get though is why the $plugin_id is NULL. As visible in #1, the field instance actually refers to a 'type' "path_default" - and if I get it right, then that's the widget's plugin ID.
So the widget factory should actually still know the plugin's ID?
But yet, the exception is thrown on a $plugin_id that is NULL...?
Also, the DefaultFactory code shows this:
$plugin_definition is NULL, too. So a second issue: Shouldn't the discovery throw an exception of its own when trying to retrieve a definition for a particular, given plugin ID that does not exist?
Comment #4
tstoecklerThat would make checking whether a plugin exists a bit cumbersome, having to catch an exception instead of simply checking for !empty($definition). I think it would make more sense for getPluginClass() to throw an exception if $definition is NULL.
Comment #5
yched CreditAttribution: yched commentedThat's because when the requested plugin id is unknown, WidgetManager::getInstance() backs off to the 'default_widget' specified in the field type info. But in your case the field type had vanished too, so that 'default_widget' is NULL.
Yeah. Field types simply vanishing from the code base are hell to deal with.
Fields and instances whose field type is not available should be marked "inactive".
But when the field type simply disappeared (e.g after a rm or a git checkout), the "active" state is refreshed on next drupal_flush_all_caches(), in field.module's hook_rebuild(). We cannot really do better than that - nor do we want to litter our runtime code with "is this active field really active ?" checks...
As far as the cryptic error message is involved, best I can offer is attached patch - done for formatters too for consistency.
Drawback: is it adds a couple func calls on each widget / formatter instanciation :-/
Feedback welcome on the most relevant Exception class to use, and on the actual message :-)
Comment #6
swentel CreditAttribution: swentel commentedMy initial reaction to this patch really is (and will probably stay for a while): this is babysitting. I mean, it's not like a field plugin is going to disappear during a request right ? I understand it's hard to to track it and will only be 'stable' when a drupal_flush_all_caches() happens, but that applies to so many things in D8 at this point that I'm not convinced to add extra function calls here.
Comment #7
sunNote that I'd be happy with a fix for this issue that makes the error message in the OP expose the actual plugin ID.
The problem is the error message that throws an error without knowing on what it throws an error.
That sounds weird and horrible, but that's exactly what the shotted error message depicts: An error on NULL. Which translates into:
The rest is left for #1862600: Entity type names/IDs are not properly namespaced by owner (e.g., taxonomy.term vs. taxonomy_term) (since the plugin ID on its own does not give any kind of clue at all).
Comment #8
yched CreditAttribution: yched commentedI probably agree, actually
[edit: crosspost - that was "I agree with @swentel #6" :-)]
[re-edit: and doh, fixed the code snippet below, was just a straight copy/paste of the current patch...]
We could do this instead, no overhead :
Still arguably on the slippy side of "start adding runtime checks for broken setups we can't really pretend to support", though...
Comment #9
yched CreditAttribution: yched commented@sun: note that the problem here is not actually that the widget is missing, but that the field type is missing :-)
Comment #10
yched CreditAttribution: yched commentedOK, let's do this then.
Will also catch cases of field types failing to provide a default_[formatter|widget].
Comment #12
yched CreditAttribution: yched commented#10: field_plugins_exception-1864014-10.patch queued for re-testing.
Comment #14
yched CreditAttribution: yched commented#10: field_plugins_exception-1864014-10.patch queued for re-testing.
Comment #15
sunThanks! Looks good to me.
Comment #16
sunThat said, I hope this does not conflict with these two issues?
#1751210: Convert URL alias form element into a field and field widget
#1825044: Turn contact form submissions into full-blown Contact Message entities, without storage
As mentioned in #1849724: Avoid critical Field API limitations, [EDIT] both issues are adding a field that only has a widget.
It looks like this code here runs very early when the widget/formatter plugin is retrieved from the instance and immediately bails out when there is no plugin.
So let me first test this change with both of those issues.
Comment #17
sun#10: field_plugins_exception-1864014-10.patch queued for re-testing.
Comment #17.0
sunUpdated issue summary.
Comment #18
AlbertLamme CreditAttribution: AlbertLamme commentedIn the patch they spoke about a directory of 'type' in the 'plugin' directory. I can only see a 'views' directory. Do i miss something?
Comment #19
jhedstromComment #20
ankitgarg CreditAttribution: ankitgarg commentedNo valid formatter Exception Added from patch.
Comment #21
jhedstromThis exception is thrown after returning, thus the code would never be executed.
Also some trailing whitespace.
I wonder if it would be possible to write a test for this?
Comment #22
XanoWe recently introduced a test to verify field plugin definition integrity. This patch extends that with checks for default widgets and formatters. Apparently there are quite a few incorrect definitions, which means this patch is supposed to fail.
Also removing the plugin system tag, as this is issue is specific to Field API.
Comment #24
XanoWhat the patch from #22 shows is that core contains several items for which we may indeed not want to provide widgets, such as
CreatedItem
andChangedItem
. Unless we change this, not providing a default widget/formatter or there not being any available widgets/formatters for a field type at all cannot really be classified an error and as such not justify exceptions being thrown.Comment #25
XanoComment #26
XanoI just discussed this with @yched at the Drupal Dev Days and our conclusion was that widgets and formatters should not be required for field types. It is good practice to provide widgets and formatters for every field type in core and contrib, but we do not want to burden developers of client projects with having to write dummy widgets and formatters for their custom field types.
Comment #31
lauriiiComment #32
heddnre #26: There is not default widget or formatter for MapItem. And the documentation for that field is a little vague, considering I can't see where it is even used in core. Otherwise, #2552799: FieldType with no available widget causes Fatal error seems to have picked up adding a default plugin for all other field types.
Comment #33
heddnResetting status. Not sure why/how that got changed.
Comment #35
lauriiiIt seems like Drupal\views\Tests\Update\EntityViewsDataUpdateTest had 22 fails. At least locally this passes and this patch shouldn't have any effect for that.
Comment #37
XanoBack to RTBC as per #31 and #33.
Comment #38
alexpottSeems like a good thing to have test coverage of. Committed 968bc41 and pushed to 8.0.x. Thanks!