Page MenuHomePhabricator

broken Lua's mw.wikibase.entity:getSitelink function
Closed, ResolvedPublic

Description

Since 12.12.19 there seems to be an issue with Lua's mw.wikibase.entity:getSitelink function on Commons. The category page with script errors started to fill up with files showing "Lua error in Module:Wikidata_label at line 24: attempt to call method 'getSitelink' (a nil value)" error.

This could be related to a change in the WikibaseMediaInfo custom Lua module for Commons. https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/WikibaseMediaInfo/+/553128/

Reported by a Wikidata user here

Details

Related Gerrit Patches:
mediawiki/extensions/WikibaseMediaInfo : wmf/1.35.0-wmf.10Override getSitelink in mediainfo table, instead of removing it
mediawiki/extensions/WikibaseMediaInfo : masterOverride getSitelink in mediainfo table, instead of removing it

Event Timeline

Restricted Application added subscribers: Liuxinyu970226, Aklapper. · View Herald TranscriptDec 12 2019, 12:04 PM

Module:Wikidata label’s getSitelink function (permalink) assumes that any entity has a getSitelink method:

-- use different sitelink call depending if you already have an entity or not
local function getSitelink(item, entity, lang)
	if entity then -- if we have entity than use it
		return entity:getSitelink(lang .. 'wiki') 
	else -- if no entity than use different function
		return mw.wikibase.sitelink( item, lang .. 'wiki' )
	end
end

With the new MediaInfo module, that’s no longer a valid assumption. (Come to think of it, we should probably remove the getSitelink method for Lexeme entities too, but that’s unrelated.) I’m not sure if the fix is an additional “does the method exist” check, though, or if getSitelink shouldn’t be called with a MediaInfo entity at all (as far as I understand, it would eventually have to return nil anyways).

Either way, I suspect this will have to be fixed on-wiki by the community editors, but they can’t very well do that when the new Lua module for MediaInfo entities hasn’t even been announced yet as far as I’m aware :)

Yes, it looks like the removal of getSitelink for MediaInfo entities probably caused this.
There's already a discussion about this on the module's talk page: https://commons.wikimedia.org/wiki/Module_talk:Wikidata_label#Lua_error_in_Module:Wikidata_label_at_line_24:_attempt_to_call_method_'getSitelink'_(a_nil_value).

Jarekt added a subscriber: Jarekt.Dec 12 2019, 6:40 PM

matthiasmullie and Lucas_Werkmeister_WMDE thanks for reply. I think I understand it now, so in the past we had the code with the same interface, documented in mw:Extension:Wikibase Client/Lua, that supported both processing of wikidata and SDC entities, and now we are moving in a now direction of having two different codes with different functions. That will make writing libraries that work with both Wikidata and SDC much harder.

I "fixed" the immediate line causing errors, and that seems to fix the pages. However the explanation still does not make much sense as the entities the module was operating on were Wikidata entities and not SDC's MediaInfo entities, and we should be able to look up sitelinks for them.

I believe that whatever I ended up doing to remove the getSitelink method from MediaInfo entities, accidentally took it away from the parent class as well. Am looking into it!

Change 556981 had a related patch set uploaded (by Matthias Mullie; owner: Matthias Mullie):
[mediawiki/extensions/WikibaseMediaInfo@master] Only remove methods in mediainfo table, not in parent wikibase table

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

I believe that whatever I ended up doing to remove the getSitelink method from MediaInfo entities, accidentally took it away from the parent class as well. Am looking into it!

Can we just keep getSitelink method, and make it return nil when applied to MediaInfo entities?

After my yesterday change to Module:Wikidata label, the 30k files affected got fixed. Now the only files in Category:Pages_with_script_errors are couple hundred Gauguin paintings. Lets not "fix" those files so we can keep on diagnosing the issue. I did try to isolate the issue and found the following:

The difference between the two files is that the first one has SDC statements and labels and the second one does not. So it seems like only files with SDC statements are affected by this issue.

Bugreporter triaged this task as Unbreak Now! priority.Dec 15 2019, 7:42 PM
Bugreporter added a project: Regression.

Can we just keep getSitelink method, and make it return nil when applied to MediaInfo entities?

Yes - bringing it back as a no-op (for now at least)

The difference between the two files is that the first one has SDC statements and labels and the second one does not. So it seems like only files with SDC statements are affected by this issue.

Yes, it appears that as soon as a MediaInfo entity gets created, the code to delete getSitelink gets run and has the unfortunate side-effect from deleting it in the parent metatable as well...

Change 556981 merged by jenkins-bot:
[mediawiki/extensions/WikibaseMediaInfo@master] Override getSitelink in mediainfo table, instead of removing it

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

Change 558039 had a related patch set uploaded (by Lucas Werkmeister (WMDE); owner: Matthias Mullie):
[mediawiki/extensions/WikibaseMediaInfo@wmf/1.35.0-wmf.10] Override getSitelink in mediainfo table, instead of removing it

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

Change 558039 merged by jenkins-bot:
[mediawiki/extensions/WikibaseMediaInfo@wmf/1.35.0-wmf.10] Override getSitelink in mediainfo table, instead of removing it

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

matthiasmullie closed this task as Resolved.Dec 16 2019, 7:30 PM
matthiasmullie claimed this task.

Fix got deployed - I believe this is done, but please do re-open the ticket should you still discover issues!

I purged all the files in Category:Pages_with_script_errors and there are no more errors in file namespace.

Cutmuetia1998 set Due Date to Sat, Jan 11, 5:00 PM.Sat, Jan 11, 8:17 PM
RhinosF1 closed this task as Resolved.Sat, Jan 11, 8:29 PM
RhinosF1 removed a subscriber: Cutmuetia1998.