Skip to content

Commit 4c81369

Browse files
dmvrtxhtdat
dmvrtx
and
htdat
authored
Retry API requests on network failures. (#5309)
* Retry API requests on network failures. Request is considered failed due to network reasons if response code is not set. Code has exponential back-off to avoid overloading back-end. Co-authored-by: Dat Hoang <[email protected]>
1 parent ea5193f commit 4c81369

File tree

3 files changed

+232
-14
lines changed

3 files changed

+232
-14
lines changed

‎changelog/tweak-api-retry-on-network-failure

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
Significance: minor
2+
Type: dev
3+
4+
Retry API requests on network failure if Idempotency-Key header is present.

‎includes/wc-payment-api/class-wc-payments-api-client.php

Lines changed: 43 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99

1010
use WCPay\Exceptions\API_Exception;
1111
use WCPay\Exceptions\Amount_Too_Small_Exception;
12+
use WCPay\Exceptions\Connection_Exception;
1213
use WCPay\Fraud_Prevention\Fraud_Prevention_Service;
1314
use WCPay\Fraud_Prevention\Buyer_Fingerprinting_Service;
1415
use WCPay\Logger;
@@ -30,7 +31,9 @@ class WC_Payments_API_Client {
3031
const GET = 'GET';
3132
const DELETE = 'DELETE';
3233

33-
const API_TIMEOUT_SECONDS = 70;
34+
const API_TIMEOUT_SECONDS = 70;
35+
const API_RETRIES_LIMIT = 3;
36+
const API_RETRIES_BACKOFF_MSEC = 250;
3437

3538
const ACCOUNTS_API = 'accounts';
3639
const CAPABILITIES_API = 'accounts/capabilities';
@@ -2088,20 +2091,46 @@ protected function request( $params, $api, $method, $is_site_specific = true, $u
20882091
);
20892092
}
20902093

2091-
$response = $this->http_client->remote_request(
2092-
[
2093-
'url' => $url,
2094-
'method' => $method,
2095-
'headers' => apply_filters( 'wcpay_api_request_headers', $headers ),
2096-
'timeout' => self::API_TIMEOUT_SECONDS,
2097-
'connect_timeout' => self::API_TIMEOUT_SECONDS,
2098-
],
2099-
$body,
2100-
$is_site_specific,
2101-
$use_user_token
2102-
);
2094+
$headers = apply_filters( 'wcpay_api_request_headers', $headers );
2095+
$stop_trying_at = time() + self::API_TIMEOUT_SECONDS;
2096+
$retries = 0;
2097+
$retries_limit = array_key_exists( 'Idempotency-Key', $headers ) ? self::API_RETRIES_LIMIT : 0;
2098+
2099+
while ( true ) {
2100+
$response_code = null;
2101+
$last_exception = null;
2102+
2103+
try {
2104+
$response = $this->http_client->remote_request(
2105+
[
2106+
'url' => $url,
2107+
'method' => $method,
2108+
'headers' => $headers,
2109+
'timeout' => self::API_TIMEOUT_SECONDS,
2110+
'connect_timeout' => self::API_TIMEOUT_SECONDS,
2111+
],
2112+
$body,
2113+
$is_site_specific,
2114+
$use_user_token
2115+
);
2116+
2117+
$response = apply_filters( 'wcpay_api_request_response', $response, $method, $url, $api );
2118+
$response_code = wp_remote_retrieve_response_code( $response );
2119+
} catch ( Connection_Exception $e ) {
2120+
$last_exception = $e;
2121+
}
21032122

2104-
$response = apply_filters( 'wcpay_api_request_response', $response, $method, $url, $api );
2123+
if ( $response_code || time() >= $stop_trying_at || $retries_limit === $retries ) {
2124+
if ( null !== $last_exception ) {
2125+
throw $last_exception;
2126+
}
2127+
break;
2128+
}
2129+
2130+
// Use exponential backoff to not overload backend.
2131+
usleep( self::API_RETRIES_BACKOFF_MSEC * ( 2 ** $retries ) );
2132+
$retries++;
2133+
}
21052134

21062135
$this->check_response_for_errors( $response );
21072136

‎tests/unit/wc-payment-api/test-class-wc-payments-api-client.php

Lines changed: 185 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
*/
77

88
use WCPay\Exceptions\API_Exception;
9+
use WCPay\Exceptions\Connection_Exception;
910
use WCPay\Fraud_Prevention\Fraud_Prevention_Service;
1011
use WCPay\Fraud_Prevention\Buyer_Fingerprinting_Service;
1112

@@ -2146,6 +2147,190 @@ public function test_authorizations_summary_success() {
21462147
$this->assertSame( 1200, $summary['total'] );
21472148
}
21482149

2150+
/**
2151+
* Test that API client will retry request in case of network error
2152+
*
2153+
* POST calls have `Idempotency-Key` set in the `request`, thus are
2154+
* possible to retry.
2155+
*
2156+
* @throws Exception in case of the test failure.
2157+
*/
2158+
public function test_request_retries_post_on_network_failure() {
2159+
$this->mock_http_client
2160+
->expects( $this->exactly( 4 ) )
2161+
->method( 'remote_request' )
2162+
->willReturn(
2163+
[
2164+
'body' => wp_json_encode( [ 'result' => 'error' ] ),
2165+
'response' => [
2166+
'code' => 0,
2167+
'message' => 'Unknown network error',
2168+
],
2169+
]
2170+
);
2171+
2172+
PHPUnit_Utils::call_method(
2173+
$this->payments_api_client,
2174+
'request',
2175+
[ [], 'intentions', 'POST' ]
2176+
);
2177+
}
2178+
2179+
/**
2180+
* Test that API client will retry request in case of network error
2181+
* indiciated by Connection_Exception.
2182+
*
2183+
* POST calls have `Idempotency-Key` set in the `request`, thus are
2184+
* possible to retry.
2185+
*
2186+
* @throws Exception in case of the test failure.
2187+
*/
2188+
public function test_request_retries_post_on_network_failure_exception() {
2189+
$this->mock_http_client
2190+
->expects( $this->exactly( 4 ) )
2191+
->method( 'remote_request' )
2192+
->willThrowException(
2193+
new Connection_Exception( 'HTTP request failed', 'wcpay_http_request_failed', 500 )
2194+
);
2195+
2196+
$this->expectException( Connection_Exception::class );
2197+
2198+
PHPUnit_Utils::call_method(
2199+
$this->payments_api_client,
2200+
'request',
2201+
[ [], 'intentions', 'POST' ]
2202+
);
2203+
}
2204+
2205+
/**
2206+
* Test that API client will retry request in case of network error
2207+
* and stop on success.
2208+
*
2209+
* POST calls have `Idempotency-Key` set in the `request`, thus are
2210+
* possible to retry.
2211+
*
2212+
* @throws Exception in case of the test failure.
2213+
*/
2214+
public function test_request_retries_post_on_network_failure_exception_and_stops_on_success() {
2215+
$this->mock_http_client
2216+
->expects( $this->exactly( 3 ) )
2217+
->method( 'remote_request' )
2218+
->willReturnOnConsecutiveCalls(
2219+
$this->throwException(
2220+
new Connection_Exception( 'HTTP request failed', 'wcpay_http_request_failed', 500 )
2221+
),
2222+
$this->throwException(
2223+
new Connection_Exception( 'HTTP request failed', 'wcpay_http_request_failed', 500 )
2224+
),
2225+
[
2226+
'body' => wp_json_encode( [ 'result' => 'success' ] ),
2227+
'response' => [
2228+
'code' => 200,
2229+
'message' => 'OK',
2230+
],
2231+
]
2232+
);
2233+
2234+
PHPUnit_Utils::call_method(
2235+
$this->payments_api_client,
2236+
'request',
2237+
[ [], 'intentions', 'POST' ]
2238+
);
2239+
}
2240+
2241+
/**
2242+
* Test that API client will not retry if connection exception indicates there
2243+
* was a response.
2244+
*
2245+
* @throws Exception in case of the test failure.
2246+
*/
2247+
public function test_request_doesnt_retry_on_other_exceptions() {
2248+
$this->mock_http_client
2249+
->expects( $this->exactly( 1 ) )
2250+
->method( 'remote_request' )
2251+
->willThrowException(
2252+
new Exception( 'Random exception' )
2253+
);
2254+
2255+
$this->expectException( Exception::class );
2256+
2257+
PHPUnit_Utils::call_method(
2258+
$this->payments_api_client,
2259+
'request',
2260+
[ [], 'intentions', 'POST' ]
2261+
);
2262+
}
2263+
2264+
/**
2265+
* Test that API client will retry request in case of network error with
2266+
* Idempotency-Key header
2267+
*
2268+
* @throws Exception in case of the test failure.
2269+
*/
2270+
public function test_request_retries_get_with_idempotency_header_on_network_failure() {
2271+
$this->mock_http_client
2272+
->expects( $this->exactly( 4 ) )
2273+
->method( 'remote_request' )
2274+
->willReturn(
2275+
[
2276+
'body' => wp_json_encode( [ 'result' => 'error' ] ),
2277+
'response' => [
2278+
'code' => 0,
2279+
'message' => 'Unknown network error',
2280+
],
2281+
]
2282+
);
2283+
2284+
$callable = function ( $headers ) {
2285+
$headers['Idempotency-Key'] = 'ik_42';
2286+
return $headers;
2287+
};
2288+
2289+
add_filter(
2290+
'wcpay_api_request_headers',
2291+
$callable,
2292+
10,
2293+
2
2294+
);
2295+
2296+
PHPUnit_Utils::call_method(
2297+
$this->payments_api_client,
2298+
'request',
2299+
[ [], 'intentions', 'GET' ]
2300+
);
2301+
2302+
remove_filter(
2303+
'wcpay_api_request_headers',
2304+
$callable,
2305+
10
2306+
);
2307+
}
2308+
2309+
/**
2310+
* Test that API client won't retry GET request without Idemptency-Key header.
2311+
*
2312+
* @throws Exception in case of the test failure.
2313+
*/
2314+
public function test_request_doesnt_retry_get_without_idempotency_header_on_network_failure() {
2315+
$this->mock_http_client
2316+
->expects( $this->exactly( 1 ) )
2317+
->method( 'remote_request' )
2318+
->willReturn(
2319+
[
2320+
'body' => wp_json_encode( [ 'result' => 'error' ] ),
2321+
'response' => [
2322+
'code' => 0,
2323+
'message' => 'Unknown network error',
2324+
],
2325+
]
2326+
);
2327+
2328+
PHPUnit_Utils::call_method(
2329+
$this->payments_api_client,
2330+
'request',
2331+
[ [], 'intentions', 'GET' ]
2332+
);
2333+
}
21492334
/**
21502335
* Set up http mock response.
21512336
*

0 commit comments

Comments
 (0)