Page MenuHomePhabricator
Create Task
Maniphest T189842

Undefined index error in CachingResultsBuilder when checking constraints on empty entity
Closed, ResolvedPublicPRODUCTION ERROR
Actions

Description

If an entity has no statements, then we don’t have any CheckResults to store in the result array (not even NullResults), so CheckingResultsBuilder returns a completely empty array. This is already a violation of the wbcheckconstraints API, I think (consumers should probably be able to expect the entity ID and an empty claims array, at least), but it also results in an error in CachingResultsBuilder when trying to access the results for that entity to cache them:

Notice: Undefined index: Q50580715 in /srv/mediawiki/php-1.31.0-wmf.25/extensions/WikibaseQualityConstraints/src/Api/CachingResultsBuilder.php on line 242

Not sure what the best solution for this will be… probably some kind of NullContextCursor (only entity ID, no statement etc.) that we can plug into the NullResult we already have from T178160?

Patch-For-Review (two chains):

Details

SubjectRepoBranchLines +/-
Revert "Work around CheckingResultsBuilder bug"mediawiki/extensions/WikibaseQualityConstraintsmaster+0 -4
Add EntityContextCursormediawiki/extensions/WikibaseQualityConstraintsmaster+162 -5
Ensure that result is populated even for empty entitiesmediawiki/extensions/WikibaseQualityConstraintsmaster+130 -41
Replace NullResult’s Context with ContextCursormediawiki/extensions/WikibaseQualityConstraintsmaster+10 -12
Work around CheckingResultsBuilder bugmediawiki/extensions/WikibaseQualityConstraintsmaster+4 -0
Customize query in gerrit

Related Objects

Event Timeline

Reedy created this task.Mar 15 2018, 11:41 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptMar 15 2018, 11:41 PM
Lydia_Pintscher added a subscriber: Lucas_Werkmeister_WMDE.Mar 16 2018, 8:38 AM
Lucas_Werkmeister_WMDE added a project: Wikibase-Quality.Mar 16 2018, 11:18 AM
Lucas_Werkmeister_WMDE renamed this task from Notice: Undefined index: Q50580715 to Undefined index error in CachingResultsBuilder when checking constraints on empty entity.Mar 16 2018, 11:40 AM
Lucas_Werkmeister_WMDE updated the task description. (Show Details)
gerritbot subscribed.Mar 16 2018, 11:46 AM

Change 420010 had a related patch set uploaded (by Lucas Werkmeister (WMDE); owner: Lucas Werkmeister (WMDE)):
[mediawiki/extensions/WikibaseQualityConstraints@master] Work around CheckingResultsBuilder bug

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

gerritbot added a project: Patch-For-Review.Mar 16 2018, 11:46 AM
Lucas_Werkmeister_WMDE added a comment.Mar 16 2018, 11:47 AM

I uploaded a workaround to fix the PHP warning at least; do you think this is serious enough to warrant a backport?

Lucas_Werkmeister_WMDE updated the task description. (Show Details)Mar 16 2018, 3:08 PM
Lucas_Werkmeister_WMDE claimed this task.Mar 19 2018, 12:13 PM
Lucas_Werkmeister_WMDE added a project: Wikidata-Ministry-Of-Magic.
Lucas_Werkmeister_WMDE moved this task from Tasks to In Progress on the Wikidata-Ministry-Of-Magic board.
Lucas_Werkmeister_WMDE added a comment.Mar 19 2018, 12:33 PM

This is already a violation of the wbcheckconstraints API, I think (consumers should probably be able to expect the entity ID and an empty claims array, at least)

Well, not quite… we actually work around this in CheckConstraints.php itself, so the API end-user never sees this :) but this fix belongs into the proper constraint checking machinery, not tacked onto it in the API wrapper like that, IMHO.

gerritbot added a comment.Mar 19 2018, 1:08 PM

Change 420319 had a related patch set uploaded (by Lucas Werkmeister (WMDE); owner: Lucas Werkmeister (WMDE)):
[mediawiki/extensions/WikibaseQualityConstraints@master] Replace NullResult’s Context with ContextCursor

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

gerritbot added a comment.Mar 19 2018, 1:08 PM

Change 420320 had a related patch set uploaded (by Lucas Werkmeister (WMDE); owner: Lucas Werkmeister (WMDE)):
[mediawiki/extensions/WikibaseQualityConstraints@master] Add EntityContextCursor

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

gerritbot added a comment.Mar 19 2018, 1:08 PM

Change 420321 had a related patch set uploaded (by Lucas Werkmeister (WMDE); owner: Lucas Werkmeister (WMDE)):
[mediawiki/extensions/WikibaseQualityConstraints@master] Ensure that result is populated even for empty entities

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

Lucas_Werkmeister_WMDE updated the task description. (Show Details)Mar 19 2018, 1:09 PM
Lucas_Werkmeister_WMDE moved this task from In Progress to Needs Review on the Wikidata-Ministry-Of-Magic board.
gerritbot added a comment.Mar 21 2018, 10:55 AM

Change 420010 merged by jenkins-bot:
[mediawiki/extensions/WikibaseQualityConstraints@master] Work around CheckingResultsBuilder bug

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

ReleaseTaggerBot added a project: MW-1.31-release-notes (WMF-deploy-2018-03-27 (1.31.0-wmf.27)).Mar 21 2018, 11:00 AM
gerritbot added a comment.Mar 21 2018, 11:00 AM

Change 420319 merged by jenkins-bot:
[mediawiki/extensions/WikibaseQualityConstraints@master] Replace NullResult’s Context with ContextCursor

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

• Jonas moved this task from Needs Review to Blocked on the Wikidata-Ministry-Of-Magic board.Mar 21 2018, 11:01 AM
gerritbot added a comment.Mar 21 2018, 11:03 AM

Change 420320 merged by jenkins-bot:
[mediawiki/extensions/WikibaseQualityConstraints@master] Add EntityContextCursor

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

gerritbot added a comment.Mar 21 2018, 11:13 AM

Change 420321 merged by jenkins-bot:
[mediawiki/extensions/WikibaseQualityConstraints@master] Ensure that result is populated even for empty entities

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

Lucas_Werkmeister_WMDE added a subscriber: • Jonas.Mar 21 2018, 11:29 AM

@Jonas why is this in the “blocked” column?

gerritbot added a comment.Mar 21 2018, 11:31 AM

Change 420993 had a related patch set uploaded (by Lucas Werkmeister (WMDE); owner: Lucas Werkmeister (WMDE)):
[mediawiki/extensions/WikibaseQualityConstraints@master] Revert "Work around CheckingResultsBuilder bug"

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

Lucas_Werkmeister_WMDE moved this task from Blocked to Needs Review on the Wikidata-Ministry-Of-Magic board.Mar 21 2018, 11:31 AM

Pushed a new commit anyways, so moving back to “needs review”.

• Jonas moved this task from Needs Review to Done on the Wikidata-Ministry-Of-Magic board.Mar 21 2018, 12:26 PM
thiemowmde subscribed.Mar 26 2018, 1:09 PM

Guys, please communicate. This back-and-forth does not look like fun.

gerritbot added a comment.Mar 26 2018, 2:36 PM

Change 420993 abandoned by Lucas Werkmeister (WMDE):
Revert "Work around CheckingResultsBuilder bug"

Reason:
CachingResultsBuilder has been removed and we never copied this workaround to its successor, CachingResultsSource.

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

Lucas_Werkmeister_WMDE closed this task as Resolved.Mar 26 2018, 2:39 PM

I think this is resolved now, we’ve done a lot of refactorings in this area anyways (see T189593). The proper fix is still in place (though there are some more fixes for semi-related issues in T190684), and the workaround was removed as part of the refactoring already.

Lucas_Werkmeister_WMDE mentioned this in T190955: Undefined index in extensions/WikibaseQualityConstraints/src/Api/CachingResultsBuilder.php on line 242.Mar 28 2018, 6:22 PM
• mmodell merged a task: T190955: Undefined index in extensions/WikibaseQualityConstraints/src/Api/CachingResultsBuilder.php on line 242.Mar 28 2018, 6:25 PM
• mmodell subscribed.
• mmodell changed the subtype of this task from "Task" to "Production Error".Aug 28 2019, 11:09 PM
Maintenance_bot removed a project: Patch-For-Review.Aug 28 2019, 11:22 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