dev/core#3502 - CiviEventDispatcher - Softer errors for not-ready. More comments.
#23739
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
(Standard links)
|
|
Just noting to be careful with versions here - our process is to merge to the rc & then back-port to stable. This is against stable (& the rc is not yet forked) |
totten
changed the title
not-ready. More comments.not-ready. More comments.
|
@eileenmcnaughton OK, I fixed my issue with the logging level and retested. Looks better. Squashed. Opened RC PR as #23785. |
|
I just tried this code and it still gives me the error commented here. |
|
@francescbassas That's very strange. With this patch, at a minimum, the prose of the message should have changed, eg Maybe search the filesystem (eg |
|
@totten sorry. I must have misapplied the patch. Now when I clean drupal caches I get the following warnings but page reloads well. |
OK, so it is an improvement. I'll go ahead and merge for 5.50/5.51 on that basis. That list is really long. Curious about why your system produces so many more than mine. Maybe it somehow depends on the specific list of But in any event, this is better than status-quo. |
|
@totten I think Campaign extension is throwing some warnings but I think that a Drupal module (Print PDF) is also throwing a lot of warnings. I don't use rules. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Overview
v5.50 is more likely to raise complaints about hooks that fire during bootstrap (as in the case of https://lab.civicrm.org/dev/core/-/issues/3502). This patch tries to turn-down the volume on such errors.
(This is the 5.50-stable variant of the patch. RC patch is #23785.)
Before
After
trigger_error(...E_USER_WARNING))error_log()) with backtrace.Comments
Two use-cases:
The easiest way to trigger an error is to hack
boot()function, eg\CRM_Utils_Hook::singleton()->commonBuildModuleList('civicrm_boot'); +\CRM_Utils_Hook::entityTypes($x); $bootServices['dispatcher.boot']->setDispatchPolicy($mainDispatchPolicy);I also managed to [reproduce the same error as @francescbassas](https://lab.civicrm.org/dev/core/-/issues/3502#note_75272. Activating
ruleswasn't enough -- I needed also needed a rule to specifically react to logs.Trying out these two cases with each way of recording the error, I'm a little torn:Witherror_log(), it was really quiet. There was no hint of the problem -- unless you dig into server logs. But I do think it's good to have it in the server log (eg to get backtrace).Withtrigger_error(), it's... somewhat softer, but not as much as I hoped. Case#1definitely softened. However, case#2still produced a crash -- becausetrigger_error()still activates Drupal watchdog (watchdog=>rules=>entity=>civicrm_entity=>hook).UPDATE: My last comment was probably misplaced - eg I had used
trigger_error(...E_USER_ERROR), which caused a full stop (sometimes). Changing totrigger_error(...E_USER_WARNING)seems to produce more desirable+consistent behavior (ie in all configurations that I tested on D7+WP, it showed the warning to the user - but continued executing).