Skip to content

Commit d6fc741

Browse files
daledupreezdaledupreez
authored
Add logging for HTTP validation failures (#4764)
* Add central logger and log HTTP validation details * Add unit tests * Changelog * Add checks for new detailed logging * Changelog * Update changelog.txt with Copilot suggestion * Update changelog entry in readme.txt * Update text domain handling in unit test
1 parent a208bf6 commit d6fc741

File tree

4 files changed

+212
-19
lines changed

4 files changed

+212
-19
lines changed

‎changelog.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
*** Changelog ***
22

33
= 10.2.0 - xxxx-xx-xx =
4+
* Dev - Add logging with DNS resolution diagnostics for URL validation issues when calling Stripe API
45

56
= 10.1.0 - 2025-11-11 =
67
* Dev - Remove unused `shouldShowPaymentRequestButton` parameter and calculations from backend

‎includes/class-wc-stripe-api.php‎

Lines changed: 53 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -259,17 +259,12 @@ public static function request( $request, $api = 'charges', $method = 'POST', $w
259259
$response_headers = wp_remote_retrieve_headers( $response );
260260

261261
if ( is_wp_error( $response ) || empty( $response['body'] ) ) {
262-
// Stripe redacts API keys in the response.
263-
WC_Stripe_Logger::error(
264-
"Stripe API error: {$method} {$api}",
265-
[
266-
'stripe_api_key' => $masked_secret_key,
267-
'stripe_request_id' => self::get_stripe_request_id( $response ),
268-
'idempotency_key' => $idempotency_key,
269-
'request' => $request,
270-
'response' => $response,
271-
]
272-
);
262+
$error_data = [
263+
'stripe_api_key' => $masked_secret_key,
264+
'request' => $request,
265+
'idempotency_key' => $idempotency_key,
266+
];
267+
self::log_error_response( $response, $api, $method, $error_data );
273268

274269
throw new WC_Stripe_Exception( print_r( $response, true ), __( 'There was a problem connecting to the Stripe API endpoint.', 'woocommerce-gateway-stripe' ) );
275270
}
@@ -374,14 +369,11 @@ public static function retrieve( $api ) {
374369
}
375370

376371
if ( is_wp_error( $response ) || empty( $response['body'] ) ) {
377-
WC_Stripe_Logger::error(
378-
"Stripe API error: GET {$api}",
379-
[
380-
'stripe_api_key' => $masked_secret_key,
381-
'stripe_request_id' => self::get_stripe_request_id( $response ),
382-
'response' => $response,
383-
]
384-
);
372+
$error_data = [
373+
'stripe_api_key' => $masked_secret_key,
374+
];
375+
self::log_error_response( $response, $api, 'GET', $error_data );
376+
385377
return new WP_Error( 'stripe_error', __( 'There was a problem connecting to the Stripe API endpoint.', 'woocommerce-gateway-stripe' ) );
386378
}
387379

@@ -664,6 +656,48 @@ public function update_payment_method_configurations( $id, $payment_method_confi
664656
return $response;
665657
}
666658

659+
/**
660+
* Log an error response from the Stripe API.
661+
*
662+
* @param array|WP_Error $response HTTP response or error.
663+
* @param string $api The API endpoint.
664+
* @param string $method The HTTP method used for the request.
665+
* @param array $data Additional data to add to the log.
666+
* @return void
667+
*/
668+
private static function log_error_response( $response, string $api, string $method, array $data = [] ): void {
669+
$error_message = "Stripe API error: {$method} {$api}";
670+
$error_data = array_merge(
671+
$data,
672+
[
673+
'stripe_request_id' => self::get_stripe_request_id( $response ),
674+
'response' => $response,
675+
]
676+
);
677+
678+
// Add logging for URL validation errors.
679+
if (
680+
is_wp_error( $response ) &&
681+
'http_request_failed' === $response->get_error_code() &&
682+
// phpcs:ignore WordPress.WP.I18n.MissingArgDomain
683+
__( 'A valid URL was not provided.' ) === $response->get_error_message()
684+
) {
685+
$stripe_api_host = 'api.stripe.com';
686+
$resolved_ip_address = gethostbyname( $stripe_api_host );
687+
688+
$error_data['resolved_ip_address'] = $resolved_ip_address;
689+
690+
if ( $resolved_ip_address === $stripe_api_host ) {
691+
$error_data['validation_details'] = "$stripe_api_host could not be resolved to an IP address";
692+
} else {
693+
$error_message .= "; Possible DNS resolution problem for $stripe_api_host";
694+
$error_data['validation_details'] = "$stripe_api_host resolved to $resolved_ip_address";
695+
}
696+
}
697+
698+
WC_Stripe_Logger::error( $error_message, $error_data );
699+
}
700+
667701
/**
668702
* Returns the Stripe's request_id associated with the response.
669703
*

‎readme.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,5 +111,6 @@ If you get stuck, you can ask for help in the [Plugin Forum](https://wordpress.o
111111
== Changelog ==
112112

113113
= 10.2.0 - xxxx-xx-xx =
114+
* Dev - Add logging with DNS resolution diagnostics for URL validation issues when calling Stripe API
114115

115116
[See changelog for full details across versions](https://raw.githubusercontent.com/woocommerce/woocommerce-gateway-stripe/trunk/changelog.txt).

‎tests/phpunit/WC_Stripe_API_Test.php‎

Lines changed: 157 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -210,6 +210,163 @@ public function mock_unauthorized_response() {
210210
];
211211
}
212212

213+
/**
214+
* Test WC_Stripe_API::log_error_response() as called from WC_Stripe_API::request() and WC_Stripe_API::retrieve().
215+
*
216+
* @param array|WP_Error $response The mock response.
217+
* @param string $api The API endpoint.
218+
* @param string $method The HTTP method used for the request.
219+
* @param array|null $request_data The mock request data. Only used for POST requests.
220+
* @dataProvider provide_test_log_error_response_tests
221+
*/
222+
public function test_log_error_response( $response, string $api, string $method, ?array $request_data = null ) {
223+
$expected_url = WC_Stripe_API::ENDPOINT . $api;
224+
225+
$pre_http_filter = function ( $return_value, $parsed_args, $url ) use ( $response, $method, $expected_url ) {
226+
if ( $url !== $expected_url ) {
227+
return $return_value;
228+
}
229+
if ( ( $parsed_args['method'] ?? null ) !== $method ) {
230+
return $return_value;
231+
}
232+
233+
return $response;
234+
};
235+
236+
$mock_logger = $this->createMock( \WC_Logger::class );
237+
\WC_Stripe_Logger::$logger = $mock_logger;
238+
239+
$expected_data_keys = [
240+
'stripe_request_id',
241+
'response',
242+
];
243+
244+
if ( 'POST' === $method ) {
245+
$expected_data_keys[] = 'idempotency_key';
246+
$expected_data_keys[] = 'request';
247+
}
248+
249+
if (
250+
is_wp_error( $response ) &&
251+
'http_request_failed' === $response->get_error_code() &&
252+
// phpcs:ignore WordPress.WP.I18n.MissingArgDomain
253+
__( 'A valid URL was not provided.' ) === $response->get_error_message()
254+
) {
255+
$expected_data_keys[] = 'resolved_ip_address';
256+
$expected_data_keys[] = 'validation_details';
257+
}
258+
259+
$expected_data_keys_callback = function ( $context ) use ( $expected_data_keys ) {
260+
$this->assertLessThanOrEqual( count( $context ), count( $expected_data_keys ) );
261+
foreach ( $expected_data_keys as $key ) {
262+
$this->assertArrayHasKey( $key, $context );
263+
}
264+
return true;
265+
};
266+
267+
$mock_logger->expects( $this->once() )
268+
->method( 'error' )
269+
->with(
270+
$this->stringStartsWith( "Stripe API error: $method $api" ),
271+
$this->callback( $expected_data_keys_callback )
272+
);
273+
274+
add_filter( 'pre_http_request', $pre_http_filter, 10, 3 );
275+
276+
if ( 'GET' === $method ) {
277+
$result = WC_Stripe_API::retrieve( $api );
278+
} else {
279+
$caught_exception = null;
280+
try {
281+
$result = WC_Stripe_API::request( $request_data, $api, $method, false );
282+
} catch ( \WC_Stripe_Exception $stripe_exception ) {
283+
$caught_exception = $stripe_exception;
284+
}
285+
}
286+
287+
// Clean up before we perform any assertions.
288+
remove_filter( 'pre_http_request', $pre_http_filter );
289+
\WC_Stripe_Logger::$logger = null;
290+
291+
if ( 'GET' === $method ) {
292+
$this->assertInstanceof( \WP_Error::class, $result );
293+
$this->assertEquals( 'stripe_error', $result->get_error_code() );
294+
$this->assertEquals( __( 'There was a problem connecting to the Stripe API endpoint.', 'woocommerce-gateway-stripe' ), $result->get_error_message() );
295+
} else {
296+
$this->assertInstanceof( \WC_Stripe_Exception::class, $caught_exception );
297+
$this->assertEquals( print_r( $response, true ), $caught_exception->getMessage() );
298+
$this->assertEquals( __( 'There was a problem connecting to the Stripe API endpoint.', 'woocommerce-gateway-stripe' ), $caught_exception->getLocalizedMessage() );
299+
}
300+
}
301+
302+
/**
303+
* Data provider for {@see test_log_error_response()}.
304+
*
305+
* @return array
306+
*/
307+
public function provide_test_log_error_response_tests(): array {
308+
return [
309+
'generic error for GET account' => [
310+
'response' => new \WP_Error( 'mock_error', 'Mock Error' ),
311+
'api' => 'account',
312+
'method' => 'GET',
313+
],
314+
'generic error for POST account' => [
315+
'response' => new \WP_Error( 'mock_error', 'Mock Error' ),
316+
'api' => 'account',
317+
'method' => 'POST',
318+
'request_data' => [ 'test' => 'test' ],
319+
],
320+
'general http_request_failed error for GET account' => [
321+
'response' => new \WP_Error( 'http_request_failed', 'Mock Error' ),
322+
'api' => 'account',
323+
'method' => 'GET',
324+
],
325+
'general http_request_failed error for POST account' => [
326+
'response' => new \WP_Error( 'http_request_failed', 'Mock Error' ),
327+
'api' => 'account',
328+
'method' => 'POST',
329+
'request_data' => [ 'test' => 'test' ],
330+
],
331+
'URL validation http_request_failed error for GET account' => [
332+
// phpcs:ignore WordPress.WP.I18n.MissingArgDomain
333+
'response' => new \WP_Error( 'http_request_failed', __( 'A valid URL was not provided.' ) ),
334+
'api' => 'account',
335+
'method' => 'GET',
336+
],
337+
'URL validation http_request_failed error for POST account' => [
338+
// phpcs:ignore WordPress.WP.I18n.MissingArgDomain
339+
'response' => new \WP_Error( 'http_request_failed', __( 'A valid URL was not provided.' ) ),
340+
'api' => 'account',
341+
'method' => 'POST',
342+
'request_data' => [ 'test' => 'test' ],
343+
],
344+
'empty response body for GET account' => [
345+
'response' => [
346+
'response' => [
347+
'code' => 200,
348+
'message' => 'OK',
349+
],
350+
'body' => '',
351+
],
352+
'api' => 'account',
353+
'method' => 'GET',
354+
],
355+
'empty response body for POST account' => [
356+
'response' => [
357+
'response' => [
358+
'code' => 200,
359+
'message' => 'OK',
360+
],
361+
'body' => '',
362+
],
363+
'api' => 'account',
364+
'method' => 'POST',
365+
'request_data' => [ 'test' => 'test' ],
366+
],
367+
];
368+
}
369+
213370
public function provide_test_should_detach_payment_method_from_customer(): array {
214371
return [
215372
'test mode from non-admin context should detach' => [

0 commit comments

Comments
 (0)