diff --git a/src/Transport.php b/src/Transport.php index edcc53e..831fcce 100644 --- a/src/Transport.php +++ b/src/Transport.php @@ -116,6 +116,15 @@ class Transport implements EventHandlingInterface, ClientInterface { */ protected $requests_from_last_429 = 0; + /** + * Retry connecting with a longer connect_timeout, if a ConnectException is thrown while attempting to do a request with one of the + * following messages: + * cURL error 28: Connection timed out after 1001 milliseconds + * Operation timed out after <...> milliseconds with 0 out of 0 bytes received + * @var bool + */ + public $retry_on_connection_errors = true; + /** * GuzzleHttp client. * @var Client $client @@ -124,9 +133,9 @@ class Transport implements EventHandlingInterface, ClientInterface { /** * A cookie string to use for requests. - * @var string $cookie + * @var false|string $cookie */ - public $cookie = ''; + public $cookie = false; /** * Static instance of Transport class. @@ -261,26 +270,21 @@ protected function doRequest( $url, $method = self::REQUEST_GET, $options = [] ) $opt[ 'connect_timeout' ] = $this->connect_timeout; } - $opt['headers']['Cookie'] = $this->cookie; + if ( $this->cookie !== false ) { + + $opt[ 'headers' ][ 'Cookie' ] = $this->cookie; + } if ( $this->use_proxy ) { $opt[ 'proxy' ] = $this->proxy_list->getCurrentAddress(); } - if ( $method === self::REQUEST_GET ) { - - return $this->client->get( - $url, - $opt - ); - } else { - - return $this->client->post( - $url, - $opt - ); - } + return $this->client->request( + $method, + $url, + $opt + ); } /** @@ -313,23 +317,29 @@ public function request( string $method, $url, array $options = [] ): ResponseIn $this->trigger( self::EVENT_AFTER_RESPONSE, $event ); } catch ( ConnectException $exception ) { - if ( !$this->use_proxy || ( ( $this->max_proxies_in_a_row !== null ) && ( $count >= $this->max_proxies_in_a_row ) ) ) { + if ( $this->retry_on_connection_errors ) { - throw $exception; - } + if ( ( strpos( $exception->getMessage(), 'with 0 out of 0 bytes received' ) !== false + || strpos( $exception->getMessage(), 'Connection timed out after ' ) !== false ) + && !isset( $options[ '_connect_timeout' ] ) + ) { - if ( strpos( $exception->getMessage(), 'with 0 out of 0 bytes received' ) !== false && !isset( $options['_connect_timeout'] ) ) { + // I've noticed that sometimes, after having received this error, you can extend a connection timeout and it will work, so we try that, + // before failing + $options[ '_connect_timeout' ] = $this->connect_timeout + 5; + continue; + } + } - // I've noticed that sometimes, after having received this error, you can extend a connection timeout and it will work, so we try that, - // before failing - $options['_connect_timeout'] = $this->connect_timeout + 5; - } else { + if ( !$this->use_proxy || ( ( $this->max_proxies_in_a_row !== null ) && ( $count >= $this->max_proxies_in_a_row ) ) ) { - unset( $options['_connect_timeout'] ); - $this->log( $exception->getCode() . ': ' . $exception->getMessage() . "\r\n", true ); - $this->proxy_list->markFailed( $exception ); - $count++; + throw $exception; } + + unset( $options['_connect_timeout'] ); + $this->log( $exception->getCode() . ': ' . $exception->getMessage() . "\r\n", true ); + $this->proxy_list->markFailed( $exception ); + $count++; } catch ( ClientException $exception ) { unset( $options['_connect_timeout'] ); diff --git a/tests/tests/TransportTest.php b/tests/tests/TransportTest.php index 0cec5b6..1bd328b 100644 --- a/tests/tests/TransportTest.php +++ b/tests/tests/TransportTest.php @@ -258,6 +258,70 @@ public function testRequestSuccess() { $this->assertSame( $response, $res ); } + /** + * @covers \unique\proxyswitcher\Transport::request + */ + public function testRequestConnectExceptionNoProxyListTimeOutErrorWithoutRetry() { + + // No proxy list must forward the exception: + $transport = $this->getMockBuilder( Transport::class ) + ->setMethods( [ 'doRequest' ] ) + ->getMock(); + + $transport->retry_on_connection_errors = false; + + $expected_exception = new ConnectException( 'Connection timed out after 1001 milliseconds', $this->createMock( Request::class ) ); + $transport + ->expects( $this->once() ) + ->method( 'doRequest' ) + ->willThrowException( $expected_exception ); + + $exception = null; + + /** + * @var Transport|MockObject $transport + */ + + try { + + $transport->request( Transport::REQUEST_GET, 'url' ); + } catch ( \Exception $exception ) {} + + $this->assertSame( $expected_exception, $exception ); + } + + /** + * @covers \unique\proxyswitcher\Transport::request + */ + public function testRequestConnectExceptionNoProxyListTimeOutErrorWithRetry() { + + // No proxy list must forward the exception: + $transport = $this->getMockBuilder( Transport::class ) + ->setMethods( [ 'doRequest' ] ) + ->getMock(); + + $transport->retry_on_connection_errors = true; + + $expected_exception = new ConnectException( 'Connection timed out after 1001 milliseconds', $this->createMock( Request::class ) ); + $transport + ->expects( $this->exactly( 2 ) ) + ->method( 'doRequest' ) + ->willThrowException( $expected_exception ); + + $exception = null; + + /** + * @var Transport|MockObject $transport + */ + + try { + + $transport->request( Transport::REQUEST_GET, 'url' ); + } catch ( \Exception $exception ) {} + + $this->assertSame( $expected_exception, $exception ); + } + /** * @covers \unique\proxyswitcher\Transport::request */ @@ -663,7 +727,7 @@ public function testRequestRequestExceptionOtherErrors() { */ public function testRequestSwitchTransportOn() { - $client = $this->createConfiguredMock( Client::class, [ 'get' => $this->createMock( Response::class ) ] ); + $client = $this->createConfiguredMock( Client::class, [ 'request' => $this->createMock( Response::class ) ] ); $transport = $this->createPartialMock( Transport::class, [ 'getNewSwitchTransportOn' ] ); $transport @@ -718,7 +782,7 @@ public function testRequestSwitchTransportOn() { */ public function testRequestTimeout() { - $client = $this->createConfiguredMock( Client::class, [ 'get' => $this->createMock( Response::class ) ] ); + $client = $this->createConfiguredMock( Client::class, [ 'request' => $this->createMock( Response::class ) ] ); $transport = $this->createPartialMock( Transport::class, [ 'getNewSwitchTransportOn', 'getNewNextTimeoutOn', 'log' ] ); $transport @@ -762,24 +826,30 @@ public function testRequestConnectOptions() { $client = $this->createMock( Client::class ); $client - ->expects( $this->exactly( 3 ) ) - ->method( 'get' ) + ->expects( $this->exactly( 4 ) ) + ->method( 'request' ) ->withConsecutive( - [ 'url', [ 'connect_timeout' => 1, 'headers' => [ 'Cookie' => '' ] ] ], - [ 'url', [ 'connect_timeout' => 5, 'headers' => [ 'Cookie' => '123' ] ] ], - [ 'url', [ 'connect_timeout' => 10, 'headers' => [ 'Cookie' => '123' ], 'proxy' => 'test' ] ], + [ 'GET', 'url', [ 'connect_timeout' => 1, 'headers' => [ 'Cookie' => '' ] ] ], + [ 'GET', 'url', [ 'connect_timeout' => 5, 'headers' => [ 'Cookie' => '123' ] ] ], + [ 'GET', 'url', [ 'connect_timeout' => 5 ] ], + [ 'GET', 'url', [ 'connect_timeout' => 10, 'headers' => [ 'Cookie' => '123' ], 'proxy' => 'test' ] ], ) ->willReturn( $this->createMock( Response::class ) ); $transport = new Transport(); $transport->setClient( $client ); $transport->connect_timeout = 1; + $transport->cookie = ''; $transport->request( Transport::REQUEST_GET, 'url' ); $transport->cookie = '123'; $transport->request( Transport::REQUEST_GET, 'url', [ '_connect_timeout' => 5 ] ); + $transport->cookie = false; + $transport->request( Transport::REQUEST_GET, 'url', [ '_connect_timeout' => 5 ] ); + + $transport->cookie = '123'; $proxy_list = $this->createMock( ArrayProxyList::class ); $proxy_list ->expects( $this->once() ) @@ -799,16 +869,13 @@ public function testRequestDifferentMethods() { $post_response = $this->createMock( Response::class ); $client - ->expects( $this->once() ) - ->method( 'get' ) - ->with( 'url1', [ 'connect_timeout' => 1, 'headers' => [ 'Cookie' => '' ] ] ) - ->willReturn( $get_response ); - - $client - ->expects( $this->once() ) - ->method( 'post' ) - ->with( 'url2', [ 'connect_timeout' => 1, 'headers' => [ 'Cookie' => '' ] ] ) - ->willReturn( $post_response ); + ->expects( $this->exactly( 2 ) ) + ->method( 'request' ) + ->withConsecutive( + [ 'GET', 'url1', [ 'connect_timeout' => 1 ] ], + [ 'POST', 'url2', [ 'connect_timeout' => 1 ] ], + ) + ->willReturnOnConsecutiveCalls( $get_response, $post_response ); $transport = new Transport(); $transport->setClient( $client );