r41445 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:< r41444‎ | r41445 | r41446 >
Date:19:58, 30 September 2008
Author:demon
Status:old (Comments)
Tags:
Comment:
Add some more debugging goodies to this. $lastCurlErrno and $lastHttpResponse are static members saved with the last returned status codes.
Modified paths:
  • /trunk/phase3/includes/HttpFunctions.php (modified) (history)

Diff [purge]

Index: trunk/phase3/includes/HttpFunctions.php
——@@ -12,6 +12,9 @@
1313 */
1414 class Http {
1515
 16+ public static $lastCurlErrno = 0;
 17+ public static $lastHttpResponse = 0;
 18+
1619 /**
1720 * Simple wrapper for Http::request( 'GET' )
1821 * @see Http::request()
——@@ -82,16 +85,16 @@
8386 ob_end_clean();
8487
8588 # Don't return the text of error messages, return false on error
86 - $retcode = curl_getinfo( $c, CURLINFO_HTTP_CODE );
87 - if ( $retcode != 200 ) {
88 - wfDebug( __METHOD__ . ": HTTP return code $retcode\n" );
 89+ self::$lastHttpResponse = curl_getinfo( $c, CURLINFO_HTTP_CODE );
 90+ if ( self::$lastHttpResponse != 200 ) {
 91+ wfDebug( __METHOD__ . ": HTTP return code " . self::$lastHttpResponse . "\n" );
8992 $text = false;
9093 }
9194 # Don't return truncated output
92 - $errno = curl_errno( $c );
93 - if ( $errno != CURLE_OK ) {
 95+ self::$lastCurlErrno = curl_errno( $c );
 96+ if ( self::$lastCurlErrno != CURLE_OK ) {
9497 $errstr = curl_error( $c );
95 - wfDebug( __METHOD__ . ": CURL error code $errno: $errstr\n" );
 98+ wfDebug( __METHOD__ . ": CURL error code " . self::$lastCurlErrno . ": $errstr\n" );
9699 $text = false;
97100 }
98101 curl_close( $c );

Follow-up revisions

RevisionCommit summaryAuthorDate
r41959Revert r41445: static members invite permanent use in the caller, which would...tstarling05:20, 11 October 2008

Comments

#Comment by Brion VIBBER (talk | contribs)   20:11, 30 September 2008

Hmm, I'm not sure I like setting static members. That always feels icky, since you have to be careful about the order of operations and making sure something else doesn't run another hit in the meantime before you checked it.

#Comment by Brion VIBBER (talk | contribs)   20:18, 30 September 2008

This is a followup to r41440

#Comment by Tim Starling (talk | contribs)   05:11, 11 October 2008

It says it's for debugging but the way it's written invites permanent use in calling code. For permanent use, we should have a wrapper object around the curl handle (or other request method), and present an interface similar to the one used to invoke curl. Then Http::get() could be a frontend to that. For debugging, I think the wfDebug() calls added in r41440 are enough. I'm going to revert.

Status & tagging log