Page MenuHomePhabricator
Create Task
Maniphest T356895

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

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

Details

Related Changes in Gerrit:
SubjectRepoBranchLines +/-
Use real anonymous user in ComputedUserImpactLookupmediawiki/extensions/GrowthExperimentswmf/1.42.0-wmf.17+4 -4
Use real anonymous user in ComputedUserImpactLookupmediawiki/extensions/GrowthExperimentsmaster+4 -4
Customize query in gerrit

Related Objects

Event Timeline

Etonkovidova created this task.Feb 7 2024, 5:26 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptFeb 7 2024, 5:26 PM
Urbanecm_WMF claimed this task.Feb 7 2024, 5:30 PM
Urbanecm_WMF triaged this task as Unbreak Now! priority.
Urbanecm_WMF edited projects, added Growth-Team (Sprint 7 (Growth Team)); removed Growth-Team.
Urbanecm_WMF added a comment.Feb 7 2024, 5:34 PM

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.

Urbanecm_WMF added a parent task: T354435: 1.42.0-wmf.17 deployment blockers.Feb 7 2024, 5:37 PM

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.

Zabe subscribed.Feb 7 2024, 6:14 PM

Dupe of T356741?

Urbanecm_WMF merged a task: T356741: InvalidArgumentException: Cannot create a user with no name, no ID, and no actor ID.Feb 7 2024, 6:16 PM
Urbanecm_WMF added subscribers: kostajh, Urbanecm.
Restricted Application removed a subscriber: Urbanecm. · View Herald TranscriptFeb 7 2024, 6:16 PM
Urbanecm_WMF added a comment.Feb 7 2024, 7:12 PM

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.

brennen subscribed.Feb 7 2024, 7:28 PM
Novem_Linguae subscribed.Feb 7 2024, 9:20 PM
Tgr subscribed.Feb 7 2024, 11:08 PM

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

Tgr added a comment.Feb 7 2024, 11:18 PM
In T356895#9522300, @Urbanecm_WMF wrote:

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).

gerritbot added a comment.Feb 7 2024, 11:21 PM

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

gerritbot added a project: Patch-For-Review.Feb 7 2024, 11:21 PM
brennen mentioned this in T354435: 1.42.0-wmf.17 deployment blockers.Feb 7 2024, 11:35 PM
gerritbot added a comment.Feb 8 2024, 6:54 AM

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

gerritbot added a comment.Feb 8 2024, 7:22 AM

Change 998455 merged by jenkins-bot:

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

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

gerritbot added a comment.Feb 8 2024, 8:27 AM

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

Stashbot added a comment.Feb 8 2024, 8:29 AM

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)]]

Maintenance_bot removed a project: Patch-For-Review.Feb 8 2024, 8:30 AM
Stashbot added a comment.Feb 8 2024, 8:37 AM

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)

ReleaseTaggerBot added a project: MW-1.42-notes (1.42.0-wmf.17; 2024-02-06).Feb 8 2024, 9:01 AM
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.

Urbanecm_WMF removed a parent task: T354435: 1.42.0-wmf.17 deployment blockers.Feb 8 2024, 1:54 PM
In T356895#9523142, @Tgr wrote:
In T356895#9522300, @Urbanecm_WMF wrote:

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.

Urbanecm_WMF mentioned this in T356998: UserIdentityValue::newAnonymous and UserIdentityValue::__construct should not generate invalid anonymous users.Feb 8 2024, 2:01 PM
Etonkovidova closed this task as Resolved.Feb 8 2024, 6:40 PM
Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL · Credits