Page MenuHomePhabricator
Create Task
Maniphest T177524

"User::loadFromSession called before the end of Setup.php" due to a combination of TwoColConflict and BetaFeatures
Closed, ResolvedPublicPRODUCTION ERROR
Actions

Description

I don't know which extension to blame for this one, so I'll tag all of them.

During user auto-creation, AbuseFilter calls SpecialPage::getTitleFor. During the hook for that method to allow extensions to add special pages, TwoColConflictHooks::onSpecialPage_initList calls BetaFeatures::isFeatureEnabled which attempts to load the user from session before it's ready.

One of the following must be true:

  • SpecialPage::getTitleFor is not safe to call during auto-creation. Although that seems somewhat restrictive.
  • BetaFeatures::isFeatureEnabled is not safe to call during auto-creation. Extensions should register their special pages without that conditional.
  • BetaFeatures::isFeatureEnabled needs to somehow do its job without checking the session-user.
#0 /srv/mediawiki/php-1.31.0-wmf.2/includes/user/User.php(2233): User->load()
#1 /srv/mediawiki/php-1.31.0-wmf.2/includes/user/User.php(5277): User->getId()
#2 /srv/mediawiki/php-1.31.0-wmf.2/includes/user/User.php(2887): User->loadOptions()
#3 /srv/mediawiki/php-1.31.0-wmf.2/extensions/BetaFeatures/includes/BetaFeaturesUtil.php(43): User->getOption(string)
#4 /srv/mediawiki/php-1.31.0-wmf.2/extensions/TwoColConflict/includes/TwoColConflictHooks.php(124): BetaFeatures::isFeatureEnabled(User, string)
#5 /srv/mediawiki/php-1.31.0-wmf.2/includes/Hooks.php(177): TwoColConflictHooks::onSpecialPage_initList(array)
#6 /srv/mediawiki/php-1.31.0-wmf.2/includes/Hooks.php(205): Hooks::callHook(string, array, array, NULL)
#7 /srv/mediawiki/php-1.31.0-wmf.2/includes/specialpage/SpecialPageFactory.php(268): Hooks::run(string, array)
#8 /srv/mediawiki/php-1.31.0-wmf.2/includes/specialpage/SpecialPageFactory.php(285): SpecialPageFactory::getPageList()
#9 /srv/mediawiki/php-1.31.0-wmf.2/includes/specialpage/SpecialPageFactory.php(660): SpecialPageFactory::getAliasList()
#10 /srv/mediawiki/php-1.31.0-wmf.2/includes/specialpage/SpecialPage.php(98): SpecialPageFactory::getLocalNameFor(string, boolean)
#11 /srv/mediawiki/php-1.31.0-wmf.2/includes/specialpage/SpecialPage.php(85): SpecialPage::getTitleValueFor(string, boolean, string)
#12 /srv/mediawiki/php-1.31.0-wmf.2/extensions/AbuseFilter/includes/AbuseFilterPreAuthenticationProvider.php(41): SpecialPage::getTitleFor(string)
#13 /srv/mediawiki/php-1.31.0-wmf.2/extensions/AbuseFilter/includes/AbuseFilterPreAuthenticationProvider.php(13): AbuseFilterPreAuthenticationProvider->testUser(User, User, boolean)
#14 /srv/mediawiki/php-1.31.0-wmf.2/includes/auth/AuthManager.php(1649): AbuseFilterPreAuthenticationProvider->testUserForCreation(User, string, array)
#15 /srv/mediawiki/php-1.31.0-wmf.2/includes/Setup.php(869): MediaWiki\Auth\AuthManager->autoCreateUser(User, string, boolean)
#16 /srv/mediawiki/php-1.31.0-wmf.2/includes/WebStart.php(114): include(string)
#17 /srv/mediawiki/php-1.31.0-wmf.2/api.php(38): include(string)
#18 /srv/mediawiki/w/api.php(3): include(string)
#19 {main}

Alternative trace:

#0 /srv/mediawiki/php-1.31.0-wmf.2/includes/user/User.php(5269): User->load()
#1 /srv/mediawiki/php-1.31.0-wmf.2/includes/user/User.php(2887): User->loadOptions()
#2 /srv/mediawiki/php-1.31.0-wmf.2/extensions/BetaFeatures/includes/BetaFeaturesUtil.php(43): User->getOption(string)
#3 /srv/mediawiki/php-1.31.0-wmf.2/extensions/TwoColConflict/includes/TwoColConflictHooks.php(128): BetaFeatures::isFeatureEnabled(User, string)
#4 /srv/mediawiki/php-1.31.0-wmf.2/includes/Hooks.php(177): TwoColConflictHooks::onSpecialPage_initList(array)
#5 /srv/mediawiki/php-1.31.0-wmf.2/includes/Hooks.php(205): Hooks::callHook(string, array, array, NULL)
#6 /srv/mediawiki/php-1.31.0-wmf.2/includes/specialpage/SpecialPageFactory.php(268): Hooks::run(string, array)
#7 /srv/mediawiki/php-1.31.0-wmf.2/includes/specialpage/SpecialPageFactory.php(285): SpecialPageFactory::getPageList()
#8 /srv/mediawiki/php-1.31.0-wmf.2/includes/specialpage/SpecialPageFactory.php(660): SpecialPageFactory::getAliasList()
#9 /srv/mediawiki/php-1.31.0-wmf.2/includes/specialpage/SpecialPage.php(98): SpecialPageFactory::getLocalNameFor(string, string)
#10 /srv/mediawiki/php-1.31.0-wmf.2/includes/specialpage/SpecialPage.php(85): SpecialPage::getTitleValueFor(string, string, string)
#11 /srv/mediawiki/php-1.31.0-wmf.2/extensions/ShortUrl/ShortUrl.hooks.php(23): SpecialPage::getTitleFor(string, string)
#12 /srv/mediawiki/php-1.31.0-wmf.2/includes/Hooks.php(177): ShortUrlHooks::setupUrlRouting(PathRouter)
#13 /srv/mediawiki/php-1.31.0-wmf.2/includes/Hooks.php(205): Hooks::callHook(string, array, array, NULL)
#14 /srv/mediawiki/php-1.31.0-wmf.2/includes/WebRequest.php(171): Hooks::run(string, array)
#15 /srv/mediawiki/php-1.31.0-wmf.2/includes/WebRequest.php(316): WebRequest::getPathInfo(string)
#16 /srv/mediawiki/php-1.31.0-wmf.2/includes/Setup.php(721): WebRequest->interpolateTitle()
#17 /srv/mediawiki/php-1.31.0-wmf.2/includes/WebStart.php(114): include(string)
#18 /srv/mediawiki/php-1.31.0-wmf.2/index.php(40): include(string)
#19 /srv/mediawiki/w/index.php(3): include(string)
#20 {main}

Details

SubjectRepoBranchLines +/-
Revert "Dont register simulate page when not enabled as a BF"mediawiki/extensions/TwoColConflictmaster+4 -36
Revert "Dont register simulate page when not enabled as a BF"mediawiki/extensions/TwoColConflictwmf/1.31.0-wmf.2+3 -35
Customize query in gerrit

Related Objects
Search...

Event Timeline

Anomie created this task.Oct 5 2017, 6:30 PM
Restricted Application added a project: TCB-Team (now WMDE-TechWish). · View Herald TranscriptOct 5 2017, 6:30 PM
Restricted Application added a subscriber: Aklapper. · View Herald Transcript
Anomie added a comment.Oct 5 2017, 7:16 PM

I see the TwoColConflict/BetaFeatures combination is also hitting the "Sessions are disabled for this entry point" exception in load.php:

#0 /srv/mediawiki/php-1.31.0-wmf.2/includes/session/SessionManager.php(309): MediaWiki\Session\SessionManager->getSessionFromInfo(MediaWiki\Session\SessionInfo, WebRequest)
#1 /srv/mediawiki/php-1.31.0-wmf.2/includes/session/SessionManager.php(243): MediaWiki\Session\SessionManager->getEmptySessionInternal(WebRequest)
#2 /srv/mediawiki/php-1.31.0-wmf.2/includes/session/SessionManager.php(193): MediaWiki\Session\SessionManager->getEmptySession(WebRequest)
#3 /srv/mediawiki/php-1.31.0-wmf.2/includes/WebRequest.php(735): MediaWiki\Session\SessionManager->getSessionForRequest(WebRequest)
#4 /srv/mediawiki/php-1.31.0-wmf.2/includes/user/User.php(1231): WebRequest->getSession()
#5 /srv/mediawiki/php-1.31.0-wmf.2/includes/user/User.php(408): User->loadFromSession()
#6 /srv/mediawiki/php-1.31.0-wmf.2/includes/user/User.php(5269): User->load()
#7 /srv/mediawiki/php-1.31.0-wmf.2/includes/user/User.php(2887): User->loadOptions()
#8 /srv/mediawiki/php-1.31.0-wmf.2/extensions/BetaFeatures/includes/BetaFeaturesUtil.php(43): User->getOption(string)
#9 /srv/mediawiki/php-1.31.0-wmf.2/extensions/TwoColConflict/includes/TwoColConflictHooks.php(124): BetaFeatures::isFeatureEnabled(User, string)
#10 /srv/mediawiki/php-1.31.0-wmf.2/includes/Hooks.php(177): TwoColConflictHooks::onSpecialPage_initList(array)
#11 /srv/mediawiki/php-1.31.0-wmf.2/includes/Hooks.php(205): Hooks::callHook(string, array, array, NULL)
#12 /srv/mediawiki/php-1.31.0-wmf.2/includes/specialpage/SpecialPageFactory.php(268): Hooks::run(string, array)
#13 /srv/mediawiki/php-1.31.0-wmf.2/includes/specialpage/SpecialPageFactory.php(285): SpecialPageFactory::getPageList()
#14 /srv/mediawiki/php-1.31.0-wmf.2/includes/specialpage/SpecialPageFactory.php(660): SpecialPageFactory::getAliasList()
#15 /srv/mediawiki/php-1.31.0-wmf.2/includes/specialpage/SpecialPage.php(98): SpecialPageFactory::getLocalNameFor(string, boolean)
#16 /srv/mediawiki/php-1.31.0-wmf.2/includes/specialpage/SpecialPage.php(85): SpecialPage::getTitleValueFor(string, boolean, string)
#17 /srv/mediawiki/php-1.31.0-wmf.2/extensions/VisualEditor/VisualEditor.hooks.php(846): SpecialPage::getTitleFor(string)
#18 /srv/mediawiki/php-1.31.0-wmf.2/includes/Hooks.php(177): VisualEditorHooks::onResourceLoaderGetConfigVars(array)
#19 /srv/mediawiki/php-1.31.0-wmf.2/includes/Hooks.php(205): Hooks::callHook(string, array, array, NULL)
#20 /srv/mediawiki/php-1.31.0-wmf.2/includes/resourceloader/ResourceLoaderStartUpModule.php(118): Hooks::run(string, array)
#21 /srv/mediawiki/php-1.31.0-wmf.2/includes/resourceloader/ResourceLoaderStartUpModule.php(403): ResourceLoaderStartUpModule->getConfigSettings(DerivativeResourceLoaderContext)
#22 /srv/mediawiki/php-1.31.0-wmf.2/includes/resourceloader/ResourceLoaderModule.php(844): ResourceLoaderStartUpModule->getDefinitionSummary(DerivativeResourceLoaderContext)
#23 /srv/mediawiki/php-1.31.0-wmf.2/includes/resourceloader/ResourceLoader.php(667): ResourceLoaderModule->getVersionHash(DerivativeResourceLoaderContext)
#24 [internal function]: Closure$ResourceLoader::getCombinedVersion(string)
#25 /srv/mediawiki/php-1.31.0-wmf.2/includes/resourceloader/ResourceLoader.php(680): array_map(Closure$ResourceLoader::getCombinedVersion;1712, array)
#26 /srv/mediawiki/php-1.31.0-wmf.2/includes/resourceloader/ResourceLoader.php(760): ResourceLoader->getCombinedVersion(ResourceLoaderContext, array)
#27 /srv/mediawiki/php-1.31.0-wmf.2/load.php(53): ResourceLoader->respond(ResourceLoaderContext)
#28 /srv/mediawiki/w/load.php(3): include(string)
#29 {main}
Nikerabbit raised the priority of this task from Medium to Unbreak Now!.Oct 6 2017, 11:59 AM
Nikerabbit subscribed.

My guess is that this is causing user interface language selection to fail on hi.wikipedia.org and other sites, but for example on gu.wikipedia.org where I don't see this exception with mwdebug. End result seems to be the same as in T177478: Unable to change user settings except this is currently in production (and weekend is coming).

Restricted Application added subscribers: Liuxinyu970226, Jay8g, TerraCodes. · View Herald TranscriptOct 6 2017, 11:59 AM
Nikerabbit removed a project: AbuseFilter.Oct 6 2017, 12:06 PM
Nikerabbit added a subscriber: greg.

I also see a trace with ShortUrl instead of AbuseFilter, hence removing it as probably unrelated.

Nikerabbit mentioned this in T177478: Unable to change user settings.Oct 6 2017, 12:09 PM
Nikerabbit updated the task description. (Show Details)Oct 6 2017, 12:15 PM
Nikerabbit added a comment.Oct 6 2017, 12:24 PM

It looks like rMWb9146ba1d932 didn't fully solve the issue, but I'm not sure what the original issue was that caused the first revert. Maybe reverting that again would solve this.

Instead of conditionally registering the special page, could it just be marked as unlisted or listed depending whether the beta feature is enabled, and show an error if it is not supposed to be accessed?

Nikerabbit renamed this task from "User::loadFromSession called before the end of Setup.php" due to a combination of AbuseFilter, TwoColConflict, and BetaFeatures to "User::loadFromSession called before the end of Setup.php" due to a combination of TwoColConflict and BetaFeatures.Oct 6 2017, 12:27 PM
Addshore subscribed.Oct 6 2017, 2:17 PM
Addshore added a comment.Oct 6 2017, 2:20 PM

I think this might be a dupe of T177468 or at least the issues described and covered in that?
This is all fixed up on master now.

Anomie added a comment.EditedOct 6 2017, 2:27 PM

No, T177468 is about ->getUser() methods returning null. In this task, the ->getUser() methods are returning a valid User object, but it's attempting to load that user object while $user->isSafeToLoad() returns false.

The title and description of the task refer to situations where $user->isSafeToLoad() returns false because setup has not yet completed, while T177524#3662027 refers to situations where $user->isSafeToLoad() returns false because the endpoint does not have any session to load a user from.

It looks like your patch https://gerrit.wikimedia.org/r/#/c/382453/ fixed the no-session case, but the main case here is still occurring (see https://logstash.wikimedia.org/goto/51715162dbd687ade2f9a6f08824bcc9).

Nikerabbit's suggestion of always registering the special page and changing its listed status based on the beta feature toggle is probably the more reliable solution.

Trizek-WMF subscribed.Oct 6 2017, 2:37 PM
Capankajsmilyo subscribed.Oct 6 2017, 4:51 PM
Jdforrester-WMF subscribed.Oct 6 2017, 5:07 PM
Jdforrester-WMF added a subtask: T177590: New filters for edit review can't be opt-out on some wikis.Oct 6 2017, 5:25 PM
Jdforrester-WMF added a parent task: T174358: 1.31.0-wmf.2 deployment blockers.Oct 6 2017, 5:46 PM
greg added a comment.Oct 6 2017, 5:51 PM

What is the user facing impact of this?

Jdforrester-WMF added a comment.Oct 6 2017, 5:56 PM
In T177524#3665377, @greg wrote:

What is the user facing impact of this?

Users unable to change any preferences on some (?) wikis; made particularly acute by the roll-out from beta to default of the new RecentChanges tool, as complained about in T177590. (This is speculative causation.)

Anomie added a comment.Oct 6 2017, 6:11 PM
In T177524#3665377, @greg wrote:

What is the user facing impact of this?

I personally don't know of any user-facing impact, but I can't say there isn't one. It doesn't help that TwoColConflict had a different bug in T177468 and the attempted removal of StubUserLang caused T177478 at the same time.

gerritbot subscribed.Oct 6 2017, 6:14 PM

Change 382753 had a related patch set uploaded (by Catrope; owner: Catrope):
[mediawiki/extensions/TwoColConflict@master] Revert "Dont register simulate page when not enabled as a BF"

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

gerritbot added a project: Patch-For-Review.Oct 6 2017, 6:14 PM
Catrope mentioned this in rESCC2ec9c09e3239: Revert "Dont register simulate page when not enabled as a BF".Oct 6 2017, 6:14 PM
Catrope subscribed.Oct 6 2017, 6:17 PM

Registering a special page conditionally based on a user preference seems shady to me anyway. I've put up a patch to revert this behavior from master.

What confuses me, though, is that the wmf.2 backport of the patch that introduced this behavior, has already been reverted in wmf.2. I'll keep digging.

Catrope added a comment.Oct 6 2017, 6:19 PM

Aha, it's because it was reapplied later. Let's revert that.

gerritbot added a comment.Oct 6 2017, 6:19 PM

Change 382756 had a related patch set uploaded (by Catrope; owner: Catrope):
[mediawiki/extensions/TwoColConflict@wmf/1.31.0-wmf.2] Revert "Dont register simulate page when not enabled as a BF"

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

Catrope mentioned this in rESCC78b39382a57a: Revert "Dont register simulate page when not enabled as a BF".Oct 6 2017, 6:20 PM
gerritbot added a comment.Oct 6 2017, 6:27 PM

Change 382756 merged by jenkins-bot:
[mediawiki/extensions/TwoColConflict@wmf/1.31.0-wmf.2] Revert "Dont register simulate page when not enabled as a BF"

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

greg added a parent task: T174359: 1.31.0-wmf.3 deployment blockers.Oct 6 2017, 6:28 PM
Catrope assigned this task to Addshore.Oct 6 2017, 6:28 PM
Restricted Application added a project: User-Addshore. · View Herald TranscriptOct 6 2017, 6:28 PM
Stashbot subscribed.Oct 6 2017, 6:38 PM

Mentioned in SAL (#wikimedia-operations) [2017-10-06T18:38:58Z] <catrope@tin> Synchronized php-1.31.0-wmf.2/extensions/TwoColConflict/: Unbreak preference changes (T177524) (duration: 00m 47s)

Catrope added a comment.Oct 6 2017, 6:40 PM

My revert fixes this in production (wmf.2). We'll still have to merge my revert in master or do something else about this problem, otherwise it'll recur in wmf.3

ReleaseTaggerBot added a project: MW-1.31-release-notes (WMF-deploy-2017-10-03 (1.31.0-wmf.2)).Oct 6 2017, 7:00 PM
BukhariSaeed closed subtask T177590: New filters for edit review can't be opt-out on some wikis as Resolved.Oct 7 2017, 5:14 AM
• mmodell lowered the priority of this task from Unbreak Now! to Medium.Oct 8 2017, 9:13 AM
gerritbot added a comment.Oct 9 2017, 6:17 AM

Change 382753 merged by jenkins-bot:
[mediawiki/extensions/TwoColConflict@master] Revert "Dont register simulate page when not enabled as a BF"

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

ReleaseTaggerBot edited projects, added MW-1.31-release-notes (WMF-deploy-2017-10-10 (1.31.0-wmf.3)); removed MW-1.31-release-notes (WMF-deploy-2017-10-03 (1.31.0-wmf.2)).Oct 9 2017, 7:01 AM
Addshore closed this task as Resolved.Oct 9 2017, 3:47 PM

It looks like the hook that registered the special page could have been fixed by checking both $wgFullyInitialised and the existence of MW_NO_SESSION.
Although I havn't checked when wgFullyInitialised is actually set and if that is before of after the hook call on a regular index.php hit.

We are going to take a different approach and show a note on the special page when the BF is disabled rather than not register the page in the first place so we won't be reverting the revert and fixing. T177758

As this is reverted on branches & on master I'll mark as closed.

Addshore moved this task from Unsorted 💣 to Closing ✔️ on the User-Addshore board.Oct 9 2017, 3:47 PM
Addshore added a project: WMDE-QWERTY-Sprint-2017-09-19.
Addshore moved this task from Sprint Backlog to Done on the WMDE-QWERTY-Sprint-2017-09-19 board.
Anomie added a comment.Oct 10 2017, 1:25 AM
In T177524#3669767, @Addshore wrote:

It looks like the hook that registered the special page could have been fixed by checking both $wgFullyInitialised and the existence of MW_NO_SESSION.

Doing that might have wound up with the special page never being registered on wikis with the ShortUrl extension enabled.

A more correct solution would have been to check ->isSafeToLoad() on the session User object ($wgUser or RequestContext::getMain()->getUser()).

Although I havn't checked when wgFullyInitialised is actually set and if that is before of after the hook call on a regular index.php hit.

It's set at the end of Setup.php.

Tobi_WMDE_SW moved this task from Incoming to Edit Conflict Handling on the TCB-Team (now WMDE-TechWish) board.Oct 10 2017, 9:36 AM
Liuxinyu970226 unsubscribed.Oct 10 2017, 3:04 PM
Tobi_WMDE_SW added a project: WMDE-TechWish.Oct 17 2017, 1:07 PM
Tobi_WMDE_SW moved this task from Proposed to Done on the WMDE-TechWish board.Oct 17 2017, 1:09 PM
Tobi_WMDE_SW moved this task from Done to Demoed on the WMDE-TechWish board.Oct 17 2017, 2:11 PM
• mmodell changed the subtype of this task from "Task" to "Production Error".Aug 28 2019, 11:10 PM
Aklapper removed a subscriber: Anomie.Oct 16 2020, 5:42 PM
thiemowmde removed a project: TCB-Team (now WMDE-TechWish).Jan 7 2022, 3:29 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