Skip to content

Commit

Permalink
Restore exceptions for BC
Browse files Browse the repository at this point in the history
Closes #249.
  • Loading branch information
trowski committed Jan 13, 2020
1 parent 5016b14 commit ff2f6bb
Show file tree
Hide file tree
Showing 4 changed files with 60 additions and 21 deletions.
16 changes: 16 additions & 0 deletions src/Connection/Http2ConnectionException.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
<?php

namespace Amp\Http\Client\Connection;

use Amp\Http\Client\HttpException;

/**
* @deprecated Exception moved to amphp/http. Catch the base exception class (HttpException) instead.
*/
final class Http2ConnectionException extends HttpException
{
public function __construct(string $message, int $code, ?\Throwable $previous = null)
{
parent::__construct($message, $code, $previous);
}
}
25 changes: 25 additions & 0 deletions src/Connection/Http2StreamException.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
<?php

namespace Amp\Http\Client\Connection;

use Amp\Http\Client\HttpException;

/**
* @deprecated Exception moved to amphp/http. Catch the base exception class (HttpException) instead.
*/
final class Http2StreamException extends HttpException
{
/** @var int */
private $streamId;

public function __construct(string $message, int $streamId, int $code, ?\Throwable $previous = null)
{
parent::__construct($message, $code, $previous);
$this->streamId = $streamId;
}

public function getStreamId(): int
{
return $this->streamId;
}
}
36 changes: 17 additions & 19 deletions src/Connection/Internal/Http2ConnectionProcessor.php
Original file line number Diff line number Diff line change
Expand Up @@ -199,7 +199,7 @@ public function handleShutdown(int $lastId, int $error): void
$error
);

$this->shutdown($lastId, new Http2ConnectionException($message, $error));
$this->shutdown($lastId, new HttpException($message, $error));
}

public function handleStreamWindowIncrement(int $streamId, int $windowSize): void
Expand Down Expand Up @@ -632,12 +632,10 @@ public function handleStreamException(Http2StreamException $exception): void
$id = $exception->getStreamId();
$code = $exception->getCode();

$exception = new HttpException($exception->getMessage(), 0, $exception);

This comment has been minimized.

Copy link
@kelunik

kelunik Jan 13, 2020

Member

This should be an Http2StreamException to keep BC.


if ($code === Http2Parser::REFUSED_STREAM) {
$exception = new UnprocessedRequestException(new HttpException(
$exception->getMessage(),
0,
$exception
));
$exception = new UnprocessedRequestException($exception);
}

$this->writeFrame(Http2Parser::RST_STREAM, Http2Parser::NO_FLAG, $id, \pack("N", $code));
Expand All @@ -649,7 +647,7 @@ public function handleStreamException(Http2StreamException $exception): void

public function handleConnectionException(Http2ConnectionException $exception): void
{
$this->shutdown(null, $exception);
$this->shutdown(null, new HttpException($exception->getMessage(), 0, $exception));

This comment has been minimized.

Copy link
@kelunik

kelunik Jan 13, 2020

Member

This should be an Http2ConnectionException to keep BC.

}

public function handleData(int $streamId, string $data): void
Expand Down Expand Up @@ -994,9 +992,9 @@ private function run(): \Generator
\assert($return === null);
}

$this->shutdown(null);
$this->shutdown();
} catch (\Throwable $exception) {
$this->shutdown(null, $exception);
$this->shutdown(null, new HttpException("The HTTP/2 connection closed unexpectedly", 0, $exception));
}
}

Expand Down Expand Up @@ -1167,13 +1165,13 @@ private function releaseStream(int $streamId, ?\Throwable $exception = null): vo

$stream = $this->streams[$streamId];

$exception = $exception ?? new Http2StreamException(
"Stream closed unexpectedly",
$streamId,
Http2Parser::INTERNAL_ERROR
);

if ($stream->responsePending || $stream->body || $stream->trailers) {
$exception = $exception ?? new HttpException(\sprintf("Stream %d closed unexpectedly", $streamId));

if (!$exception instanceof HttpException && !$exception instanceof CancelledException) {
$exception = new HttpException($exception->getMessage(), 0, $exception);
}

/** @var Deferred[]|Emitter[] $deferredAndEmitter */
$deferredAndEmitter = [];

Expand Down Expand Up @@ -1287,13 +1285,13 @@ private function ping(): Promise
}

/**
* @param int|null $lastId ID of last processed frame. Null to use the last opened frame ID or 0 if no
* streams have been opened.
* @param \Throwable|null $reason
* @param int|null $lastId ID of last processed frame. Null to use the last opened frame ID or 0 if no
* streams have been opened.
* @param HttpException|null $reason
*
* @return Promise
*/
private function shutdown(?int $lastId = null, ?\Throwable $reason = null): Promise
private function shutdown(?int $lastId = null, ?HttpException $reason = null): Promise
{
if ($this->onClose === null) {
return new Success;
Expand Down
4 changes: 2 additions & 2 deletions test/Connection/Http2ConnectionTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@

namespace Amp\Http\Client\Connection;

use Amp\Http\Client\HttpException;
use Amp\Http\Client\Request;
use Amp\Http\Client\Response;
use Amp\Http\Client\Trailers;
use Amp\Http\HPack;
use Amp\Http\Http2\Http2ConnectionException;
use Amp\Http\Http2\Http2Parser;
use Amp\Http\Status;
use Amp\NullCancellationToken;
Expand Down Expand Up @@ -78,7 +78,7 @@ public function testSwitchingProtocols(): \Generator
["date", formatDateHeader()],
]), Http2Parser::HEADERS, Http2Parser::END_HEADERS, 1));

$this->expectException(Http2ConnectionException::class);

This comment has been minimized.

Copy link
@kelunik

kelunik Jan 13, 2020

Member

This change shows it's not backwards compatible.

This comment has been minimized.

Copy link
@trowski

trowski Jan 13, 2020

Author Member

We never specified that Http2ConnectionException would be thrown from any interface… but whatever I guess, I'll just change the exceptions thrown.

This comment has been minimized.

Copy link
@trowski

trowski Jan 13, 2020

Author Member

Ok, hopefully 5b8cc0d does it. 🤞

$this->expectException(HttpException::class);
$this->expectExceptionMessage('Switching Protocols (101) is not part of HTTP/2');

yield $stream->request($request, new NullCancellationToken);
Expand Down

0 comments on commit ff2f6bb

Please sign in to comment.