First of all, thanks for the module, so that I do not have to rely on zend-framework to do http request. However, I found this snippet in HttpClient.inc that is quite interesting.

    if ($response->responseCode == 200) {
      if ($this->formatter) {
        try {
          $result = $this->formatter->unserialize($response->body);
        }
        catch (Exception $e) {
          throw new HttpClientException('Failed to unserialize response', 0, $response, $e);
        }
      }
      else {
        $result = $response->body;
      }
    }
    // Output any errors set by remote drupal sites.
    elseif (!empty($response->drupalErrors)) {
      throw new HttpClientException(check_plain(implode("\n", $response->drupalErrors)), $response->responseCode, $response);
    }
    // Treat all remaining non-200 responses as errors
    else {
      throw new HttpClientException(check_plain($response->responseMessage), $response->responseCode, $response);
    }

I have a webservice that returns a number of 2xx status for different transactions (not always '200 OK'), if I use this class, I will keep getting exception whenever my script receives other 2xx status. I am just curious whether there is a reason behind the code above (throwing exception for all non-200 status) and is it OK to allow all 2xx (and probably all 1xx) statuses?

CommentFileSizeAuthor
#4 722886-http_client-2xx.patch411 bytesklausi

Comments

Hugo Wetterberg’s picture

Not accepting all 2xx-responses was a mistake, they should indeed be treated as valid. Regarding the 1xx-responses I think that the 100 is the only one that should be gracefully accepted, as 101 is client-initiated (which therefore shouldn't occur). 102 might be treated as a 100 as far as I can tell, but I don't know that much about webdav and it might not make much sense to support in in the http client.

aaron.r.carlton’s picture

subscribing

klausi’s picture

Version: 6.x-2.0 » 7.x-2.x-dev
Category: feature » bug
Status: Active » Needs review
StatusFileSize
new411 bytes

Here is a first primitive patch that accepts all 2xx status codes.

Hugo Wetterberg’s picture

Status: Needs review » Fixed

Fixed by allowing all response codes in the 200-range

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.