Page MenuHomePhabricator

Argument 1 passed to DataValues\UnboundedQuantityValue::newFromArray() must be an instance of array, string given in extensions/Wikidata/vendor/data-values/serialization/src/Deserializers/DataValueDeserializer.php on line 141
Closed, ResolvedPublic

Description

Catchable fatal error: Argument 1 passed to DataValues\UnboundedQuantityValue::newFromArray() must be an instance of array, string given in /srv/mediawiki/php-1.30.0-wmf.6/extensions/Wikidata/vendor/data-values/serialization/src/Deserializers/DataValueDeserializer.php on line 141

We accidentally added strict array type hints to the newFromArray methods of some DataValue classes:

  • GlobeCoordinateValue::newFromArray
  • UnboundedQuantityValue::newFromArray

The only relevant caller of these methods is in DataValueDeserializer::getDeserialization. The caller can not know what the specific newFromArray method it is going to call expects. StringValue for example expects a string, while all more complex DataValues expect an array. Each individual newFromArray method must check if the value it gets matches the expected format, and throw a proper exception if not.

Adding the static array type hints was a mistake, mostly caused by the misleading name "newFromArray". What it means is "newFromIntermediateDeserialization". The provided value comes from an array (see DataValueObject::toArray), but must not be an array. This was never a problem as long as nobody tried to pass bad serializations to our code. This changed now because of a misbehaving bot.

Patch-For-Review:

Other components that should be updated too, but don't need an immediate bugfix release:

Event Timeline

mmodell created this task.Jun 22 2017, 7:49 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJun 22 2017, 7:49 PM
mmodell triaged this task as High priority.Jun 22 2017, 7:49 PM
mmodell added a project: Wikidata.

Mentioned in SAL (#wikimedia-operations) [2017-06-22T19:52:27Z] <twentyafterfour> the train is currently blocked by https://phabricator.wikimedia.org/T168681

@aude or @Addshore: Any clue what might be causing this? I'm not sure how vendor'd code works in the wikidata extension. This error started showing up suddenly after the most recent SWAT even though no code was deployed which should have affected this.

greg added a subscriber: greg.
mmodell raised the priority of this task from High to Unbreak Now!.Jun 22 2017, 8:03 PM
Restricted Application added subscribers: Jay8g, TerraCodes. · View Herald TranscriptJun 22 2017, 8:03 PM
mmodell lowered the priority of this task from Unbreak Now! to High.Jun 22 2017, 8:34 PM

ok this error seems to have disappeared as mysteriously as it appeared.

mmodell raised the priority of this task from High to Unbreak Now!.Jun 22 2017, 8:40 PM

Spoke too soon, now it's back.

This comment was removed by Dereckson.
Dereckson added a comment.EditedJun 22 2017, 8:56 PM

It occurs at API level (so could explain why it disappeared, then came back) in UnboundedQuantityValue class:

#0 /srv/mediawiki/php-1.30.0-wmf.6/extensions/Wikidata/vendor/data-values/number/src/DataValues/UnboundedQuantityValue.php(277): MWExceptionHandler::handleError(integer, string, string, integer, array, array)
#1 /srv/mediawiki/php-1.30.0-wmf.6/extensions/Wikidata/vendor/data-values/serialization/src/Deserializers/DataValueDeserializer.php(141): DataValues\UnboundedQuantityValue::newFromArray(string)
#2 /srv/mediawiki/php-1.30.0-wmf.6/extensions/Wikidata/vendor/data-values/serialization/src/Deserializers/DataValueDeserializer.php(97): DataValues\Deserializers\DataValueDeserializer->getDeserialization()

A deserialized value is expected to be an array, but is a string. The UnboundedQuantityValue code didn't change these last two months.

That's what I don't understand, the code didn't change... So you think this is just invalid data passed to the API and the timing (which coincided with SWAT) was just a coincidence?

It seems to be possible to produce this error via the API, at least, using api.php?action=wbformatvalue&format=json&datavalue=%7B%20%22type%22%3A%20%22quantity%22%2C%20%22value%22%3A%20%2210%22%20%7D (i. e., wbformatvalue with datavalue { "type": "quantity", "value": "10" }). I tried it on my local installation and got the same error message – on Wikidata, I get HTTP 500 but there’s no error message or stack trace in the response, but I assume that’s normal.

With this API request, the stack trace I get is https://gist.github.com/lucaswerkmeister/fa0a142dfcc3360daa72c0c1b4e9ba3c – does that mostly match the stack trace you saw, @Dereckson?

greg added a comment.Jun 22 2017, 10:31 PM

which is:

#0 /var/www/html/wiki1/extensions/WikibaseQualityConstraints/vendor/data-values/serialization/src/Deserializers/DataValueDeserializer.php(141): DataValues\UnboundedQuantityValue::newFromArray(string)<br />
#1 /var/www/html/wiki1/extensions/WikibaseQualityConstraints/vendor/data-values/serialization/src/Deserializers/DataValueDeserializer.php(97): DataValues\Deserializers\DataValueDeserializer-&gt;getDeserialization()<br />
#2 /var/www/html/wiki1/extensions/Wikibase/lib/includes/DataValueFactory.php(42): DataValues\Deserializers\DataValueDeserializer-&gt;deserialize(array)<br />
#3 /var/www/html/wiki1/extensions/Wikibase/lib/includes/DataValueFactory.php(73): DataValues\DataValueFactory-&gt;newDataValue(string, string)<br />
#4 /var/www/html/wiki1/extensions/Wikibase/repo/includes/Api/FormatSnakValue.php(182): DataValues\DataValueFactory-&gt;newFromArray(array)<br />
#5 /var/www/html/wiki1/extensions/Wikibase/repo/includes/Api/FormatSnakValue.php(89): Wikibase\Repo\Api\FormatSnakValue-&gt;decodeDataValue(string)<br />
#6 /var/www/html/wiki1/includes/api/ApiMain.php(1578): Wikibase\Repo\Api\FormatSnakValue-&gt;execute()<br />
#7 /var/www/html/wiki1/includes/api/ApiMain.php(545): ApiMain-&gt;executeAction()<br />
#8 /var/www/html/wiki1/includes/api/ApiMain.php(516): ApiMain-&gt;executeActionWithErrorHandling()<br />
#9 /var/www/html/wiki1/api.php(83): ApiMain-&gt;execute()<br />
#10 {main}<br />

What I want to know is why this just started happening. If it's not really related to today's SWAT then perhaps it shouldn't block the train.

aude added a comment.Jun 23 2017, 12:07 AM

haven't investigated yet but my guess is it's a bot misbehaving

aude added a comment.Jun 23 2017, 12:08 AM

PS we prepared a bump (to wmf/1.30.0-wmf.6) for Wikidata extensions on Tuesday but forgot to merge the bump in release tools in time for the branch cut.

so, Wikidata extensions are still on older version (wmf/1.30.0-wmf.4) which has been deployed for 2 weeks. So I don't believe this is related to deployment.

(nonetheless can take a look)

aude added a comment.Jun 23 2017, 1:33 AM

the servers that the errors occurred on are all api servers, so it's something hitting the api

thiemowmde updated the task description. (Show Details)
thiemowmde added a subscriber: thiemowmde.
thiemowmde updated the task description. (Show Details)Jun 23 2017, 8:35 AM
thiemowmde updated the task description. (Show Details)Jun 23 2017, 8:42 AM
thiemowmde updated the task description. (Show Details)Jun 23 2017, 9:03 AM
thiemowmde updated the task description. (Show Details)Jun 23 2017, 9:16 AM
thiemowmde updated the task description. (Show Details)
thiemowmde added a project: Patch-For-Review.
thiemowmde moved this task from Monitoring to Review on the Wikidata-Former-Sprint-Board board.
thiemowmde moved this task from incoming to in progress on the Wikidata board.
thiemowmde updated the task description. (Show Details)Jun 23 2017, 10:19 AM
WMDE-leszek lowered the priority of this task from Unbreak Now! to High.Jun 23 2017, 11:54 AM

Based on the information we have, we believe this was not related to deployment.

We assume this was caused by a bot doing "bad" request to Wikibase API, that our code didn't handle properly.
There is an issue in the Wikibase-related code, and we're working on getting it fixed. This should not hold the train etc, though. The issue is not new, it has only become visible now.

If those errors continue to be logged on high rate, can we get some more information so we could find out what bot is actually doing those API calls, so we could reach out to people running this bot, so they adjust their bot?

aude added a comment.Jun 23 2017, 11:57 AM

After looking into this, I still think this was caused by a bot misbehaving. No relevant code has changed/deployed recently.

The error stopped (and briefly occurred again around 2:28 UTC).

We can try to have code changes out next week (in regular deployment / wmf.7) that handle such bad input better. I don't think this needs to block wmf.6 deployment also don't think swat backports are necessary if this error only happens sporadically.

Removing this as a deployment blocker per the investigation. Thanks all.

Change 361406 had a related patch set uploaded (by WMDE-leszek; owner: WMDE-leszek):
[mediawiki/extensions/Wikibase@master] Update several DataValues libraries to current bug fix releases

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

Change 361406 merged by jenkins-bot:
[mediawiki/extensions/Wikibase@master] Update several DataValues libraries to current bug fix releases

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

thiemowmde closed this task as Resolved.Jun 30 2017, 10:14 AM
thiemowmde removed a project: Patch-For-Review.
thiemowmde updated the task description. (Show Details)