When Drupal issues an HTTP request using drupal_http_request() and the request fails (for example, you send a request to http://example.com:88 but there is no webserver running on that port) the error code of the response to the fsockopen() call is only preserved by concatenation:

  // Make sure the socket opened properly.
  if (!$fp) {
    $result->error = trim($errno .' '. $errstr);
    return $result;
  }

IMO, we should be doing

$result->code = $errno;

so that we save the error code separately when the request fails as well as when it succeeds.

CommentFileSizeAuthor
#3 errcode3jv.patch1.23 KBjvandyk
#1 errcode2jv.patch558 bytesjvandyk
errcodejv.patch552 bytesjvandyk

Comments

jvandyk’s picture

jvandyk commented
StatusFileSize
new errcode2jv.patch558 bytes

Here's a patch that places the error code in a separate namespace to enforce the checking of isset($result->error) instead of looking only at the result code.

This is only needed if numeric result codes from fsockopen() overlap with HTTP result codes. Anyone know?

dries’s picture

dries commented

I can reproduce this problem -- and it breaks some of my XML-RPC code. (Actually, it was me who reported this problem to John.)

If we create two namespaces (one for network/tcp errors and one for HTTP errors), I suggest that we rename the variables/fields to be slightly more intuitive. The names should be somewhat self-explanatory, especially in CVS HEAD where we can break backward compatibility.

The alternative, is to return negative error numbers for network errors and positive error numbers for HTTP errors. Of course, that would modify the original error code, but I don't think we can use these -- and these might not be standard error codes to begin with.

Either solution works for me -- I leave it up to John to decide. :)

jvandyk’s picture

jvandyk commented
StatusFileSize
new errcode3jv.patch1.23 KB

Here's a simple patch that implements negative result codes. This is the smallest step forward that makes it work.

dries’s picture

dries commented
Status: Needs review » Reviewed & tested by the community

I've tested this and it works. I committed this to CVS HEAD but this needs to be backported to DRUPAL-5 because otherwise xmlrpc_errno() will fail to work properly in certain situations.

drumm’s picture

drumm
he/him
Location NY, US
commented
Status: Reviewed & tested by the community » Fixed

Committed to 5.

For your next patch.. try doing xml-rpc to a nonexistent domain, such as distributed login with a nonexistent domain. The error is not too useful, seems like it might even be a random number.

Anonymous’s picture

(not verified) commented
Status: Fixed » Closed (fixed)