Page MenuHomePhabricator

onResourceLoaderGetConfigVars can not depend on user-specific info for wikidata settings
Closed, ResolvedPublic

Description

onResourceLoaderGetConfigVars can not depend (indirectly) on isBetaGroupMember .

onResourceLoaderGetConfigVars must be 100% static. It can not depend on:

  • Current page
  • Current user
  • Current request

(This should probably be clarified in onResourceLoaderGetConfigVars and getWikibaseStaticConfigVars docs. Also, "vary with the html" does not sound right to me. It should not depend on which page and thus which HTML is requested.)

In this case, you can just check client-side using wgMFMode .

Trace is:

2016-07-26 17:34:20 [V5efHApEEaoAAHgCVmoAAAAH] deployment-mediawiki01 cawiki 1.28.0-alpha exception ERROR: [V5efHApEEaoAAHgCVmoAAAAH] /w/load.php?debug=false&lang=ca&modules=startup&only=scripts&skin=vector   BadMethodCallException from line 1025 of /srv/mediawiki/php-master/includes/session/SessionManager.php: Sessions are disabled for this entry point {"exception_id":"V5efHApEEaoAAHgCVmoAAAAH"} 
[Exception BadMethodCallException] (/srv/mediawiki/php-master/includes/session/SessionManager.php:1025) Sessions are disabled for this entry point
  #0 /srv/mediawiki/php-master/includes/session/SessionManager.php(187): MediaWiki\Session\SessionManager->getSessionFromInfo(MediaWiki\Session\SessionInfo, WebRequest)
  #1 /srv/mediawiki/php-master/includes/WebRequest.php(699): MediaWiki\Session\SessionManager->getSessionForRequest(WebRequest)
  #2 /srv/mediawiki/php-master/includes/user/User.php(1197): WebRequest->getSession()
  #3 /srv/mediawiki/php-master/includes/user/User.php(403): User->loadFromSession()
  #4 /srv/mediawiki/php-master/includes/user/User.php(2082): User->load()
  #5 /srv/mediawiki/php-master/includes/user/User.php(3476): User->getId()
  #6 /srv/mediawiki/php-master/includes/user/User.php(3484): User->isLoggedIn()
  #7 /srv/mediawiki/php-master/extensions/MobileFrontend/includes/MobileContext.php(327): User->isAnon()
  #8 /srv/mediawiki/php-master/extensions/MobileFrontend/includes/MobileContext.php(379): MobileContext->getMobileMode()
  #9 /srv/mediawiki/php-master/extensions/MobileFrontend/includes/MobileContext.php(1207): MobileContext->isBetaGroupMember()
  #10 /srv/mediawiki/php-master/extensions/MobileFrontend/includes/MobileFrontend.hooks.php(473): MobileContext->shouldShowWikibaseDescriptions(string)
  #11 /srv/mediawiki/php-master/extensions/MobileFrontend/includes/MobileFrontend.hooks.php(458): MobileFrontendHooks::getWikibaseStaticConfigVars(MobileContext)
  #12 /srv/mediawiki/php-master/includes/Hooks.php(195): MobileFrontendHooks::onResourceLoaderGetConfigVars(array)
  #13 /srv/mediawiki/php-master/includes/resourceloader/ResourceLoaderStartUpModule.php(117): Hooks::run(string, array)
  #14 /srv/mediawiki/php-master/includes/resourceloader/ResourceLoaderStartUpModule.php(375): ResourceLoaderStartUpModule->getConfigSettings(DerivativeResourceLoaderContext)
  #15 /srv/mediawiki/php-master/includes/resourceloader/ResourceLoaderModule.php(713): ResourceLoaderStartUpModule->getDefinitionSummary(DerivativeResourceLoaderContext)
  #16 /srv/mediawiki/php-master/includes/resourceloader/ResourceLoader.php(621): ResourceLoaderModule->getVersionHash(DerivativeResourceLoaderContext)
  #17 [internal function]: Closure$ResourceLoader::getCombinedVersion(string)
  #18 /srv/mediawiki/php-master/includes/resourceloader/ResourceLoader.php(622): array_map(Closure$ResourceLoader::getCombinedVersion;1768148240, array)
  #19 /srv/mediawiki/php-master/includes/resourceloader/ResourceLoader.php(674): ResourceLoader->getCombinedVersion(ResourceLoaderContext, array)
  #20 /srv/mediawiki/php-master/load.php(46): ResourceLoader->respond(ResourceLoaderContext)
  #21 /srv/mediawiki/w/load.php(3): include(string)
  #22 {main}

Caused by rEMFR880bc9644a75: Always show Wikidata descriptions in beta mode.

See also T127860: MobileFrontend using ResourceLoaderGetConfigVars hook with User dependent data.

Details

Related Gerrit Patches:
mediawiki/extensions/MobileFrontend : masterRevert "Always show Wikidata descriptions in beta mode"
mediawiki/extensions/MobileFrontend : wmf/1.28.0-wmf.12Revert "Always show Wikidata descriptions in beta mode"

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJul 26 2016, 5:58 PM

Change 301310 had a related patch set uploaded (by Jdlrobson):
Revert "Always show Wikidata descriptions in beta mode"

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

@Mattflaschen-WMF do we need to SWAT deploy the above fix?

This is also documented on https://www.mediawiki.org/wiki/Manual:Hooks/ResourceLoaderGetConfigVars and in hooks.txt. Including about using MakeGlobalVariablesScript instead if it is request-specific (user or page).

The underlying reason is that they come from a separate (static) web request. As such, aside from not being able to very by page, they are also not cached with the page they will update immediately and "affect" older pages).

Likewise, the request-specific export is cached in the page, and as such will not retroactively affect older pages. If the exported data is used by pre-existing modules, the newer version of that module must also support the older pages.

Change 301312 had a related patch set uploaded (by Reedy):
Revert "Always show Wikidata descriptions in beta mode"

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

Yes, the "isBetaGroupMember" part, but see my note in the original report about "vary with the html". That doesn't sound right.

@Mattflaschen-WMF do we need to SWAT deploy the above fix?

@Reedy is deploying it now.

Change 301312 merged by jenkins-bot:
Revert "Always show Wikidata descriptions in beta mode"

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

Mentioned in SAL [2016-07-26T23:59:04Z] <reedy@tin> Synchronized php-1.28.0-wmf.12/extensions/MobileFrontend/: Deploy revert for group0 for T141386 (duration: 00m 30s)

Change 301310 merged by jenkins-bot:
Revert "Always show Wikidata descriptions in beta mode"

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

phuedx triaged this task as Unbreak Now! priority.Jul 27 2016, 12:24 PM
Restricted Application added subscribers: Luke081515, TerraCodes. · View Herald TranscriptJul 27 2016, 12:24 PM
MBinder_WMF set the point value for this task to 0.5.Jul 27 2016, 5:13 PM
MBinder_WMF removed the point value for this task.
greg added a subscriber: greg.Jul 27 2016, 7:06 PM

@Mattflaschen-WMF do we need to SWAT deploy the above fix?

@Reedy is deploying it now.

Just for recording keeping: because of ^^ that means this should no longer block the wmf.12 release rolling to group1 (non-wikipedias) today, right?

Since the revert has been merged and deployed is this still blocking the train?

Reedy added a comment.Jul 27 2016, 7:53 PM

No reason it should block

greg added a comment.Jul 27 2016, 8:35 PM

@Krinkle: do you disagree that this shouldn't block the deployment (iow: do you think it should)?

@Krinkle: do you disagree that this shouldn't block the deployment (iow: do you think it should)?

My mistake. I thought it was only reverted in wmf.12 and not in master. I meant to agree on this weeks unblock and added to next week, but undone now.

phuedx closed this task as Resolved.Jul 29 2016, 10:19 AM

(Per the conversation above.)

mmodell changed the subtype of this task from "Task" to "Production Error".Aug 28 2019, 11:10 PM