diff --git a/CHANGELOG.md b/CHANGELOG.md index 9634eec..01219b3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -45,6 +45,18 @@ you were catching the `TypeError` from any of the getters above as a workaround, always permitted; dropping 8.1 is the only real constraint change. ### Changed +- **BREAKING** — **transport failures are now `PostmarkTransportException`, not Guzzle exceptions.** + `processRestRequest()` previously documented `@throws \GuzzleHttp\Exception\GuzzleException`, + making Guzzle's hierarchy part of this SDK's public contract. Because the SDK disables + `http_errors` and maps responses itself, the transport family was the *only* Guzzle family that + could reach your code — so a Guzzle upgrade silently changed what your `catch` blocks matched. + Anything that never reached the API (DNS, connect, TLS, timeout, socket) is now + `Postmark\Models\PostmarkTransportException`, which extends `PostmarkException`, so existing + `catch (PostmarkException $e)` blocks keep working. The original is preserved on `getPrevious()`. + + Replace `catch (\GuzzleHttp\Exception\ConnectException $e)` with + `catch (\Postmark\Models\PostmarkTransportException $e)` and use `$e->isTimeout()` / + `$e->isConnectionFailure()` — the SDK normalises across Guzzle majors so you don't have to. - **BREAKING** — `PostmarkAttachment::fromRawData()`, `::fromBase64EncodedData()` and `::fromFile()` now declare `string` for their first two parameters and a `PostmarkAttachment` return type. Passing `null`, an array, or a non-Stringable object now raises a `TypeError`; previously it diff --git a/src/Postmark/Models/PostmarkTransportException.php b/src/Postmark/Models/PostmarkTransportException.php new file mode 100644 index 0000000..4ce5961 --- /dev/null +++ b/src/Postmark/Models/PostmarkTransportException.php @@ -0,0 +1,74 @@ +message = $message; + $this->kind = $kind; + } + + /** + * The request was not answered within the configured timeout. + * + * Retry is at-least-once, not safe-to-repeat: a timeout means no response was + * seen, which does not prove the message was rejected. + */ + public function isTimeout(): bool + { + return self::KIND_TIMEOUT === $this->kind; + } + + /** + * The connection failed before a response could be produced. + * + * Safe to retry — the request did not complete. + */ + public function isConnectionFailure(): bool + { + return self::KIND_CONNECTION === $this->kind; + } + + /** One of the KIND_* constants. */ + public function getKind(): string + { + return $this->kind; + } +} diff --git a/src/Postmark/PostmarkClientBase.php b/src/Postmark/PostmarkClientBase.php index 9ed5e52..e981f36 100644 --- a/src/Postmark/PostmarkClientBase.php +++ b/src/Postmark/PostmarkClientBase.php @@ -9,8 +9,10 @@ namespace Postmark; use GuzzleHttp\Client; +use GuzzleHttp\Exception\GuzzleException; use GuzzleHttp\RequestOptions; use Postmark\Models\PostmarkException; +use Postmark\Models\PostmarkTransportException; /** * This is the core class that interacts with the Postmark API. All clients should @@ -93,6 +95,55 @@ protected function getClient() return $this->client; } + /** + * Normalise a Guzzle transport failure to a PostmarkTransportException kind. + * + * All Guzzle-version knowledge is confined here. The two majors express the + * same failures differently: + * - Guzzle 7 raises ConnectException for connect AND timeout, and carries a + * cURL errno on the handler context, which is what separates them. + * - Guzzle 8 re-parents ConnectException under NetworkException, DROPS + * getHandlerContext(), and adds precise subclasses instead + * (ConnectTimeoutException, NetworkTimeoutException, ResponseTimeoutException). + * Matching on the short class name keeps this working under both without + * referencing classes that only exist in one. + */ + private static function classifyTransportFailure(GuzzleException $e): string + { + $shortName = substr(strrchr('\\' . get_class($e), '\\') ?: '', 1); + + // Guzzle 8: the class already says which it is. + if (in_array($shortName, ['ConnectTimeoutException', 'NetworkTimeoutException', 'ResponseTimeoutException'], true)) { + return PostmarkTransportException::KIND_TIMEOUT; + } + + // Guzzle 7: disambiguate with the cURL errno when the handler supplied one. + if (method_exists($e, 'getHandlerContext')) { + $errno = $e->getHandlerContext()['errno'] ?? null; + + if (28 === $errno) { + return PostmarkTransportException::KIND_TIMEOUT; + } + + // Resolve, connect, TLS, empty reply, send/recv. + if (in_array($errno, [5, 6, 7, 35, 52, 55, 56], true)) { + return PostmarkTransportException::KIND_CONNECTION; + } + } + + $message = strtolower($e->getMessage()); + + if (str_contains($message, 'timed out') || str_contains($message, 'timeout')) { + return PostmarkTransportException::KIND_TIMEOUT; + } + + if (in_array($shortName, ['ConnectException', 'NetworkException'], true)) { + return PostmarkTransportException::KIND_CONNECTION; + } + + return PostmarkTransportException::KIND_UNKNOWN; + } + /** * The base request method for all API access. * @@ -102,8 +153,8 @@ protected function getClient() * * @return mixed * - * @throws PostmarkException - * @throws \GuzzleHttp\Exception\GuzzleException + * @throws PostmarkException if the API returns an error response + * @throws PostmarkTransportException if the API could not be reached at all */ protected function processRestRequest($method = null, $path = null, array $body = []): mixed { @@ -142,7 +193,20 @@ protected function processRestRequest($method = null, $path = null, array $body } } - $response = $client->request($method, self::$BASE_URL . $path, $options); + try { + $response = $client->request($method, self::$BASE_URL . $path, $options); + } catch (GuzzleException $e) { + // The HTTP client's exception hierarchy is not part of this SDK's contract. + // Guzzle 8 reclassified the transport family — a plain timeout stopped being + // a ConnectException — and with HTTP_ERRORS disabled that family is the only + // one that can reach a caller, so leaking it made every consumer's catch + // block version-dependent. + throw new PostmarkTransportException( + sprintf('Could not reach the Postmark API: %s', $e->getMessage()), + $e, + self::classifyTransportFailure($e) + ); + } switch ($response->getStatusCode()) { case 200: diff --git a/tests/TransportExceptionTest.php b/tests/TransportExceptionTest.php new file mode 100644 index 0000000..e64508b --- /dev/null +++ b/tests/TransportExceptionTest.php @@ -0,0 +1,141 @@ +clientThatFailsWith( + new ConnectException('cURL error 7: Failed to connect', new Request('POST', 'test')) + ); + + try { + $client->sendEmail('a@example.com', 'b@example.com', 'Subject', null, 'Body'); + $this->fail('Expected a PostmarkTransportException.'); + } catch (PostmarkTransportException $e) { + $this->assertStringContainsString('Could not reach the Postmark API', $e->getMessage()); + $this->assertInstanceOf(ConnectException::class, $e->getPrevious()); + } + } + + /** Existing `catch (PostmarkException)` blocks must keep working. */ + public function testItIsCatchableAsAPostmarkException(): void + { + $client = $this->clientThatFailsWith( + new ConnectException('cURL error 7', new Request('POST', 'test')) + ); + + $this->expectException(PostmarkException::class); + $client->sendEmail('a@example.com', 'b@example.com', 'Subject', null, 'Body'); + } + + /** + * The point of the change: a caller can classify the failure without + * importing anything from GuzzleHttp, and gets the same answer on either + * major even though the underlying class differs. + */ + public function testCallersCanClassifyWithoutTouchingGuzzle(): void + { + $timeout = $this->captureFrom($this->connectFailure('cURL error 28: Operation timed out', 28)); + + $this->assertTrue($timeout->isTimeout()); + $this->assertFalse($timeout->isConnectionFailure()); + + $refused = $this->captureFrom($this->connectFailure('cURL error 7: Failed to connect', 7)); + + $this->assertTrue($refused->isConnectionFailure()); + $this->assertFalse($refused->isTimeout()); + } + + /** Handlers that report no errno still classify, via the message. */ + public function testClassificationFallsBackToTheMessageWithoutAnErrno(): void + { + $timeout = $this->captureFrom( + new ConnectException('cURL error 28: Operation timed out', new Request('POST', 'test')) + ); + + $this->assertTrue($timeout->isTimeout()); + } + + /** A response that arrives carrying an error is NOT a transport failure. */ + public function testApiErrorsAreStillPlainPostmarkExceptions(): void + { + $mock = new MockHandler([new \GuzzleHttp\Psr7\Response(401, [], '{}')]); + $guzzle = new Client(['handler' => HandlerStack::create($mock)]); + $client = new PostmarkClient('test-token'); + $client->setClient($guzzle); + + try { + $client->sendEmail('a@example.com', 'b@example.com', 'Subject', null, 'Body'); + $this->fail('Expected a PostmarkException.'); + } catch (PostmarkTransportException $e) { + $this->fail('A 401 is a response, not a transport failure.'); + } catch (PostmarkException $e) { + $this->assertSame(401, $e->getHttpStatusCode()); + } + } + + /** + * Guzzle 7's ConnectException accepts a handler context (which is where the + * cURL errno lives); Guzzle 8 removed that parameter and uses precise + * subclasses instead. Build whichever the installed major supports. + */ + private function connectFailure(string $message, int $errno): GuzzleException + { + $class = new \ReflectionClass(ConnectException::class); + $arguments = [$message, new Request('POST', 'test')]; + + // Built reflectively rather than with a literal 4-argument call: PHPStan + // resolves the constructor against whichever major is installed and would + // reject the wider form under Guzzle 8, even guarded. + if ($class->getConstructor()->getNumberOfParameters() >= 4) { + $arguments[] = null; + $arguments[] = ['errno' => $errno]; + } + + return $class->newInstanceArgs($arguments); + } + + private function captureFrom(GuzzleException $failure): PostmarkTransportException + { + try { + $this->clientThatFailsWith($failure) + ->sendEmail('a@example.com', 'b@example.com', 'Subject', null, 'Body'); + } catch (PostmarkTransportException $e) { + return $e; + } + + $this->fail('Expected a PostmarkTransportException.'); + } + + private function clientThatFailsWith(GuzzleException $failure): PostmarkClient + { + $guzzle = new Client(['handler' => HandlerStack::create(new MockHandler([$failure]))]); + $client = new PostmarkClient('test-token'); + $client->setClient($guzzle); + + return $client; + } +}