Requests: Handle requests.failed hook correctly in case of redirects

src/Exception.php

@@ -29,6 +29,13 @@ class Exception extends PHPException {
  */
  protected $data;
 
+ /**
+ * Whether the exception was already passed to the requests.failed hook or not
+ *
+ * @var boolean
+ */
+ public $failed_hook_handled = FALSE;
+
  /**
  * Create a new exception
  *

src/Exception/InvalidArgument.php

@@ -12,6 +12,13 @@
  */
 final class InvalidArgument extends InvalidArgumentException {
 
+ /**
+ * Whether the exception was already passed to the requests.failed hook or not
+ *
+ * @var boolean
+ */
+ public $failed_hook_handled = FALSE;
+
  /**
  * Create a new invalid argument exception with a standardized text.
  *

src/Requests.php

@@ -474,7 +474,10 @@ public static function request($url, $headers = [], $data = [], $type = self::GE
 
  $parsed_response = self::parse_response($response, $url, $headers, $data, $options);
  } catch (Exception|InvalidArgument $e) {
- $options['hooks']->dispatch('requests.failed', [&$e, $url, $headers, $data, $type, $options]);
+ if ($e->failed_hook_handled === FALSE) {
+ $options['hooks']->dispatch('requests.failed', [&$e, $url, $headers, $data, $type, $options]);
+ $e->failed_hook_handled = TRUE;
+ }
  throw $e;
  }
 

tests/Fixtures/TransportRedirectMock.php

@@ -0,0 +1,113 @@
+<?php
+
+namespace WpOrg\Requests\Tests\Fixtures;
+
+use WpOrg\Requests\Transport;
+
+final class TransportRedirectMock implements Transport {
+ public $code = 302;
+ public $chunked = false;
+ public $body = '';
+ public $raw_headers = '';
+
+ private $redirected = [];
+
+ public $redirected_transport = NULL;
+
+ private static $messages = [
+ 100 => '100 Continue',
+ 101 => '101 Switching Protocols',
+ 200 => '200 OK',
+ 201 => '201 Created',
+ 202 => '202 Accepted',
+ 203 => '203 Non-Authoritative Information',
+ 204 => '204 No Content',
+ 205 => '205 Reset Content',
+ 206 => '206 Partial Content',
+ 300 => '300 Multiple Choices',
+ 301 => '301 Moved Permanently',
+ 302 => '302 Found',
+ 303 => '303 See Other',
+ 304 => '304 Not Modified',
+ 305 => '305 Use Proxy',
+ 306 => '306 (Unused)',
+ 307 => '307 Temporary Redirect',
+ 400 => '400 Bad Request',
+ 401 => '401 Unauthorized',
+ 402 => '402 Payment Required',
+ 403 => '403 Forbidden',
+ 404 => '404 Not Found',
+ 405 => '405 Method Not Allowed',
+ 406 => '406 Not Acceptable',
+ 407 => '407 Proxy Authentication Required',
+ 408 => '408 Request Timeout',
+ 409 => '409 Conflict',
+ 410 => '410 Gone',
+ 411 => '411 Length Required',
+ 412 => '412 Precondition Failed',
+ 413 => '413 Request Entity Too Large',
+ 414 => '414 Request-URI Too Long',
+ 415 => '415 Unsupported Media Type',
+ 416 => '416 Requested Range Not Satisfiable',
+ 417 => '417 Expectation Failed',
+ 418 => '418 I\'m a teapot',
+ 428 => '428 Precondition Required',
+ 429 => '429 Too Many Requests',
+ 431 => '431 Request Header Fields Too Large',
+ 500 => '500 Internal Server Error',
+ 501 => '501 Not Implemented',
+ 502 => '502 Bad Gateway',
+ 503 => '503 Service Unavailable',
+ 504 => '504 Gateway Timeout',
+ 505 => '505 HTTP Version Not Supported',
+ 511 => '511 Network Authentication Required',
+ ];
+
+ public function request($url, $headers = [], $data = [], $options = []) {
+ if (array_key_exists($url, $this->redirected))
+ {
+ return $this->redirected_transport->request($url, $headers, $data, $options);
+ }
+
+ $redirect_url = "https://example.com/redirected?url=" . urlencode($url);
+
+ $status = isset(self::$messages[$this->code]) ? self::$messages[$this->code] : $this->code . ' unknown';
+ $response = "HTTP/1.0 $status\r\n";
+ $response .= "Content-Type: text/plain\r\n";
+ if ($this->chunked) {
+ $response .= "Transfer-Encoding: chunked\r\n";
+ }
+ $response .= "Location: $redirect_url\r\n";
+ $response .= $this->raw_headers;
+ $response .= "Connection: close\r\n\r\n";
+ $response .= $this->body;
+
+ $this->redirected[$url] = TRUE;
+ $this->redirected[$redirect_url] = TRUE;
+
+ return $response;
+ }
+
+ public function request_multiple($requests, $options) {
+ $responses = [];
+ foreach ($requests as $id => $request) {
+ $handler = new self();
+ $handler->code = $request['options']['mock.code'];
+ $handler->chunked = $request['options']['mock.chunked'];
+ $handler->body = $request['options']['mock.body'];
+ $handler->raw_headers = $request['options']['mock.raw_headers'];
+ $responses[$id] = $handler->request($request['url'], $request['headers'], $request['data'], $request['options']);
+
+ if (!empty($options['mock.parse'])) {
+ $request['options']['hooks']->dispatch('transport.internal.parse_response', [&$responses[$id], $request]);
+ $request['options']['hooks']->dispatch('multiple.request.complete', [&$responses[$id], $id]);
+ }
+ }
+
+ return $responses;
+ }
+
+ public static function test($capabilities = []) {
+ return true;
+ }
+}

tests/RequestsTest.php

@@ -20,6 +20,7 @@
 use WpOrg\Requests\Tests\Fixtures\TransportFailedMock;
 use WpOrg\Requests\Tests\Fixtures\TransportInvalidArgumentMock;
 use WpOrg\Requests\Tests\Fixtures\TransportMock;
+use WpOrg\Requests\Tests\Fixtures\TransportRedirectMock;
 
 final class RequestsTest extends TestCase {
 
@@ -177,7 +178,7 @@ public function testDefaultTransport() {
 
  public function testTransportFailedTriggersRequestsFailedCallback() {
  $mock = $this->getMockBuilder(stdClass::class)->setMethods(['failed'])->getMock();
- $mock->expects($this->atLeastOnce())->method('failed');
+ $mock->expects($this->once())->method('failed');
  $hooks = new Hooks();
  $hooks->register('requests.failed', [$mock, 'failed']);
 
@@ -195,7 +196,7 @@ public function testTransportFailedTriggersRequestsFailedCallback() {
 
  public function testTransportInvalidArgumentTriggersRequestsFailedCallback() {
  $mock = $this->getMockBuilder(stdClass::class)->setMethods(['failed'])->getMock();
- $mock->expects($this->atLeastOnce())->method('failed');
+ $mock->expects($this->once())->method('failed');
  $hooks = new Hooks();
  $hooks->register('requests.failed', [$mock, 'failed']);
 
@@ -319,7 +320,7 @@ public function testInvalidProtocolVersion() {
  */
  public function testInvalidProtocolVersionTriggersRequestsFailedCallback() {
  $mock = $this->getMockBuilder(stdClass::class)->setMethods(['failed'])->getMock();
- $mock->expects($this->atLeastOnce())->method('failed');
+ $mock->expects($this->once())->method('failed');
  $hooks = new Hooks();
  $hooks->register('requests.failed', [$mock, 'failed']);
 
@@ -357,7 +358,7 @@ public function testSingleCRLFSeparator() {
  */
  public function testSingleCRLFSeparatorTriggersRequestsFailedCallback() {
  $mock = $this->getMockBuilder(stdClass::class)->setMethods(['failed'])->getMock();
- $mock->expects($this->atLeastOnce())->method('failed');
+ $mock->expects($this->once())->method('failed');
  $hooks = new Hooks();
  $hooks->register('requests.failed', [$mock, 'failed']);
 
@@ -389,7 +390,7 @@ public function testInvalidStatus() {
 
  public function testInvalidStatusTriggersRequestsFailedCallback() {
  $mock = $this->getMockBuilder(stdClass::class)->setMethods(['failed'])->getMock();
- $mock->expects($this->atLeastOnce())->method('failed');
+ $mock->expects($this->once())->method('failed');
  $hooks = new Hooks();
  $hooks->register('requests.failed', [$mock, 'failed']);
 
@@ -418,6 +419,52 @@ public function test30xWithoutLocation() {
  $this->assertSame(0, $response->redirects);
  }
 
+ public function testRedirectToExceptionTriggersRequestsFailedCallbackOnce() {
+ $mock = $this->getMockBuilder(stdClass::class)->setMethods(['failed'])->getMock();
+ $mock->expects($this->once())->method('failed');
+ $hooks = new Hooks();
+ $hooks->register('requests.failed', [$mock, 'failed']);
+
+ $transport = new TransportRedirectMock();
+ $transport->redirected_transport = new TransportFailedMock();
+
+ $options = [
+ 'hooks' => $hooks,
+ 'transport' => $transport,
+ ];
+
+ $this->expectException(Exception::class);
+ $this->expectExceptionMessage('Transport failed!');
+
+ $response = Requests::get('http://example.com/', [], $options);
+
+ $this->assertSame(302, $response->status_code);
+ $this->assertSame(1, $response->redirects);
+ }
+
+ public function testRedirectToInvalidArgumentTriggersRequestsFailedCallbackOnce() {
+ $mock = $this->getMockBuilder(stdClass::class)->setMethods(['failed'])->getMock();
+ $mock->expects($this->once())->method('failed');
+ $hooks = new Hooks();
+ $hooks->register('requests.failed', [$mock, 'failed']);
+
+ $transport = new TransportRedirectMock();
+ $transport->redirected_transport = new TransportInvalidArgumentMock();
+
+ $options = [
+ 'hooks' => $hooks,
+ 'transport' => $transport,
+ ];
+
+ $this->expectException(InvalidArgument::class);
+ $this->expectExceptionMessage('Argument #1 ($url) must be of type string|Stringable');
+
+ $response = Requests::get('http://example.com/', [], $options);
+
+ $this->assertSame(302, $response->status_code);
+ $this->assertSame(1, $response->redirects);
+ }
+
  public function testTimeoutException() {
  $options = ['timeout' => 0.5];
  $this->expectException(Exception::class);