Steps:
1) I enabled a custom module
2) after getting this error: Fatal error: Unsupported operand types in rules.module on line 227
3) I applied the Patch #15 https://www.drupal.org/node/2161847#comment-8875651
4) For some reason, then I'm getting this error:
Fatal error: Call to undefined function entity_load_multiple_by_name() in rules.module on line 807
Undoing the Patch or disabling the custom module, does not impact anything?
I even tried running the update.php (to clear the registry) but still no success.
I'm using Entity 7.x-1.5 which is the latest version.
Anybody else ran into this issue? And how to resolve this?
I think it seems that something breaks the proper module loading, which causes the Entity classes not being loaded.
Comment | File | Size | Author |
---|---|---|---|
#37 | rules-invocation-2324587-35.patch | 4.17 KB | das-peter |
#35 | interdiff-2324587-32-35-do-not-test.diff | 1.82 KB | das-peter |
Comments
Comment #1
thim CreditAttribution: thim commentedI have no explanation for the solution.
I undid the patch and removed some watchdog() i had set in another module.
Removing the patch, resolved:
Fatal error: Call to undefined function entity_load_multiple_by_name() in rules.module on line 807
And it turns out that in some situations that when the watchdog() is used in hook_boot() that an error pops up:
Fatal error: Unsupported operand types in rules.module on line 227
Removing the watchdogs, resolved:
Fatal error: Unsupported operand types in rules.module on line 227
It even got weirder, since afterwards I turned on the watchdogs again and the system didnt report any errors anymore.
Comment #2
thim CreditAttribution: thim commentedUpdate: only when adding a new module this error appears when using the watchdog function in the hook_boot
Fatal error: Unsupported operand types in rules.module on line 227
Comment #3
SproetS CreditAttribution: SproetS commentedManually empty cache and cache_* tables in the database helped me.
Comment #4
geek-merlinCame here from #2161847: Fatal error: Unsupported operand types in rules.module on line 227...
Tried debugging to syslog
or to watchdog
but when i had my spies in place i could not reproduce. Maybe this helps someone else.
Comment #5
geek-merlinOK gotcha: So we have:
* somthing throws a notice in early bootstrap (here language negotiation)
* rules fires a watchdog event
* and does some recuild - but it's too early for this!
Setting to critical, this causes hard to track WSODs.
Comment #6
joelpittet@axel.rutz Thanks for the debug line: I ran your debug and this is the backtrack I got.
Comment #7
joelpittetSo mine is language negotiation related too it seems.
Maybe I've configured the hook wrong (my worry).
Comment #8
capogeannis CreditAttribution: capogeannis commentedFor me personally, I encountered this error only when turning on and off modules, after applying the operands patch to fix the prior error. My site got the WSOD. Clear cache via Drush and all is well.
The fix in my case was to move a php plugin class object I was using in my custom module in conjunction with a function local to that module out of the boot hook and into init. Adjusting the weight of the module etc didn't seem to change anything.
Comment #9
fagoRe-titling to reflect the general problem.
We've already got rules_event_invocation_enabled() in HEAD, so what about disabling it by default and only running Rules evaluation only *after* the initial hook init call?
Comment #10
fagoSo what about a patch like that?
Comment #11
geek-merlinCool, i did not notice we already have that.
The patch in #10 looks solid. Lights are green.
THe biggest problem i can imagine is to miss rules invocations we might want.
So i support getting this in thus ffixing a major WTF and amending later if we need.
A minor issue is to document that behavior, i have no proposal where.
Comment #12
joelpittetI've had this applied on a high traffic site that just went into production for a couple of days. So far so good, thanks @fago.
Comment #13
AlfTheCat CreditAttribution: AlfTheCat commentedAwesome, I'd missed this one too. I have two sites (one high trafficed) where this issue appears. Will test Monday. Thanks everyone!
Comment #14
fubhy CreditAttribution: fubhy commentedThe patch fixes the problem mentioned in this issue, however wouldn't it be better to actually check the current bootstrap phase? Why do we need a custom switch?
Comment #15
scripthead CreditAttribution: scripthead commentedThis patch fails against 7.x-2.7. Here are the rejects
Comment #16
marcelovaniI am using this patch for Drupal core
https://www.drupal.org/files/drupal-1081266-102-drupal_get_filename-D7.p...
see https://www.drupal.org/node/1285856 which uses watchdog().
When I clear the cache, the watchdog() triggers hook_watchdog() which is implemented by Rules module.
Rules then tries to perform some database actions, but since it is too early in the bootstrap, it will generate a fatal error.
I am relying on this patch and it will soon become a major issue if they commit that patch in drupal core.
The patch in #10 works fine and I would recommend applying it immediately.
Comment #17
fago@fubhy: The switch is already there, not invented here. Committed #10.
Comment #20
bojanz CreditAttribution: bojanz commentedThis broke Commerce completely, because we use %commerce_order in our menu paths, and loading the order triggers an event which refreshes the order.
This appears to be done before hook_init(), so it's no longer happening. Here's my explanation of the process in a previous issue:
https://www.drupal.org/node/2120421#comment-8176391
Comment #21
klausiInteresting, commerce_cart_commerce_order_load() looks suspicious. Firing events when an order entity is just loaded sounds dangerous to me. Instead of implementing hook_commerce_order_load() commerce_cart should refresh orders explicitly on its page callbacks. Maybe that is not possible for some reason? Unfortunately the docs on commerce_cart_commerce_order_load() do not tell why this is done in an entity load hook.
Comment #22
bojanz CreditAttribution: bojanz commented@klausi
Commerce relies on the fact that the loaded order is always up to date. It has worked that way since the 2010 alphas, and changing it now would be a big BC break.
Comment #23
klausiHm, we cannot roll this back since you get fatal errors in the early Drupal bootstrap when Rules is invoked without a database. We cannot change commerce_cart_commerce_order_load() for backwards compatibility either.
Solution 1: introduce explicit commerce_cart_order_refresh() calls in commerce_cart module when they are actually needed (in page callbacks and whatever).
Solution 2: Check the current Drupal bootstrap phase as proposed by fubhy in Rules when rules_invoke_event() is called. If it is >= DRUPAL_BOOTSTRAP_LANGUAGE then we can actually invoke the event, otherwise we just dismiss it.
Comment #24
bojanz CreditAttribution: bojanz commentedAs I said, solution #1 is not an option, because other parts of the code (and other modules) also depend on the order being loaded refreshed.
Comment #25
Leeteq CreditAttribution: Leeteq commentedComment #26
fagoyeah, I see that changing commerce order behaviour aka solution #1 is not an option.
However, I do not think solution #2 is either as it does not seem to me that the language bootstrap is enough for rules - thus it would be wrong and possibly lead to other issue.
So what about just adding a hook menu implementation to workaround the problem of hook_menu() being executed before hook_init() and moving it up, such that we are one of the first hooks being called. Then, we can enable rules event evaluation there in addition?
Comment #27
klausiWhat do you mean by adding a hook menu implementation? Did you mean hook_boot()? hook_menu() is not invoked on runtime?
Comment #28
fagooh, no. I thought the issue is when building menu items, but it's actually in the menu item loader callbacks :-( So maybe doing the same within hook_menu_get_item_alter() would work then?
Comment #29
das-peter CreditAttribution: das-peter commentedI just came across this. Also having trouble with commerce.
Same finding here.
menu_set_custom_theme();
in_drupal_bootstrap_full()
triggers the whole chain of loading menu items, those loading entities and so on.How about enabling rules not in
hook_init()
but in the middle betweenhook_boot()
andhook_init()
?This is possible due the fancy
hook_stream_wrappers_alter()
.Comment #30
klausiThat is quite obscure. I think we should try hook_menu_get_item_alter() as fago proposed, which is called right before _menu_translate() entity loading in the menu system. And we need a doc block that tells why the heck we need it, describing the Commerce case as example.
Comment #31
fagoI agree with klausi - relying on just on the onobviously working stream alter would be quite confusing. If possible I'd prefer the hook_menu_get_item_alter way as well.
Comment #32
klausiPatch with hook_menu_get_item_alter() + test case.
Comment #33
das-peter CreditAttribution: das-peter commented@klausi Shouldn't we implement
hook_module_implements_alter()
to ensure the rules implementation is fired before others?rules_install()
even set's a high weight, so it's even more likely we're too late inhook_module_implements_alter()
.Comment #34
klausiThe order of hook_menu_get_item_alter() does not matter because it is called before _menu_translate() (which will invoke the menu object loading) in menu_get_item().
So what we could do in addition is check drupal_get_bootstrap_phase() in rules_menu_get_item_alter() before messing with rules_event_invocation_enabled(). Currently with the patch this will not work:
Comment #35
das-peter CreditAttribution: das-peter commentedNot sure if I agree, is it really just about
_menu_translate()
? What about another module that implementshook_menu_get_item_alter()
and which does entity loading?While I agree that this fixes the current issue, it's not a "longterm" solution in terms "what could go wrong".
Thus I think implementing
hook_menu_get_item_alter()
still would be a good idea.Agreed, how about the solution in the attached patch?
This is has become "quite complex" - I still like the
hook_stream_wrappers_alter()
approach. Looks about similar failure prone while having less code.Edit: Fixed formatting
Comment #37
das-peter CreditAttribution: das-peter commentedDarn, creating new files using git apply always inserts carriage returns on my system.
Re-rolled patch.
Comment #38
klausiWorks for me, although I don't think the ordering of hook_menu_item_alter() is important. There can also be other hooks like hook_boot() or any hook called before hook_menu_item_alter() in the bootstrap where firing events is disabled and developers want to do so while loading entities.
So regardless what we do this can only be a band aid around the bleeding use case of Commerce; I don't think that enabling Rules event invocation even earlier in hook_stream_wrappers() is a good idea since the path is not initialized for example; might lead to other problems.
Assigning to fago.
Comment #39
ledbelly2142 CreditAttribution: ledbelly2142 commentedFollowing
Comment #40
mthomason CreditAttribution: mthomason commentedThis seems like a case of changing default functionally in a minor release, to get the expected results: breaking anything that depends on it. First thing people notice is commerce.
If you are going to change default behavior, why not wait until a major release? Shouldn't this be recalled? The fix the commerce people are suggesting is to downgrade to Rules 2.7.
Comment #41
fagoI agree that the module implements alter is a good idea and that we are doing it in the right place here. I hope that there are no more other hooks invoked too early in the bootstrap (which I strictly speaking would consider as core bug), except for hook_boot() which is documented and intended as such.
The patch looks great, just found the following:
This seems to be useless and should be removed.
The "default behaviour" has led to various problems, that's why we improved it. I'd expect no further side-effects for this, so the benefits outweigh possible problems. But anyway, we usually wait ~1month without introducing any larger changes before tagging a release so that dev versions and changes can be tested upfront.
Comment #43
fagooh, figured what's rules_init static var is used for. Thus added a comment to it and commited it, thanks!
Comment #44
fagoComment #46
kenorb CreditAttribution: kenorb commentedComment #47
Fabianx CreditAttribution: Fabianx as a volunteer commentedI think watchdog should still be removed from the early bootstrap rules hooks, because:
Even with this change, entity_get_info() could be called after rules was enabled, but then if watchdog() is called, then rules_watchdog() and therefore entity_get_info() is being called again, which due to some other core problem can lead to an incomplete schema cache and ultimately to the site failing.
Therefore it would be great to allow three states:
- allow rules_watchdog just when hook_init() is done
watchdog() is just so low-level that that is very important.
In case there is worry that messages are missed, they could be collected till hook_init().
Comment #48
chrisfromredfinIt took me quite a while to finally deduce and realize this, but I'm being bitten by EXACTLY what Fabianx describes in #47.
entity_get_info is being called, and schemas are not "complete" - for example "node" has a "base table" of null under the schema_sql_fields.
I've got a watchdog() that's happening early in one of my custom modules. If I comment out the rules_invoke_event('watchdog') then everything works fine.
I can try and work on a patch if someone can point me in a direction, write some pseudo-code, etc.
Comment #49
Fabianx CreditAttribution: Fabianx as a volunteer commented#48:
The idea is simple (thought it might not be simple to understand):
- In rules_watchdog add:
- In rules_init:
and that should be it already.
One edge case is when the process terminates before hook_init(), though it might be questionable if its safe to invoke rules_watchdog then at all.
Comment #50
fagoI see - so the workaround to postpone watchdog-event stuff until the end of hook init is welcome. Also the approach outlined in #49 makes sense to me - patch welcome!
Comment #51
kenorb CreditAttribution: kenorb commented