Page MenuHomePhabricator

Retries broken for BatchRequest
Closed, ResolvedPublic

Description

In the very first tests after batching support was enabled, I ran into errors with "Failed to parse the JSON response for Batch request" but since they were very intermittent, we couldn't reliably reproduce them.

However, I ran into them again, and I noticed that they happen consistently after a "Failed API request, {"error":{"code":"ETIMEDOUT"},"retries-remaining":1}" .. I suspect this is because the generic retry handling in ApiRequest may not be working properly for BatchRequest. API args are submitted via req.form in the BatchRequest constructor. But, ApiRequest._requestCB doesn't do that .. it just calls this.request(...) and this retry for BatchRequest might not be passing along the api args. All other POST requests set this.requestOptions.form whereas BatchRequest doesn't.

I noticed this on testwiki/User:PiRSquared17/Sandbox7. But, you can reproduce this on any page by setting an artificially low timeout value for batch requests.

Event Timeline

ssastry assigned this task to tstarling.
ssastry raised the priority of this task from to High.
ssastry updated the task description. (Show Details)
ssastry added a project: Parsoid.
ssastry subscribed.

As I noted in a comment in the BatchRequest constructor, the reason for not using the form option is to allow the use of multipart encoding, which is much more efficient when posting large amounts of text. I can probably move the multipart code to ApiRequest.request(), so that it will work with retries, and so that all API actions can take advantage of it, not just BatchRequest.

As I noted in a comment in the BatchRequest constructor, the reason for not using the form option is to allow the use of multipart encoding, which is much more efficient when posting large amounts of text. I can probably move the multipart code to ApiRequest.request(), so that it will work with retries, and so that all API actions can take advantage of it, not just BatchRequest.

In https://gerrit.wikimedia.org/r/#/c/227208/6/lib/mediawiki.ApiRequest.js .. Arlo suggested a fix based on your comment in case it helps fix this issue as well.

Change 240296 had a related patch set uploaded (by Tim Starling):
Fix batch retries and upgrade to request 2.63

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

Change 240296 merged by jenkins-bot:
Fix batch retries and upgrade to request 2.63

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

ssastry removed a project: Patch-For-Review.
ssastry set Security to None.
ssastry removed a subscriber: gerritbot.