Page MenuHomePhabricator

Passing in the "body" request option as an array to send a POST request has been deprecated
Closed, ResolvedPublic

Description

Noticed in Fatal-Monitor after deploying 1.33.0-wmf.8 to group1

Url: /w/api.php?format=json&formatversion=2&action=jsondata&title=Plas+Gogerddan.map on wikidatawiki

[{exception_id}] {exception_url} InvalidArgumentException from line 420 of /srv/mediawiki/php-1.33.0-wmf.8/vendor/guzzlehttp/guzzle/src/Client.php: Passing in the "body" request option as an array to send a POST request has been deprecated. Please use the "form_params" request option to send a application/x-www-form-urlencoded request, or the "multipart" request option to send a multipart/form-data request.
#0 /srv/mediawiki/php-1.33.0-wmf.8/vendor/guzzlehttp/guzzle/src/Client.php(340): GuzzleHttp\Client->invalidBody()
#1 /srv/mediawiki/php-1.33.0-wmf.8/vendor/guzzlehttp/guzzle/src/Client.php(273): GuzzleHttp\Client->applyOptions(GuzzleHttp\Psr7\Request, array)
#2 /srv/mediawiki/php-1.33.0-wmf.8/vendor/guzzlehttp/guzzle/src/Client.php(100): GuzzleHttp\Client->transfer(GuzzleHttp\Psr7\Request, array)
#3 /srv/mediawiki/php-1.33.0-wmf.8/vendor/guzzlehttp/guzzle/src/Client.php(106): GuzzleHttp\Client->sendAsync(GuzzleHttp\Psr7\Request, array)
#4 /srv/mediawiki/php-1.33.0-wmf.8/includes/http/GuzzleHttpRequest.php(121): GuzzleHttp\Client->send(GuzzleHttp\Psr7\Request)
#5 /srv/mediawiki/php-1.33.0-wmf.8/extensions/JsonConfig/includes/JCUtils.php(116): GuzzleHttpRequest->execute()
#6 /srv/mediawiki/php-1.33.0-wmf.8/extensions/JsonConfig/includes/JCCache.php(221): JsonConfig\JCUtils::callApi(GuzzleHttpRequest, array, string)
#7 /srv/mediawiki/php-1.33.0-wmf.8/extensions/JsonConfig/includes/JCCache.php(183): JsonConfig\JCCache->getPageFromApi(string, GuzzleHttpRequest, array)
#8 /srv/mediawiki/php-1.33.0-wmf.8/extensions/JsonConfig/includes/JCCache.php(60): JsonConfig\JCCache->loadRemote()
#9 /srv/mediawiki/php-1.33.0-wmf.8/extensions/JsonConfig/includes/JCSingleton.php(373): JsonConfig\JCCache->get()
#10 /srv/mediawiki/php-1.33.0-wmf.8/extensions/JsonConfig/includes/JCDataApi.php(20): JsonConfig\JCSingleton::getContent(JsonConfig\JCTitle)
#11 /srv/mediawiki/php-1.33.0-wmf.8/includes/api/ApiMain.php(1595): JsonConfig\JCDataApi->execute()
#12 /srv/mediawiki/php-1.33.0-wmf.8/includes/api/ApiMain.php(531): ApiMain->executeAction()
#13 /srv/mediawiki/php-1.33.0-wmf.8/includes/api/ApiMain.php(502): ApiMain->executeActionWithErrorHandling()
#14 /srv/mediawiki/php-1.33.0-wmf.8/api.php(87): ApiMain->execute()
#15 /srv/mediawiki/w/api.php(3): include(string)
#16 {main}

Url: /w/index.php?title=Especial:Livro&bookcmd=render_collection&colltitle=Wikilivros%3ALivros%2FIntrodu%C3%A7%C3%A3o+%C3%A0+l%C3%ADngua+portuguesa&writer=rl on ptwikibooks

#0 /srv/mediawiki/php-1.33.0-wmf.8/vendor/guzzlehttp/guzzle/src/Client.php(340): GuzzleHttp\Client->invalidBody()
#1 /srv/mediawiki/php-1.33.0-wmf.8/vendor/guzzlehttp/guzzle/src/Client.php(273): GuzzleHttp\Client->applyOptions(GuzzleHttp\Psr7\Request, array)
#2 /srv/mediawiki/php-1.33.0-wmf.8/vendor/guzzlehttp/guzzle/src/Client.php(100): GuzzleHttp\Client->transfer(GuzzleHttp\Psr7\Request, array)
#3 /srv/mediawiki/php-1.33.0-wmf.8/vendor/guzzlehttp/guzzle/src/Client.php(106): GuzzleHttp\Client->sendAsync(GuzzleHttp\Psr7\Request, array)
#4 /srv/mediawiki/php-1.33.0-wmf.8/includes/http/GuzzleHttpRequest.php(121): GuzzleHttp\Client->send(GuzzleHttp\Psr7\Request)
#5 /srv/mediawiki/php-1.33.0-wmf.8/includes/http/Http.php(75): GuzzleHttpRequest->execute()
#6 /srv/mediawiki/php-1.33.0-wmf.8/includes/http/Http.php(122): Http::request(string, string, array, string)
#7 /srv/mediawiki/php-1.33.0-wmf.8/extensions/Collection/rendering/MWServeRenderingAPI.php(39): Http::post(string, array, string)
#8 /srv/mediawiki/php-1.33.0-wmf.8/extensions/Collection/rendering/CollectionRenderingAPI.php(78): MWServeRenderingAPI->makeRequest(string, array)
#9 /srv/mediawiki/php-1.33.0-wmf.8/extensions/Collection/rendering/CollectionRenderingAPI.php(55): CollectionRenderingAPI->doRender(array)
#10 /srv/mediawiki/php-1.33.0-wmf.8/extensions/Collection/Collection.body.php(1055): CollectionRenderingAPI->render(array)
#11 /srv/mediawiki/php-1.33.0-wmf.8/extensions/Collection/Collection.body.php(261): SpecialCollection->renderCollection(array, Title, string)
#12 /srv/mediawiki/php-1.33.0-wmf.8/includes/specialpage/SpecialPage.php(569): SpecialCollection->execute(NULL)
#13 /srv/mediawiki/php-1.33.0-wmf.8/includes/specialpage/SpecialPageFactory.php(558): SpecialPage->run(NULL)
#14 /srv/mediawiki/php-1.33.0-wmf.8/includes/MediaWiki.php(288): MediaWiki\Special\SpecialPageFactory->executePath(Title, RequestContext)
#15 /srv/mediawiki/php-1.33.0-wmf.8/includes/MediaWiki.php(862): MediaWiki->performRequest()
#16 /srv/mediawiki/php-1.33.0-wmf.8/includes/MediaWiki.php(517): MediaWiki->main()
#17 /srv/mediawiki/php-1.33.0-wmf.8/index.php(42): MediaWiki->run()
#18 /srv/mediawiki/w/index.php(3): include(string)
#19 {main}

Event Timeline

zeljkofilipin triaged this task as Unbreak Now! priority.Dec 12 2018, 6:47 PM
zeljkofilipin created this task.
Restricted Application changed the subtype of this task from "Release" to "Task". · View Herald TranscriptDec 12 2018, 6:47 PM
Restricted Application added subscribers: Liuxinyu970226, TerraCodes, Aklapper. · View Herald Transcript

Can we get more of a stack?

How frequently is it happening?

Seems to be "bad calling" from a couple of entry points including Collection

#0 /srv/mediawiki/php-1.33.0-wmf.8/vendor/guzzlehttp/guzzle/src/Client.php(340): GuzzleHttp\Client->invalidBody()
#1 /srv/mediawiki/php-1.33.0-wmf.8/vendor/guzzlehttp/guzzle/src/Client.php(273): GuzzleHttp\Client->applyOptions(GuzzleHttp\Psr7\Request, array)
#2 /srv/mediawiki/php-1.33.0-wmf.8/vendor/guzzlehttp/guzzle/src/Client.php(100): GuzzleHttp\Client->transfer(GuzzleHttp\Psr7\Request, array)
#3 /srv/mediawiki/php-1.33.0-wmf.8/vendor/guzzlehttp/guzzle/src/Client.php(106): GuzzleHttp\Client->sendAsync(GuzzleHttp\Psr7\Request, array)
#4 /srv/mediawiki/php-1.33.0-wmf.8/includes/http/GuzzleHttpRequest.php(121): GuzzleHttp\Client->send(GuzzleHttp\Psr7\Request)
#5 /srv/mediawiki/php-1.33.0-wmf.8/includes/http/Http.php(75): GuzzleHttpRequest->execute()
#6 /srv/mediawiki/php-1.33.0-wmf.8/includes/http/Http.php(122): Http::request(string, string, array, string)
#7 /srv/mediawiki/php-1.33.0-wmf.8/extensions/Collection/rendering/MWServeRenderingAPI.php(39): Http::post(string, array, string)
#8 /srv/mediawiki/php-1.33.0-wmf.8/extensions/Collection/rendering/CollectionRenderingAPI.php(78): MWServeRenderingAPI->makeRequest(string, array)
#9 /srv/mediawiki/php-1.33.0-wmf.8/extensions/Collection/rendering/CollectionRenderingAPI.php(55): CollectionRenderingAPI->doRender(array)
#10 /srv/mediawiki/php-1.33.0-wmf.8/extensions/Collection/Collection.body.php(1055): CollectionRenderingAPI->render(array)
#11 /srv/mediawiki/php-1.33.0-wmf.8/extensions/Collection/Collection.body.php(261): SpecialCollection->renderCollection(array, Title, string)
#12 /srv/mediawiki/php-1.33.0-wmf.8/includes/specialpage/SpecialPage.php(569): SpecialCollection->execute(NULL)
#13 /srv/mediawiki/php-1.33.0-wmf.8/includes/specialpage/SpecialPageFactory.php(558): SpecialPage->run(NULL)
#14 /srv/mediawiki/php-1.33.0-wmf.8/includes/MediaWiki.php(288): MediaWiki\Special\SpecialPageFactory->executePath(Title, RequestContext)
#15 /srv/mediawiki/php-1.33.0-wmf.8/includes/MediaWiki.php(862): MediaWiki->performRequest()
#16 /srv/mediawiki/php-1.33.0-wmf.8/includes/MediaWiki.php(517): MediaWiki->main()
#17 /srv/mediawiki/php-1.33.0-wmf.8/index.php(42): MediaWiki->run()
#18 /srv/mediawiki/w/index.php(3): include(string)
#19 {main}

84 in the last hour... I don't think this is unbreak now, but is probably at least High

Do you have an estimate on the number of errors when move group2 to wmf.8? If you estimate that the number will remain low, then I guess this is not blocking the train. If you don't have an estimate, I would be reluctant to move the train forward and this would still block the train. All train blockers have UBN priority.

https://stackoverflow.com/a/38060763 seems very likely as to what's going on, especially as we're doing json stuff in jsonconfig (well, duh)

Note in Guzzle V6.0+, another source of getting the following error may be incorrect use of JSON as an array:
Passing in the "body" request option as an array to send a POST request has been deprecated. Please use the "form_params" request option to send a application/x-www-form-urlencoded request, or a the "multipart" request option to send a multipart/form-data request.

GuzzleHttpRequest.php line 94, we are indeed setting the 'body' field and trusting that $postData was happily passed to the constructor by calling code. Fiddling...

Reproduced the exception locally using trivial test data:

$params = [ 'foo' => 'bar' ];
$response = Http::post(
	'http://www.example.net',
	[ 'postData' => $params ]
);

Then confirmed the exception disappeared per the suggestion from stack overflow, by adding this to GuzzleHttpRequest.php just before we use $postData:

			if ( is_array( $postData) ) {
				$postData = json_encode( $postData );
			}

Now looking at more complex cases, and confirming actual functionality rather than just lack of an exception.

This looks more promising:

			if ( is_array( $postData )  ) {
				$this->guzzleOptions['form_params'] = $postData;
			} else {
				$this->guzzleOptions['body'] = $postData;
			}

Per http://docs.guzzlephp.org/en/stable/quickstart.html, Guzzle will automatically set Content-Type to application/x-www-form-urlencoded . This happens within Guzzle's Client.php on line 314. This mirrors curl's behavior with CURLOPT_POSTFIELDS, which we previously used in CurlHttpRequest.php. Per https://curl.haxx.se/libcurl/c/CURLOPT_POSTFIELDS.html, if CURLOPT_POSTFIELDS is set :

This POST is a normal application/x-www-form-urlencoded kind (and libcurl will set that Content-Type by default when this option is used), which is commonly used by HTML forms.

So the normal case of sending an array of POST fields should work happily, and callers can also override as needed. I'm running unit tests locally right now, but of course, that only goes so far (else we wouldn't be here in the first place).

Worst case, if this doesn't work out, we can get the train unblocked by modifying HttpRequestFactory::create to only use guzzle if explicitly requested. The CurlHttpRequest and PhpHttpRequest code remains in the codebase.

Change 479359 had a related patch set uploaded (by BPirkle; owner: BPirkle):
[mediawiki/core@master] Fix guzzle InvalidArgumentException when body is passed as an array

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

Change 479362 had a related patch set uploaded (by Reedy; owner: BPirkle):
[mediawiki/core@wmf/1.33.0-wmf.8] Fix guzzle InvalidArgumentException when body is passed as an array

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

Change 479359 merged by jenkins-bot:
[mediawiki/core@master] Fix guzzle InvalidArgumentException when body is passed as an array

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

Change 479362 merged by jenkins-bot:
[mediawiki/core@wmf/1.33.0-wmf.8] Fix guzzle InvalidArgumentException when body is passed as an array

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

Mentioned in SAL (#wikimedia-operations) [2018-12-13T00:52:48Z] <reedy@deploy1001> Synchronized php-1.33.0-wmf.8/includes/http/GuzzleHttpRequest.php: T211806 (duration: 00m 51s)

Reedy assigned this task to BPirkle.
Reedy removed a project: Patch-For-Review.

Errors look to have stopped when I deployed the patch