Page MenuHomePhabricator

MWHttpRequest needs to be a little smarter about setting status
Closed, DeclinedPublic

Description

In looking at T200346, I noticed a couple of things with MWHttpRequest that I think could be problems:

  • https://phabricator.wikimedia.org/rMW0584339f5edf42b7f90d9cfbeb45820f483fe1f7 sets respStatus = 0 when a request failed without valid headers, but MWHttpRequest assumes that any response code < 399 is a successful response.
  • In some cases, result codes > 399 may not actually indicate a failure. In other cases it does. MWHttpRequest does not have a mechanism by which to indicate when other response codes are acceptable.

Details

Related Gerrit Patches:

Event Timeline

Imarlier triaged this task as Medium priority.Jul 27 2018, 3:21 PM
Imarlier created this task.
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptJul 27 2018, 3:21 PM

Change 448566 had a related patch set uploaded (by Imarlier; owner: Imarlier):
[mediawiki/core@master] MWHttpRequest: Be less naive about status

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

Imarlier moved this task from Inbox to Doing on the Performance-Team board.Jul 30 2018, 7:46 PM

Change 448566 abandoned by Imarlier:
MWHttpRequest: Be less naive about status

Reason:
There's other HTTP client work going on, this isn't really needed, so I'm going to drop it.

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

Imarlier closed this task as Declined.Nov 27 2018, 6:46 PM
Imarlier added a subscriber: BPirkle.

I still think that there are better options for status than the way that MWHttpRequest handles it, but I'm not going to pursue changing it. If @BPirkle happens to do this differently as part of the Guzzle work that he's been doing, though, I'd support it...