PHP Notice: Undefined index: taskType - causes Argument 1 passed to GrowthExperiments\NewcomerTasks\NewcomerTasksChangeTagsManager::apply() must be of the type string, null given
Closed, ResolvedPublicPRODUCTION ERROR
Actions

Assigned To

Authored By

	brennen
	Mar 21 2022, 6:17 PM

Project Tags

Referenced Files

None

Subscribers

Description

Error

mwversion: 1.39.0-wmf.1
reqId: 65af0da5-d300-4131-a499-d4b36495f946
Find reqId in Logstash

normalized_message

[{reqId}] {exception_url}   PHP Notice: Undefined index: taskType

exception.trace

from /srv/mediawiki/php-1.39.0-wmf.1/extensions/GrowthExperiments/includes/VisualEditorHooks.php(140)
#0 /srv/mediawiki/php-1.39.0-wmf.1/extensions/GrowthExperiments/includes/VisualEditorHooks.php(140): MWExceptionHandler::handleError(integer, string, string, integer, array)
#1 /srv/mediawiki/php-1.39.0-wmf.1/includes/HookContainer/HookContainer.php(160): GrowthExperiments\VisualEditorHooks->onVisualEditorApiVisualEditorEditPostSave(MediaWiki\Page\PageIdentityValue, User, string, array, array, array, array)
#2 /srv/mediawiki/php-1.39.0-wmf.1/extensions/VisualEditor/includes/VisualEditorHookRunner.php(75): MediaWiki\HookContainer\HookContainer->run(string, array, array)
#3 /srv/mediawiki/php-1.39.0-wmf.1/extensions/VisualEditor/includes/ApiVisualEditorEdit.php(541): MediaWiki\Extension\VisualEditor\VisualEditorHookRunner->onVisualEditorApiVisualEditorEditPostSave(MediaWiki\Page\PageIdentityValue, User, string, array, array, array, array)
#4 /srv/mediawiki/php-1.39.0-wmf.1/includes/api/ApiMain.php(1893): MediaWiki\Extension\VisualEditor\ApiVisualEditorEdit->execute()
#5 /srv/mediawiki/php-1.39.0-wmf.1/includes/api/ApiMain.php(870): ApiMain->executeAction()
#6 /srv/mediawiki/php-1.39.0-wmf.1/includes/api/ApiMain.php(841): ApiMain->executeActionWithErrorHandling()
#7 /srv/mediawiki/php-1.39.0-wmf.1/api.php(90): ApiMain->execute()
#8 /srv/mediawiki/php-1.39.0-wmf.1/api.php(45): wfApiMain()
#9 /srv/mediawiki/w/api.php(3): require(string)
#10 {main}

Seems likely to be causing:

mwversion: 1.39.0-wmf.1
reqId: fa5ce774-e65a-4678-9730-908ad7f02fed
Find reqId in Logstash

normalized_message

[{reqId}] {exception_url}   TypeError: Argument 1 passed to GrowthExperiments\NewcomerTasks\NewcomerTasksChangeTagsManager::apply() must be of the type string, null given, called in /srv/mediawiki/php-1.39.0-wmf.1/extensions/GrowthExperiments/includes/Vis

exception.trace

from /srv/mediawiki/php-1.39.0-wmf.1/extensions/GrowthExperiments/includes/NewcomerTasks/NewcomerTasksChangeTagsManager.php(71)
#0 /srv/mediawiki/php-1.39.0-wmf.1/extensions/GrowthExperiments/includes/VisualEditorHooks.php(140): GrowthExperiments\NewcomerTasks\NewcomerTasksChangeTagsManager->apply(NULL, integer, User)
#1 /srv/mediawiki/php-1.39.0-wmf.1/includes/HookContainer/HookContainer.php(160): GrowthExperiments\VisualEditorHooks->onVisualEditorApiVisualEditorEditPostSave(MediaWiki\Page\PageIdentityValue, User, string, array, array, array, array)
#2 /srv/mediawiki/php-1.39.0-wmf.1/extensions/VisualEditor/includes/VisualEditorHookRunner.php(75): MediaWiki\HookContainer\HookContainer->run(string, array, array)
#3 /srv/mediawiki/php-1.39.0-wmf.1/extensions/VisualEditor/includes/ApiVisualEditorEdit.php(541): MediaWiki\Extension\VisualEditor\VisualEditorHookRunner->onVisualEditorApiVisualEditorEditPostSave(MediaWiki\Page\PageIdentityValue, User, string, array, array, array, array)
#4 /srv/mediawiki/php-1.39.0-wmf.1/includes/api/ApiMain.php(1893): MediaWiki\Extension\VisualEditor\ApiVisualEditorEdit->execute()
#5 /srv/mediawiki/php-1.39.0-wmf.1/includes/api/ApiMain.php(870): ApiMain->executeAction()
#6 /srv/mediawiki/php-1.39.0-wmf.1/includes/api/ApiMain.php(841): ApiMain->executeActionWithErrorHandling()
#7 /srv/mediawiki/php-1.39.0-wmf.1/api.php(90): ApiMain->execute()
#8 /srv/mediawiki/php-1.39.0-wmf.1/api.php(45): wfApiMain()
#9 /srv/mediawiki/w/api.php(3): require(string)
#10 {main}

Impact

Unclear.

Notes

2 of these in 1.39.0-wmf.1 (T300203).

Details

Request URL: https://ru.wikipedia.org/w/api.php

Related Changes in Gerrit:

	Subject	Repo	Branch	Lines +/-
	VisualEditorHooks: Get task type ID from plugin data	mediawiki/extensions/GrowthExperiments	master	+13 -11

Customize query in gerrit

Related Objects

Mentioned In: T304353: PHP Warning: preg_match() expects parameter 2 to be string, array given
Mentioned Here: T297004: Rework how we track newcomer task edits
rEGREdc377d2963ec: Tag newcomer task edits via VisualEditor or post-edit dialog
T300203: 🧪🚂 Trainsperiment Week: 1.39.0-wmf.1, 1.39.0-wmf.2, 1.39.0-wmf.3, 1.39.0-wmf.4 deployment blockers

Event Timeline

brennen created this task.Mar 21 2022, 6:17 PM

Restricted Application added a project: Growth-Team. · View Herald TranscriptMar 21 2022, 6:17 PM

Restricted Application added a subscriber: Aklapper. · View Herald Transcript

kostajh edited projects, added: Growth-Team (Sprint 0 (Growth Team)); removed: Growth-Team.Mar 21 2022, 6:47 PM

kostajh moved this task from Incoming to Ready for Development on the Growth-Team (Sprint 0 (Growth Team)) board.

This is caused by the changes made in rEGREdc377d2963ec: Tag newcomer task edits via VisualEditor or post-edit dialog / T297004, not sure yet why.

The code in GrowthExperiments/includes/VisualEditorHooks.php at line 140 should only be reached if we have some data sent from the client-side, and one of those things is a field in the payload called taskType. Line 140 is complaining that this isn't set. In line 107 we call getDataFromApiRequest which should only return if there is a JSON blob that matches a data structure that we expect (we don't check for taskType, though).

Is it possible that the JavaScript file for this feature wasn't updated for these two users (who are logged-in), and so they posted incomplete payload which then caused the error? @brennen I guess this would be possible if they were mid-edit while the server-side files were synced?

dduvall mentioned this in T304353: PHP Warning: preg_match() expects parameter 2 to be string, array given.Mar 21 2022, 7:10 PM

Is it possible that the JavaScript file for this feature wasn't updated for these two users (who are logged-in), and so they posted incomplete payload which then caused the error? @brennen I guess this would be possible if they were mid-edit while the server-side files were synced?

Maybe? Seems plausible since it hasn't recurred since 18:13 UTC.

Is it possible that the JavaScript file for this feature wasn't updated for these two users (who are logged-in), and so they posted incomplete payload which then caused the error?

Client-side changes take 5-10 minutes to propagate to new browsing sessions. There is no limit to how long an existing browser tab can stay open and continue to use the running code from that process. If this was a breaking change then it should probably have been done in two stages such that they are deployed in separate trains, or by deploying the the prep stage through a backport deploy.

As for this task, the API is public and should not cause an application-level error regardless of what the HTTP endpoint receives from the outside world. However if this is indeed not "possible" for supported use cases, then the fix might be as simple as correctly discarding invalid input and/or ensuring internal methods are only called with internally created data structures based on input instead of passed through from external input.

Krinkle moved this task from Untriaged to Mar 2022 on the Wikimedia-production-error board.Mar 22 2022, 1:53 PM

In T304349#7794396, @brennen wrote:

Is it possible that the JavaScript file for this feature wasn't updated for these two users (who are logged-in), and so they posted incomplete payload which then caused the error? @brennen I guess this would be possible if they were mid-edit while the server-side files were synced?

Maybe? Seems plausible since it hasn't recurred since 18:13 UTC.

OK, thanks for checking.

In T304349#7796565, @Krinkle wrote:

Is it possible that the JavaScript file for this feature wasn't updated for these two users (who are logged-in), and so they posted incomplete payload which then caused the error?

Client-side changes take 5-10 minutes to propagate to new browsing sessions. There is no limit to how long an existing browser tab can stay open and continue to use the running code from that process. If this was a breaking change then it should probably have been done in two stages such that they are deployed in separate trains, or by deploying the the prep stage through a backport deploy.

Yeah, that would have been a good idea.

As for this task, the API is public and should not cause an application-level error regardless of what the HTTP endpoint receives from the outside world. However if this is indeed not "possible" for supported use cases, then the fix might be as simple as correctly discarding invalid input and/or ensuring internal methods are only called with internally created data structures based on input instead of passed through from external input.

That's the remaining actionable item, here, I think. We should make sure that 'taskType' is set in the data structure and return an error otherwise.

kostajh claimed this task.Mar 23 2022, 12:32 PM

kostajh triaged this task as Medium priority.

kostajh moved this task from Ready for Development to In Progress on the Growth-Team (Sprint 0 (Growth Team)) board.

Change 773216 had a related patch set uploaded (by Kosta Harlan; author: Kosta Harlan):

[mediawiki/extensions/GrowthExperiments@master] VisualEditorHooks: Get task type ID from plugin data

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

gerritbot added a project: Patch-For-Review.Mar 23 2022, 12:58 PM

kostajh moved this task from In Progress to Code Review on the Growth-Team (Sprint 0 (Growth Team)) board.Mar 23 2022, 12:58 PM

Esanders moved this task from To Triage to Triaged on the VisualEditor board.Mar 23 2022, 2:38 PM

Change 773216 merged by jenkins-bot:

[mediawiki/extensions/GrowthExperiments@master] VisualEditorHooks: Get task type ID from plugin data

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

ReleaseTaggerBot added a project: MW-1.39-notes (1.39.0-wmf.5; 2022-03-28).Mar 24 2022, 6:00 PM

Maintenance_bot removed a project: Patch-For-Review.Mar 24 2022, 6:11 PM

kostajh moved this task from Code Review to QA on the Growth-Team (Sprint 0 (Growth Team)) board.Mar 31 2022, 11:08 AM

Etonkovidova closed this task as Resolved.Apr 1 2022, 12:44 AM

PHP Notice: Undefined index: taskType - causes Argument 1 passed to GrowthExperiments\NewcomerTasks\NewcomerTasksChangeTagsManager::apply() must be of the type string, null givenClosed, ResolvedPublicPRODUCTION ERRORActions