Skip to content

Commit

Permalink
Merge pull request #26 from clue-labs/error-messages
Browse files Browse the repository at this point in the history 
Improve error reporting by always including target URI in exceptions
  • Loading branch information
@clue
clue authored Oct 25, 2018
2 parents 99300c6 + eb15d61 commit f04365d
Show file tree
Hide file tree
Showing 4 changed files with 123 additions and 53 deletions.
48 changes: 34 additions & 14 deletions src/ProxyConnector.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -146,9 +146,9 @@ public function connect($uri)

$connecting = $this->connector->connect($proxyUri);

$deferred = new Deferred(function ($_, $reject) use ($connecting) {
$deferred = new Deferred(function ($_, $reject) use ($connecting, $uri) {
$reject(new RuntimeException(
'Connection cancelled while waiting for proxy (ECONNABORTED)',
'Connection to ' . $uri . ' cancelled while waiting for proxy (ECONNABORTED)',
defined('SOCKET_ECONNABORTED') ? SOCKET_ECONNABORTED : 103
));

Expand All @@ -160,10 +160,10 @@ public function connect($uri)
});

$headers = $this->headers;
$connecting->then(function (ConnectionInterface $stream) use ($target, $headers, $deferred) {
$connecting->then(function (ConnectionInterface $stream) use ($target, $headers, $deferred, $uri) {
// keep buffering data until headers are complete
$buffer = '';
$stream->on('data', $fn = function ($chunk) use (&$buffer, $deferred, $stream, &$fn) {
$stream->on('data', $fn = function ($chunk) use (&$buffer, $deferred, $stream, &$fn, $uri) {
$buffer .= $chunk;

$pos = strpos($buffer, "\r\n\r\n");
Expand All @@ -176,19 +176,29 @@ public function connect($uri)
try {
$response = Psr7\parse_response(substr($buffer, 0, $pos));
} catch (Exception $e) {
$deferred->reject(new RuntimeException('Invalid response received from proxy (EBADMSG)', defined('SOCKET_EBADMSG') ? SOCKET_EBADMSG: 71, $e));
$deferred->reject(new RuntimeException(
'Connection to ' . $uri . ' failed because proxy returned invalid response (EBADMSG)',
defined('SOCKET_EBADMSG') ? SOCKET_EBADMSG: 71,
$e
));
$stream->close();
return;
}

if ($response->getStatusCode() === 407) {
// map status code 407 (Proxy Authentication Required) to EACCES
$deferred->reject(new RuntimeException('Proxy denied connection due to invalid authentication ' . $response->getStatusCode() . ' (' . $response->getReasonPhrase() . ') (EACCES)', defined('SOCKET_EACCES') ? SOCKET_EACCES : 13));
$deferred->reject(new RuntimeException(
'Connection to ' . $uri . ' failed because proxy denied access with HTTP error code ' . $response->getStatusCode() . ' (' . $response->getReasonPhrase() . ') (EACCES)',
defined('SOCKET_EACCES') ? SOCKET_EACCES : 13
));
$stream->close();
return;
} elseif ($response->getStatusCode() < 200 || $response->getStatusCode() >= 300) {
// map non-2xx status code to ECONNREFUSED
$deferred->reject(new RuntimeException('Proxy refused connection with HTTP error code ' . $response->getStatusCode() . ' (' . $response->getReasonPhrase() . ') (ECONNREFUSED)', defined('SOCKET_ECONNREFUSED') ? SOCKET_ECONNREFUSED : 111));
$deferred->reject(new RuntimeException(
'Connection to ' . $uri . ' failed because proxy refused connection with HTTP error code ' . $response->getStatusCode() . ' (' . $response->getReasonPhrase() . ') (ECONNREFUSED)',
defined('SOCKET_ECONNREFUSED') ? SOCKET_ECONNREFUSED : 111
));
$stream->close();
return;
}
Expand All @@ -207,23 +217,33 @@ public function connect($uri)

// stop buffering when 8 KiB have been read
if (isset($buffer[8192])) {
$deferred->reject(new RuntimeException('Proxy must not send more than 8 KiB of headers (EMSGSIZE)', defined('SOCKET_EMSGSIZE') ? SOCKET_EMSGSIZE : 90));
$deferred->reject(new RuntimeException(
'Connection to ' . $uri . ' failed because proxy response headers exceed maximum of 8 KiB (EMSGSIZE)',
defined('SOCKET_EMSGSIZE') ? SOCKET_EMSGSIZE : 90
));
$stream->close();
}
});

$stream->on('error', function (Exception $e) use ($deferred) {
$deferred->reject(new RuntimeException('Stream error while waiting for response from proxy (EIO)', defined('SOCKET_EIO') ? SOCKET_EIO : 5, $e));
$stream->on('error', function (Exception $e) use ($deferred, $uri) {
$deferred->reject(new RuntimeException(
'Connection to ' . $uri . ' failed because connection to proxy caused a stream error (EIO)',
defined('SOCKET_EIO') ? SOCKET_EIO : 5,
$e
));
});

$stream->on('close', function () use ($deferred) {
$deferred->reject(new RuntimeException('Connection to proxy lost while waiting for response (ECONNRESET)', defined('SOCKET_ECONNRESET') ? SOCKET_ECONNRESET : 104));
$stream->on('close', function () use ($deferred, $uri) {
$deferred->reject(new RuntimeException(
'Connection to ' . $uri . ' failed because connection to proxy was lost while waiting for response (ECONNRESET)',
defined('SOCKET_ECONNRESET') ? SOCKET_ECONNRESET : 104
));
});

$stream->write("CONNECT " . $target . " HTTP/1.1\r\nHost: " . $target . "\r\n" . $headers . "\r\n");
}, function (Exception $e) use ($deferred) {
}, function (Exception $e) use ($deferred, $uri) {
$deferred->reject($e = new RuntimeException(
'Unable to connect to proxy (ECONNREFUSED)',
'Connection to ' . $uri . ' failed because connection to proxy failed (ECONNREFUSED)',
defined('SOCKET_ECONNREFUSED') ? SOCKET_ECONNREFUSED : 111,
$e
));
Expand Down
34 changes: 8 additions & 26 deletions tests/AbstractTestCase.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -32,37 +32,19 @@ protected function expectCallableOnceWith($value)
$mock
->expects($this->once())
->method('__invoke')
->with($this->equalTo($value));
->with($value);

return $mock;
}

protected function expectCallableOnceWithExceptionCode($code)
protected function expectCallableOnceWithException($class, $message, $code)
{
$mock = $this->createCallableMock();
$mock
->expects($this->once())
->method('__invoke')
->with($this->logicalAnd(
$this->isInstanceOf('Exception'),
$this->callback(function ($e) use ($code) {
return $e->getCode() === $code;
})
));

return $mock;
}


protected function expectCallableOnceParameter($type)
{
$mock = $this->createCallableMock();
$mock
->expects($this->once())
->method('__invoke')
->with($this->isInstanceOf($type));

return $mock;
return $this->expectCallableOnceWith($this->logicalAnd(
$this->isInstanceOf($class),
$this->callback(function (\Exception $e) use ($message, $code) {
return strpos($e->getMessage(), $message) !== false && $e->getCode() === $code;
})
));
}

/**
Expand Down
24 changes: 20 additions & 4 deletions tests/FunctionalTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,11 @@ public function testNonListeningSocketRejectsConnection()

$promise = $proxy->connect('google.com:80');

$this->setExpectedException('RuntimeException', 'Unable to connect to proxy', SOCKET_ECONNREFUSED);
$this->setExpectedException(
'RuntimeException',
'Connection to tcp://google.com:80 failed because connection to proxy failed (ECONNREFUSED)',
SOCKET_ECONNREFUSED
);
Block\await($promise, $this->loop, 3.0);
}

Expand All @@ -44,7 +48,11 @@ public function testPlainGoogleDoesNotAcceptConnectMethod()

$promise = $proxy->connect('google.com:80');

$this->setExpectedException('RuntimeException', '405 (Method Not Allowed)', SOCKET_ECONNREFUSED);
$this->setExpectedException(
'RuntimeException',
'Connection to tcp://google.com:80 failed because proxy refused connection with HTTP error code 405 (Method Not Allowed) (ECONNREFUSED)',
SOCKET_ECONNREFUSED
);
Block\await($promise, $this->loop, 3.0);
}

Expand All @@ -59,7 +67,11 @@ public function testSecureGoogleDoesNotAcceptConnectMethod()

$promise = $proxy->connect('google.com:80');

$this->setExpectedException('RuntimeException', '405 (Method Not Allowed)', SOCKET_ECONNREFUSED);
$this->setExpectedException(
'RuntimeException',
'Connection to tcp://google.com:80 failed because proxy refused connection with HTTP error code 405 (Method Not Allowed) (ECONNREFUSED)',
SOCKET_ECONNREFUSED
);
Block\await($promise, $this->loop, 3.0);
}

Expand All @@ -69,7 +81,11 @@ public function testSecureGoogleDoesNotAcceptPlainStream()

$promise = $proxy->connect('google.com:80');

$this->setExpectedException('RuntimeException', 'Connection to proxy lost', SOCKET_ECONNRESET);
$this->setExpectedException(
'RuntimeException',
'Connection to tcp://google.com:80 failed because connection to proxy was lost while waiting for response (ECONNRESET)',
SOCKET_ECONNRESET
);
Block\await($promise, $this->loop, 3.0);
}

Expand Down
70 changes: 61 additions & 9 deletions tests/ProxyConnectorTest.php
View file
Original file line number Diff line number Diff line change
Expand Up @@ -285,16 +285,24 @@ public function testRejectsUriWithNonTcpScheme()
$promise->then(null, $this->expectCallableOnce());
}

public function testRejectsIfConnectorRejects()
public function testRejectsWithPreviousIfConnectorRejects()
{
$promise = \React\Promise\reject(new \RuntimeException());
$promise = \React\Promise\reject($previous = new \RuntimeException());
$this->connector->expects($this->once())->method('connect')->willReturn($promise);

$proxy = new ProxyConnector('proxy.example.com', $this->connector);

$promise = $proxy->connect('google.com:80');

$promise->then(null, $this->expectCallableOnce());
$promise->then(null, $this->expectCallableOnceWithException(
'RuntimeException',
'Connection to tcp://google.com:80 failed because connection to proxy failed (ECONNREFUSED)',
SOCKET_ECONNREFUSED
));

$promise->then(null, $this->expectCallableOnceWith($this->callback(function (\Exception $e) use ($previous) {
return $e->getPrevious() === $previous;
})));
}

public function testRejectsAndClosesIfStreamWritesNonHttp()
Expand All @@ -311,7 +319,11 @@ public function testRejectsAndClosesIfStreamWritesNonHttp()
$stream->expects($this->once())->method('close');
$stream->emit('data', array("invalid\r\n\r\n"));

$promise->then(null, $this->expectCallableOnceWithExceptionCode(SOCKET_EBADMSG));
$promise->then(null, $this->expectCallableOnceWithException(
'RuntimeException',
'Connection to tcp://google.com:80 failed because proxy returned invalid response (EBADMSG)',
SOCKET_EBADMSG
));
}

public function testRejectsAndClosesIfStreamWritesTooMuchData()
Expand All @@ -326,9 +338,13 @@ public function testRejectsAndClosesIfStreamWritesTooMuchData()
$promise = $proxy->connect('google.com:80');

$stream->expects($this->once())->method('close');
$stream->emit('data', array(str_repeat('*', 100000)));
$stream->emit('data', array(str_repeat('*', 10000)));

$promise->then(null, $this->expectCallableOnceWithExceptionCode(SOCKET_EMSGSIZE));
$promise->then(null, $this->expectCallableOnceWithException(
'RuntimeException',
'Connection to tcp://google.com:80 failed because proxy response headers exceed maximum of 8 KiB (EMSGSIZE)',
SOCKET_EMSGSIZE
));
}

public function testRejectsAndClosesIfStreamReturnsProyAuthenticationRequired()
Expand All @@ -345,7 +361,11 @@ public function testRejectsAndClosesIfStreamReturnsProyAuthenticationRequired()
$stream->expects($this->once())->method('close');
$stream->emit('data', array("HTTP/1.1 407 Proxy Authentication Required\r\n\r\n"));

$promise->then(null, $this->expectCallableOnceWithExceptionCode(SOCKET_EACCES));
$promise->then(null, $this->expectCallableOnceWithException(
'RuntimeException',
'Connection to tcp://google.com:80 failed because proxy denied access with HTTP error code 407 (Proxy Authentication Required) (EACCES)',
SOCKET_EACCES
));
}

public function testRejectsAndClosesIfStreamReturnsNonSuccess()
Expand All @@ -362,7 +382,35 @@ public function testRejectsAndClosesIfStreamReturnsNonSuccess()
$stream->expects($this->once())->method('close');
$stream->emit('data', array("HTTP/1.1 403 Not allowed\r\n\r\n"));

$promise->then(null, $this->expectCallableOnceWithExceptionCode(SOCKET_ECONNREFUSED));
$promise->then(null, $this->expectCallableOnceWithException(
'RuntimeException',
'Connection to tcp://google.com:80 failed because proxy refused connection with HTTP error code 403 (Not allowed) (ECONNREFUSED)',
SOCKET_ECONNREFUSED
));
}

public function testRejectsWithPreviousExceptionIfStreamEmitsError()
{
$stream = $this->getMockBuilder('React\Socket\Connection')->disableOriginalConstructor()->setMethods(array('close', 'write'))->getMock();

$promise = \React\Promise\resolve($stream);
$this->connector->expects($this->once())->method('connect')->willReturn($promise);

$proxy = new ProxyConnector('proxy.example.com', $this->connector);

$promise = $proxy->connect('google.com:80');

$stream->emit('error', array($previous = new \RuntimeException()));

$promise->then(null, $this->expectCallableOnceWithException(
'RuntimeException',
'Connection to tcp://google.com:80 failed because connection to proxy caused a stream error (EIO)',
SOCKET_EIO
));

$promise->then(null, $this->expectCallableOnceWith($this->callback(function (\Exception $e) use ($previous) {
return $e->getPrevious() === $previous;
})));
}

public function testResolvesIfStreamReturnsSuccess()
Expand Down Expand Up @@ -423,7 +471,11 @@ public function testCancelPromiseWhileConnectionIsReadyWillCloseOpenConnectionAn

$promise->cancel();

$promise->then(null, $this->expectCallableOnceWithExceptionCode(SOCKET_ECONNABORTED));
$promise->then(null, $this->expectCallableOnceWithException(
'RuntimeException',
'Connection to tcp://google.com:80 cancelled while waiting for proxy (ECONNABORTED)',
SOCKET_ECONNABORTED
));
}

public function testCancelPromiseDuringConnectionShouldNotCreateGarbageCycles()
Expand Down

0 comments on commit f04365d

Please sign in to comment.