Page MenuHomePhabricator

[wmf.17 - testwiki] Impact module: Temporary delay in getting your information
Closed, ResolvedPublicBUG REPORT

Description

Steps to replicate the issue (include links if applicable):

  • On testwiki wmf.17 go to Special:Homepage

What happens?:

  • the Impact module displays "Temporary delay in getting your information"

Screen Shot 2024-02-07 at 9.24.31 AM.png (1×688 px, 93 KB)

What should have happened instead?:

  • the Impact module displays all info correctly (as in wmf.16

Software version (skip for WMF-hosted wikis like Wikipedia):

  • wmf.17

Event Timeline

Urbanecm_WMF triaged this task as Unbreak Now! priority.

This is happening because https://test.wikipedia.org/w/rest.php/growthexperiments/v0/user-impact/%2329926?lang=en errors out hard. Appears to be conditional defaults related in some way. Looking.

This fully breaks one of the Growth team features (Impact module) and generates a lot of production errors:

InvalidArgumentException: Cannot create a user with no name, no ID, and no actor ID

from /srv/mediawiki/php-1.42.0-wmf.17/includes/user/UserFactory.php(265)
#0 /srv/mediawiki/php-1.42.0-wmf.17/includes/user/UserFactory.php(204): MediaWiki\User\UserFactory->newFromAnyId(NULL, NULL, NULL)
#1 /srv/mediawiki/php-1.42.0-wmf.17/includes/user/Registration/LocalUserRegistrationProvider.php(26): MediaWiki\User\UserFactory->newFromUserIdentity(MediaWiki\User\UserIdentityValue)
#2 /srv/mediawiki/php-1.42.0-wmf.17/includes/user/Registration/UserRegistrationLookup.php(82): MediaWiki\User\Registration\LocalUserRegistrationProvider->fetchRegistration(MediaWiki\User\UserIdentityValue)
#3 /srv/mediawiki/php-1.42.0-wmf.17/includes/user/Options/ConditionalDefaultsLookup.php(121): MediaWiki\User\Registration\UserRegistrationLookup->getRegistration(MediaWiki\User\UserIdentityValue)
#4 /srv/mediawiki/php-1.42.0-wmf.17/includes/user/Options/ConditionalDefaultsLookup.php(93): MediaWiki\User\Options\ConditionalDefaultsLookup->checkConditionForUser(MediaWiki\User\UserIdentityValue, array)
#5 /srv/mediawiki/php-1.42.0-wmf.17/includes/user/Options/ConditionalDefaultsLookup.php(76): MediaWiki\User\Options\ConditionalDefaultsLookup->checkConditionsForUser(MediaWiki\User\UserIdentityValue, array)
#6 /srv/mediawiki/php-1.42.0-wmf.17/includes/user/Options/DefaultOptionsLookup.php(141): MediaWiki\User\Options\ConditionalDefaultsLookup->getOptionDefaultForUser(string, MediaWiki\User\UserIdentityValue)
#7 /srv/mediawiki/php-1.42.0-wmf.17/includes/user/Options/UserOptionsManager.php(629): MediaWiki\User\Options\DefaultOptionsLookup->getDefaultOptions(MediaWiki\User\UserIdentityValue)
#8 /srv/mediawiki/php-1.42.0-wmf.17/includes/user/Options/UserOptionsManager.php(532): MediaWiki\User\Options\UserOptionsManager->loadOriginalOptions(MediaWiki\User\UserIdentityValue, integer, NULL)
#9 /srv/mediawiki/php-1.42.0-wmf.17/includes/user/Options/UserOptionsManager.php(159): MediaWiki\User\Options\UserOptionsManager->loadUserOptions(MediaWiki\User\UserIdentityValue, integer)
#10 /srv/mediawiki/php-1.42.0-wmf.17/includes/utils/MWTimestamp.php(68): MediaWiki\User\Options\UserOptionsManager->getOption(MediaWiki\User\UserIdentityValue, string)
#11 /srv/mediawiki/php-1.42.0-wmf.17/extensions/GrowthExperiments/includes/UserImpact/ComputedUserImpactLookup.php(295): MediaWiki\Utils\MWTimestamp->offsetForUser(MediaWiki\User\UserIdentityValue)
#12 /srv/mediawiki/php-1.42.0-wmf.17/extensions/GrowthExperiments/includes/UserImpact/ComputedUserImpactLookup.php(165): GrowthExperiments\UserImpact\ComputedUserImpactLookup->getEditData(MediaWiki\User\User, integer)
#13 /srv/mediawiki/php-1.42.0-wmf.17/extensions/GrowthExperiments/includes/Rest/Handler/UserImpactHandler.php(123): GrowthExperiments\UserImpact\ComputedUserImpactLookup->getExpensiveUserImpact(MediaWiki\User\User)
#14 /srv/mediawiki/php-1.42.0-wmf.17/extensions/GrowthExperiments/includes/Rest/Handler/UserImpactHandler.php(68): GrowthExperiments\Rest\Handler\UserImpactHandler->getUserImpact(MediaWiki\User\UserIdentityValue)
#15 /srv/mediawiki/php-1.42.0-wmf.17/includes/Rest/SimpleHandler.php(40): GrowthExperiments\Rest\Handler\UserImpactHandler->run(MediaWiki\User\UserIdentityValue)
#16 /srv/mediawiki/php-1.42.0-wmf.17/includes/Rest/Router.php(561): MediaWiki\Rest\SimpleHandler->execute()
#17 /srv/mediawiki/php-1.42.0-wmf.17/includes/Rest/Router.php(452): MediaWiki\Rest\Router->executeHandler(GrowthExperiments\Rest\Handler\UserImpactHandler)
#18 /srv/mediawiki/php-1.42.0-wmf.17/includes/Rest/EntryPoint.php(195): MediaWiki\Rest\Router->execute(MediaWiki\Rest\RequestFromGlobals)
#19 /srv/mediawiki/php-1.42.0-wmf.17/includes/Rest/EntryPoint.php(135): MediaWiki\Rest\EntryPoint->execute()
#20 /srv/mediawiki/php-1.42.0-wmf.17/rest.php(31): MediaWiki\Rest\EntryPoint::main()
#21 /srv/mediawiki/w/rest.php(3): require(string)
#22 {main}

I'm fairly certain this is going to cause issues once train moves to group1 (or at the very last, group2). Marking as a blocker.

Good news: I know what is happening here:

  1. ComputedUserImpactLookup::getEditData calls MWTimestamp::offsetForUser with new UserIdentityValue( 0, '' ).
  2. MWTimestamp uses UserOptionsLookup::getOption, which uses ConditionalDefaultsLookup thanks to the conditional defaults work.
  3. Conditional defaults need to know the registration date (to evaluate CUDCOND_AFTER), so it loops in UserRegistrationLookup, which asks LocalUserRegistrationProvider to provide the local (per-wiki) user registration timestamp.
  4. Until this point, a UserIdentity was enough (and as such, the plain UserIdentityValue was enough as well). Because T352871 is not yet finished, which means LocalUserRegistrationProvider is currently a fancy wrapper for User::getRegistration, we now need to convert UserIdentity to the full User object.
  5. UserFactory::newFromUserIdentity does not consider UserIdentityValue where ID is zero and name is empty to be valid, hence it complains.

Possible solutions

Good question :). We definitely need to make UserIdentityValue complain when an invalid value object is about to be created (aka both fields empty). We also need to replace it with something that is valid in GrowthExperiments (and possibly other places).

I am unsure why conditional defaults are consulted here, since timecorrection should not use conditional defaults as of now.

In the short term, just use new User() (which is a valid anonymous identity)?

I am unsure why conditional defaults are consulted here, since timecorrection should not use conditional defaults as of now.

UserOptionsManager loads all the options of the user all the time (which is why you see UserOptionsManager->loadUserOptions in the stack trace - that will fetch all option rows from the DB and all the defaults from the default lookup, which will involve ConditionalDefaultsLookup iterating through everything that has a conditionally default value).

Change 998455 had a related patch set uploaded (by Gergő Tisza; author: Gergő Tisza):

[mediawiki/extensions/GrowthExperiments@master] Use real anonymous user in ComputedUserImpactLookup

https://gerrit.wikimedia.org/r/998455

Change 998676 had a related patch set uploaded (by Kosta Harlan; author: Gergő Tisza):

[mediawiki/extensions/GrowthExperiments@wmf/1.42.0-wmf.17] Use real anonymous user in ComputedUserImpactLookup

https://gerrit.wikimedia.org/r/998676

Change 998455 merged by jenkins-bot:

[mediawiki/extensions/GrowthExperiments@master] Use real anonymous user in ComputedUserImpactLookup

https://gerrit.wikimedia.org/r/998455

Change 998676 merged by jenkins-bot:

[mediawiki/extensions/GrowthExperiments@wmf/1.42.0-wmf.17] Use real anonymous user in ComputedUserImpactLookup

https://gerrit.wikimedia.org/r/998676

Mentioned in SAL (#wikimedia-operations) [2024-02-08T08:29:18Z] <urbanecm@deploy2002> Started scap: Backport for [[gerrit:998676|Use real anonymous user in ComputedUserImpactLookup (T356895)]]

Mentioned in SAL (#wikimedia-operations) [2024-02-08T08:37:08Z] <urbanecm@deploy2002> Finished scap: Backport for [[gerrit:998676|Use real anonymous user in ComputedUserImpactLookup (T356895)]] (duration: 07m 49s)

Urbanecm_WMF lowered the priority of this task from Unbreak Now! to High.Feb 8 2024, 1:53 PM
Urbanecm_WMF moved this task from Incoming to QA on the Growth-Team (Sprint 7 (Growth Team)) board.

Looks fixed on testwiki. Moving to QA and de-classifying as a blocker.

I am unsure why conditional defaults are consulted here, since timecorrection should not use conditional defaults as of now.

UserOptionsManager loads all the options of the user all the time (which is why you see UserOptionsManager->loadUserOptions in the stack trace - that will fetch all option rows from the DB and all the defaults from the default lookup, which will involve ConditionalDefaultsLookup iterating through everything that has a conditionally default value).

Ahh, of course, makes sense. So this bug would only show when conditional defaults are enabled in any way, but it would show for any user property.