Page MenuHomePhabricator
Create Task
Maniphest T371432

EventBus::send incorrectly assumes that JSON response can be converted to an associative array
Closed, ResolvedPublicPRODUCTION ERROR
Actions

Description

Error
message
[{reqId}] {exception_url}   PHP Warning: json_decode() expects parameter 1 to be string, array given
trace
from /srv/mediawiki/php-1.43.0-wmf.15/includes/json/FormatJson.php(151)
#0 [internal function]: MWExceptionHandler::handleError(integer, string, string, integer, array)
#1 /srv/mediawiki/php-1.43.0-wmf.15/includes/json/FormatJson.php(151): json_decode(array, boolean)
#2 /srv/mediawiki/php-1.43.0-wmf.15/extensions/EventBus/includes/EventBus.php(390): MediaWiki\Json\FormatJson::decode(array, boolean)
#3 /srv/mediawiki/php-1.43.0-wmf.15/extensions/EventBus/includes/Adapters/JobQueue/JobQueueEventBus.php(120): MediaWiki\Extension\EventBus\EventBus->send(array, integer)
#4 /srv/mediawiki/php-1.43.0-wmf.15/includes/jobqueue/JobQueue.php(380): MediaWiki\Extension\EventBus\Adapters\JobQueue\JobQueueEventBus->doBatchPush(array, integer)
#5 /srv/mediawiki/php-1.43.0-wmf.15/includes/jobqueue/JobQueue.php(352): JobQueue->batchPush(array, integer)
#6 /srv/mediawiki/php-1.43.0-wmf.15/includes/jobqueue/JobQueueGroup.php(157): JobQueue->push(array)
#7 /srv/mediawiki/php-1.43.0-wmf.15/extensions/IPInfo/src/Rest/Handler/IPInfoHandler.php(212): JobQueueGroup->push(array)
#8 /srv/mediawiki/php-1.43.0-wmf.15/extensions/IPInfo/src/Rest/Handler/IPInfoHandler.php(182): MediaWiki\IPInfo\Rest\Handler\IPInfoHandler->logAccess(MediaWiki\User\User, string, string)
#9 /srv/mediawiki/php-1.43.0-wmf.15/includes/Rest/SimpleHandler.php(40): MediaWiki\IPInfo\Rest\Handler\IPInfoHandler->run(integer)
#10 /srv/mediawiki/php-1.43.0-wmf.15/includes/Rest/Module/Module.php(380): MediaWiki\Rest\SimpleHandler->execute()
#11 /srv/mediawiki/php-1.43.0-wmf.15/includes/Rest/Module/Module.php(269): MediaWiki\Rest\Module\Module->executeHandler(MediaWiki\IPInfo\Rest\Handler\RevisionHandler)
#12 /srv/mediawiki/php-1.43.0-wmf.15/includes/Rest/Router.php(477): MediaWiki\Rest\Module\Module->execute(string, MediaWiki\Rest\RequestFromGlobals)
#13 /srv/mediawiki/php-1.43.0-wmf.15/includes/Rest/Router.php(446): MediaWiki\Rest\Router->doExecute(string, MediaWiki\Rest\RequestFromGlobals)
#14 /srv/mediawiki/php-1.43.0-wmf.15/includes/Rest/EntryPoint.php(209): MediaWiki\Rest\Router->execute(MediaWiki\Rest\RequestFromGlobals)
#15 /srv/mediawiki/php-1.43.0-wmf.15/includes/MediaWikiEntryPoint.php(200): MediaWiki\Rest\EntryPoint->execute()
#16 /srv/mediawiki/php-1.43.0-wmf.15/rest.php(39): MediaWiki\MediaWikiEntryPoint->run()
#17 /srv/mediawiki/w/rest.php(3): require(string)
#18 {main}

Details

Request URL
/w/rest.php/ipinfo/v0/revision/X?dataContext=infobox&language=en
SubjectRepoBranchLines +/-
EventBus send - Fix json_decode bug in error metric handlingmediawiki/extensions/EventBusmaster+31 -26
Customize query in gerrit

Related Objects

Event Timeline

Dreamy_Jazz created this task.Jul 30 2024, 7:46 PM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJul 30 2024, 7:46 PM
Dreamy_Jazz edited projects, added Event-Platform; removed IP Info, Trust and Safety Product Sprint (Sprint Koto (July 15 - July 26)), Trust and Safety Product Team.Jul 30 2024, 7:49 PM

Actually this seems to be a problem in the Event-Platform extension that was surfaced by the IP Info extension.

Restricted Application added a project: Data-Engineering. · View Herald TranscriptJul 30 2024, 7:49 PM
Dreamy_Jazz renamed this task from IPInfo REST API raises json_decode warning to EventBus::send incorrectly assumes that response will always be convertible to an array.Jul 30 2024, 7:50 PM
Dreamy_Jazz renamed this task from EventBus::send incorrectly assumes that response will always be convertible to an array to EventBus::send incorrectly assumes that JSON response can be converted to an associative array.
Ottomata subscribed.Jul 30 2024, 8:36 PM
Ottomata added a subscriber: gmodena.Jul 31 2024, 6:03 PM

I think this warning was recently introduced in work for T363587: [Event Platform] Instrument EventBus with prometheus MW Statslib.

I think this is just a warning log message, not an uncaught exception, right? The code is checking if the FormatJson::decode fails, but I guess json_decode is logging a warning anyway?

Looking at FormatJson docs, perhaps we should change to use FormatJson::parse instead?

brennen added a project: User-brennen.Jul 31 2024, 6:31 PM
brennen moved this task from Backlog to Logs/Train on the User-brennen board.
Dreamy_Jazz added a comment.Jul 31 2024, 6:53 PM
In T371432#10033199, @Ottomata wrote:

I think this is just a warning log message, not an uncaught exception, right? The code is checking if the FormatJson::decode fails, but I guess json_decode is logging a warning anyway?

Yes, this is just a warning message. However, it does cause logstash spam and they are registered in logstash as an ERROR level so should be solved (even if that is just providing a more descriptive error message).

Krinkle subscribed.EditedAug 1 2024, 3:33 PM

Relevant code was last added/changed in https://gerrit.wikimedia.org/r/c/mediawiki/extensions/EventBus/+/1049831 for T363587: [Event Platform] Instrument EventBus with prometheus MW Statslib, \cc @gmodena @Ottomata

In T371432#10033199, @Ottomata wrote:

I think this is just a warning log message, not an uncaught exception, right? The code is checking if the FormatJson::decode fails, but I guess json_decode is logging a warning anyway?

Looking at FormatJson docs, perhaps we should change to use FormatJson::parse instead?

No, parsing isn't failing. EventBus is passing an array instead of a JSON string. It's a type error.

$event = FormatJson::decode( $failurePayload['event'], true );
$streamName = $event['meta']['stream'] ?? self::STREAM_UNKNOWN_NAME;

The warning is saying that $failurePayload['event'] is an array, not a JSON string. decode() returns null for this kind of error. null can in theory be a genuine JSON value, but in practice this is only ever parsing JSON objects, so we can safely assume null is an error.

I don't think this is about handling parse errors. That would be equivalant to removing this part of the instrumentation. The thing you're looking to parse, is presumably somewhere else. This function takes a JSON string. Another possibility, is that it was already parsed and you can't / don't need to parse it again?

Ottomata added a comment.Aug 1 2024, 6:18 PM

Thank you Timo! I haven't had time to look deeply yet. I think you are right!

gerritbot added a comment.Aug 6 2024, 6:15 PM

Change #1060156 had a related patch set uploaded (by Ottomata; author: Ottomata):

[mediawiki/extensions/EventBus@master] EventBus send - Fix json_decode bug in error metric handling

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

gerritbot added a project: Patch-For-Review.Aug 6 2024, 6:15 PM
Ottomata mentioned this in T363587: [Event Platform] Instrument EventBus with prometheus MW Statslib.Aug 6 2024, 6:16 PM
gerritbot added a comment.Aug 7 2024, 9:18 PM

Change #1060156 merged by jenkins-bot:

[mediawiki/extensions/EventBus@master] EventBus send - Fix json_decode bug in error metric handling

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

Maintenance_bot removed a project: Patch-For-Review.Aug 7 2024, 9:30 PM
thcipriani moved this task from Untriaged to Jul 2024 on the Wikimedia-production-error board.Aug 8 2024, 3:36 PM
Ottomata closed this task as Resolved.Oct 23 2024, 8:11 PM
Ottomata claimed this task.
Ahoelzl edited projects, added Data-Engineering (Q2 2024 October 1st - December 31th); removed Data-Engineering.Oct 23 2024, 8:11 PM
Restricted Application edited projects, added Data-Engineering; removed Data-Engineering (Q2 2024 October 1st - December 31th). · View Herald TranscriptOct 23 2024, 8:11 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