Uncaught exception in Nodes and Members tabs in rc3

I had same error message, your quick fix worked as well for me, thank you!

Anyone knows the cause of this? One similar occurrence is here when the group data is exposed via API https://www.drupal.org/project/group/issues/2906082#comment-12818713

I also saved my custom view again as a workaround fix.

Thanks

this is a change record which explains: https://www.drupal.org/node/3061215

Same here, #3 worked for me.

Did any of you run the database update like you're supposed to after every module upgrade? :)

@kristiaanvandeneynde: I just upgraded, as we are upgrading with a special script which ALWAYS does a "drush updb" at the end I know for sure that database updates have been executed. Nevertheless I got the error. #3 worked for me, though. So only a little shock moment after the upgrade ;-)

Hmm, if someone has exact steps to reproduce after running updb/cr, please let me know.

That's odd because the update hooks are supposed to remove the use of "cache_context.group_membership.roles.permissions" from those views :/

I just did a search on my system. There are other modules that call group_membership.roles.permissionss.

grep group_membership.roles.permissions -d recurse modules
grep group_membership.roles.permissions -d recurse profiles

There were several modules that I had installed that did not have the upgrade. For now I had to downgrade to rc2 and I added issues to those modules that this change in group rc3 has occurred.

I am also on Drupal 8.7.3

https://www.drupal.org/node/3061215

Re #13: That context was removed because it was deemed insecure. I'd urge the maintainers of those modules to upgrade quickly :)

Same here, #3 worked for me.

Okay, must be something wrong with the update function then. Thanks for the detailed report!

Ran into the same thing today, seems to have started after our last update 4 days ago. Caches were cleared and update.php was run after the updates.

Full error message:
Symfony\Component\DependencyInjection\Exception\ServiceNotFoundException: You have requested a non-existent service "cache_context.group_membership.roles.permissions". Did you mean one of these: "cache_context.group_membership.audience", "cache_context.group_membership.roles"? in Drupal\Component\DependencyInjection\Container->get() (line 153 of ./web/core/lib/Drupal/Component/DependencyInjection/Container.php).

I was able to access group/1/members and group/1/nodes, so the fix in #3 didn't work for me. The problem was reaching the main group pages (view tab), "website encountered an unexpected error...". Using the same technique in #3 on a couple of our custom views did the trick.

In my case, I did a search of the database for 'group_membership.roles.permissions' , and found a custom view referencing it. I turned caching off, saved the view, then re-enabled caching, and its working now.

I am maintaining one of the modules referenced in #13, and I'm working on an update function to cover the affected views. If you would like, I can forward it to y'all either for a general update, or as instructions for other maintainers of Group-integrated modules.

Hello kristiaanvandeneynde,
Yes I did run the database update after the install of RC3.
Thanks for the module. It is very useful.

I did an export using the "configuration syncronization", and I search in the *.yml files the string "group_membership.roles.permissions" to find the list of views that I must save again

Attached patch adds a post_update hook to remove the 'group_membership.roles.permissions' cache context from all existing views.

// Remove the deleted cache context and add the updated metadata.
if ($key = array_search('group_membership.roles.permissions', $cache_metadata['contexts'])) {
  unset($cache_metadata['tags'][$key]);
}

The earlier updates failed since array_search returned 0 could return 0 and the $key was unset from the wrong array key 'tags' instead of 'contexts'.

Bumping to critical because this leads to a fatal error after doing the update.

Code looks good and verified that it clears up the fail and the view is updated correctly.

Oh crap, I usually make a habit out of if (array_search() !== FALSE), don't know why I missed that here. Thanks for the patch! Will try to review and commit early next week. I'm not pushing a new release on a Friday at 4PM.

@idebr why the post update hook, though? Changing views is perfectly safe and probably much faster from a regular update hook.

post_update FTW, always post_update unless there is a reason you can't use it (Schema updates et al.), safer because the API is fully stable and zero chance of merging two patches that both declare the same hook update number, so less error prone and less re-rolling needed.

Just my 2 cents obviously :)

Hmm, I'm more of a purist. Config changes go into update hooks, content changes in post-update. Merge conflicts happen, so I'm not too bothered by them, but I can see the upside of always using post-update hooks. Then again, I can also see some downsides because of how it works.

Either way, how about this? It fixes the update hooks for people who haven't upgraded yet and introduces a new one for people who have and are experiencing problems.

Not a fan of rewriting update hooks personally because it is sort of rewriting history (some people will have run a different version of that hook then is actually in code, which could lead to major confusion should something unforeseen happen), I would just leave the old hook as it is, but maybe I am a purist in that :)

The problem is that update hooks are run in order. So if you have a broken one and don't fix it, then someone upgrading from RC2 -> RC4 will still run the wrong code. By fixing/emptying the old one, you make sure people won't run bad update code.

Anyways: Let's see what people think of this patch. Will try to run it myself sometime tomorrow or Wednesday.

We tested this patch yesterday and found it did not resolve issues on a site which was already showing the error 'You have requested a non-existent service "cache_context.group_membership.roles.permissions".'

I did test additionally edit => save on the affected views, but wasn't able to get the site back. Due to time constraints I wasn't able to debug further and rolled back to rc2 instead.

Since I don't have info to add yet, I don't want to move this back to "Needs work", but I wanted to give at least some signal here that it didn't work as hoped when I tried it. Plan to test further and give more informed feedback then.

#33 The cache context might be used in other places than Views. Can you provide a backtrace where the cache context is called to see if this something we can fix in Group?

OK, second time around I was more careful. Seems to have worked, testing the project's full migration process against rc3 now. Guessing last time I didn't update enough of the group-referencing views is all.

I edit-saved about a dozen views that referenced groups (those that matched entity_type: group or entity_type: group_content) on the site. However when I ran a config:export to sync, I saw that almost all views on the site (48 of 54 views) had the same change applied, adding route.group and user.group_permissions to the view contexts for a variety of views through the site, the majority of which do not reference entities group or group_content.

Doesn't seem like a major problem, might be expected behaviour, sharing in case it's of interest.

Thanks!

Good catch, that's a bug. The old update code targeted some views specifically, the new one doesn't so it can't assume it needs to add the new context. Only when a view had the old context, should we add the new one. Good catch!

This only changes views that had the old context.

The patch in #30 fixed the error for me. Thanks!

@phjou current patch is #37

Unsure about the test bot failures, "mkdir failed" looks like infra things not test things, retesting.

So the only remaining possible problem I see here is #34, usage of the removed cache context outside Views.

Possible solutions:

• Revert the deletion and properly @deprecated the old context, remove it later (core style)
• Update the update hook to remove it from all config (don't see how that would work, but didn't dig into it)
• Accept that this maybe breaks some sites and move forward

Reading the original CR for this https://www.drupal.org/node/3061215 it seems that option three was the chosen solution for this, so in that light, #34 should not be blocking this.

Can confirm that #3 fixes the issue for me also.

I have to remain on rc2.
I had to clear the cache
and #3 did work.

The patch in #30 fixed the error for me as well. Thanks!

Can confirm that #3 fixes the issue for me, too. Is it necessary to install group-3061321-37.patch or should I wait for RC4?

#3 has not worked for me and in fact threw up additional incomprehensible messages which were replicate when I tried again after clearing the cache. The following error persists
AssertionError: assert($this->assertValidTokens($context_tokens)) in assert() (line 103 of core/lib/Drupal/Core/Cache/Context/CacheContextsManager.php).
I'm pretty new to this Drupal business and it's impossible for me to understand from this thread what to do. I'm reverting to RC2 and will wait for RC4 - will it be fixed then? Should I be applying the patch in #37?

The patch in #37 works for me

@Lendude in #43 we chose to do this the hard way because it was a security issue waiting to happen. Sometimes, tough decisions need to be made :/

Pushing ahead with the patch from #37.

@kristiaanvandeneynde++ for making the tough decisions. Thanks for committing this.

The fix committed here doesn't take into account exported/default config in other modules. It would be worth moving to config presave so that it catches those too - then the update just has to load and resave the config without doing any specific manipulation. There are some similar examples in views already.

Re-opening for discussion. Here's what Views does: https://api.drupal.org/api/drupal/core%21modules%21views%21views.module/...

Here's a patch implementing #53.

Makes sense to me: this will take core also of views that have not been installed yet but are still relying on the old cache context.

A minor nit, if we happen to reroll the patch:

+++ b/group.post_update.php
@@ -45,3 +46,15 @@ function group_post_update_grant_access_overview_permission() {
+  \Drupal::classResolver(ConfigEntityUpdater::class)->update($sandbox, 'view', function($view) {

Missing space after function keyword.

I updated the patch to also clean out the other update functions, fixed spacing issues and a bug because $update = FALSE; was part of the loop instead of outside of it. Please review. Messed up the interdiff locally but patch is small enough as is.

Some nits from me, but I like the approach.

1. +++ b/group.module
   @@ -573,3 +574,35 @@ function _group_content_enabler_manager() {
   +/**
   + * Implements hook_view_presave().
   + *
   + * @todo Remove in 8.2.x if it still only fixes cache contexts by then.
   + */
   +function group_view_presave(ViewEntityInterface $view) {
   

   Technically this is hook_ENTITY_TYPE_presave() that typehints EntityInterface.

2. +++ b/group.module
   @@ -573,3 +574,35 @@ function _group_content_enabler_manager() {
   +        $cache_metadata['max-age'] = -1;
   

   Could be \Drupal\Core\Cache\CacheBackendInterface::CACHE_PERMANENT

Fixing nits in #58. Since those are minor changes, going ahead and re-RTBCing.

Cheers, back to fixed. Removed the unused ViewsInterface use statement.