Skip to content

Commit

Permalink
- New option to retry on connection error
Browse files Browse the repository at this point in the history 
- bugfix: cookie option is now optional by default
- bugfix: request method works with all requests not just get/post.
  • Loading branch information
@uniquexor
uniquexor committed May 10, 2021
1 parent b880b9d commit e52e889
Show file tree
Hide file tree
Showing 2 changed files with 122 additions and 45 deletions.
66 changes: 38 additions & 28 deletions src/Transport.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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.
Expand Down Expand Up @@ -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
);
}

/**
Expand Down Expand Up @@ -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'] );
Expand Down
101 changes: 84 additions & 17 deletions tests/tests/TransportTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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() )
Expand All @@ -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 );
Expand Down

0 comments on commit e52e889

Please sign in to comment.