better rfc7807 client implementation

Triplkrypl · Triplkrypl · commit f1e85a51b905 · 2025-02-20T22:38:13.000+01:00
diff --git a/.github/README.md b/.github/README.md
@@ -54,7 +54,6 @@ Deprecated or Sunset headers will be logged into defined log channel.
 $deprecationsLogger = new \SimpleAsFuck\ApiToolkit\Service\Client\DeprecationsLogger(
     $config,
     $logger,
-    new \GuzzleHttp\Psr7\HttpFactory()
 );
 
 $client = new \SimpleAsFuck\ApiToolkit\Service\Client\ApiClient(
@@ -103,8 +102,19 @@ catch (\SimpleAsFuck\ApiToolkit\Model\Client\ApiException $exception) {
      * \SimpleAsFuck\ApiToolkit\Model\Client\ApiException is thrown,
      * and you can handle any error from communication
      */
-    $exception->getCode(); // if exception contains http response, http status is here, otherwise zero is returned
-    $exception->getMessage(); // if http response contains json object with message string property, json message overwrite exception message
+    // if exception contains http response, rfc7807 status or http status is here, otherwise zero is returned
+    $exception->getCode();
+    // exception message for logging or debugging is build from https://datatracker.ietf.org/doc/html/rfc7807
+    // extended with optional message property, you SHOULD log this, so you know WTF is going wrong
+    $logger->error($exception->getMessage()); 
+    // short information for end user WTF just happened, if is not null you CAN show the tittle on your front end
+    $exception->getTitle();
+    // information for end user with more detail, if is not null you SHOULD show the detail on your front end,
+    // because detail can contain clue or information how user can solve error, mainly if error is his false :D
+    $exception->getDetail();
+    // parse from error response some extensions, is RECOMMENDED ignoring all errors from error response parsing
+    // because you can lose another useful data from error response or if response si corrupted you can lose previous exception
+    $exception->response()?->getJson(allowInvalidJson: true)->object()->property('some_error_property')->string()->nullable(failAsNull: true);
 }
 
 ```
diff --git a/src/Factory/Client/ParseResponseException.php b/src/Factory/Client/ParseResponseException.php
@@ -18,6 +18,15 @@ public function __construct(
 
     public function create(string $message): \Exception
     {
-        return new \SimpleAsFuck\ApiToolkit\Model\Client\ParseResponseException($message, $this->request, $this->response);
+        return new \SimpleAsFuck\ApiToolkit\Model\Client\ParseResponseException(
+            $message,
+            $this->response->getStatusCode(),
+            $this->request->url(),
+            null,
+            null,
+            null,
+            $this->request,
+            $this->response
+        );
     }
 }
diff --git a/src/Model/Client/ApiException.php b/src/Model/Client/ApiException.php
@@ -6,13 +6,58 @@
 
 class ApiException extends \RuntimeException
 {
+    /**
+     * @param string $message for logging or debugging purposes MUST contain only English message
+     * @param int $code https://datatracker.ietf.org/doc/html/rfc7807#section-3.1 status or HTTP status or 0 if nothing is available
+     * @param non-empty-string $instance https://datatracker.ietf.org/doc/html/rfc7807#section-3.1 instance
+     * @param non-empty-string|null $type https://datatracker.ietf.org/doc/html/rfc7807#section-3.1 type, if not available look at message
+     * @param non-empty-string|null $title https://datatracker.ietf.org/doc/html/rfc7807#section-3.1 title message for end user
+     * @param non-empty-string|null $detail https://datatracker.ietf.org/doc/html/rfc7807#section-3.1 detail for end user
+     */
     public function __construct(
         string $message,
+        int $code,
+        private readonly string $instance,
+        private readonly ?string $type,
+        private readonly ?string $title,
+        private readonly ?string $detail,
         private readonly Request $request,
-        private readonly ?Response $response = null,
-        \Throwable $previous = null
+        private readonly ?Response $response,
+        ?\Throwable $previous = null
     ) {
-        parent::__construct($message, $response?->getStatusCode() ?? 0, $previous);
+        parent::__construct($message, $code, $previous);
+    }
+
+    /**
+     * @return non-empty-string|null
+     */
+    public function getInstance(): ?string
+    {
+        return $this->instance;
+    }
+
+    /**
+     * @return non-empty-string|null
+     */
+    public function getType(): ?string
+    {
+        return $this->type;
+    }
+
+    /**
+     * @return non-empty-string|null
+     */
+    public function getTitle(): ?string
+    {
+        return $this->title;
+    }
+
+    /**
+     * @return non-empty-string|null
+     */
+    public function getDetail(): ?string
+    {
+        return $this->detail;
     }
 
     public function request(): Request
diff --git a/src/Model/Client/Request.php b/src/Model/Client/Request.php
@@ -87,6 +87,22 @@ public function withJson(mixed $jsonData, ?Transformer $transformer, int $jsonEn
         return $request;
     }
 
+    /**
+     * @return non-empty-string
+     */
+    public function method(): string
+    {
+        return $this->method;
+    }
+
+    /**
+     * @return non-empty-string
+     */
+    public function url(): string
+    {
+        return $this->url;
+    }
+
     public function hasBaseUrl(): bool
     {
         return $this->baseUrl !== null;
diff --git a/src/Model/Client/ResponseApiException.php b/src/Model/Client/ResponseApiException.php
@@ -6,13 +6,26 @@
 
 class ResponseApiException extends ApiException
 {
+    /**
+     * @param string $message for logging or debugging purposes MUST contain only English message
+     * @param int $code https://datatracker.ietf.org/doc/html/rfc7807#section-3.1 status or HTTP status
+     * @param non-empty-string $instance https://datatracker.ietf.org/doc/html/rfc7807#section-3.1 instance
+     * @param non-empty-string|null $type https://datatracker.ietf.org/doc/html/rfc7807#section-3.1 type, if not available look at message
+     * @param non-empty-string|null $title https://datatracker.ietf.org/doc/html/rfc7807#section-3.1 title message for end user
+     * @param non-empty-string|null $detail https://datatracker.ietf.org/doc/html/rfc7807#section-3.1 detail for end user
+     */
     final public function __construct(
         string $message,
+        int $code,
+        string $instance,
+        ?string $type,
+        ?string $title,
+        ?string $detail,
         Request $request,
         private readonly Response $response,
-        \Throwable $previous = null
+        ?\Throwable $previous = null
     ) {
-        parent::__construct($message, $request, $response, $previous);
+        parent::__construct($message, $code, $instance, $type, $title, $detail, $request, $response, $previous);
     }
 
     final public function response(): Response
diff --git a/src/Provider/LaravelProvider.php b/src/Provider/LaravelProvider.php
@@ -33,20 +33,20 @@ public function register(): void
             $config = $this->app->make(\Illuminate\Contracts\Config\Repository::class);
             /** @var Config $apiConfig */
             $apiConfig = $this->app->make(Config::class);
-            /** @var RequestFactoryInterface $requestFactory */
-            $requestFactory = $this->app->make(RequestFactoryInterface::class);
 
             $deprecationLogger = $config->get('logging.deprecations');
             if (is_string($deprecationLogger)) {
                 /** @var LogManager $logManager */
                 $logManager = $this->app->make(LogManager::class);
-                $deprecationLogger = new DeprecationsLogger($apiConfig, $logManager->channel($deprecationLogger), $requestFactory);
+                $deprecationLogger = new DeprecationsLogger($apiConfig, $logManager->channel($deprecationLogger));
             } else {
                 $deprecationLogger = null;
             }
 
             /** @var Client $client */
             $client = $this->app->make(Client::class);
+            /** @var RequestFactoryInterface $requestFactory */
+            $requestFactory = $this->app->make(RequestFactoryInterface::class);
 
             return new ApiClient($apiConfig, $client, $requestFactory, $deprecationLogger);
         });
diff --git a/src/Service/Client/ApiClient.php b/src/Service/Client/ApiClient.php
@@ -190,62 +190,62 @@ public function waitRaw(ResponsePromise $promise): Response
 
                 $responseContent = $response->getBody()->getContents();
                 $errorObject = Validator::make(\json_decode($responseContent))->object();
-                $errorParts = [];
+                $messageParts = [];
 
-                // https://datatracker.ietf.org/doc/html/rfc7807
+                // https://datatracker.ietf.org/doc/html/rfc7807#section-3.1 type
                 $errorType = $errorObject->property('type')->string()->notEmpty()->nullable(true);
                 if ($errorType !== null) {
-                    $errorParts[] = 'Error type: "'.$errorType.'"';
+                    $messageParts[] = 'Error type: "'.$errorType.'"';
                 }
 
-                $errorMessage =
-                    $errorObject
-                        ->property('title')
-                        ->string()
-                        ->notEmpty()
-                        ->nullable(true)
-                    ??
-                    $errorObject
-                        ->property('message')
-                        ->string()
-                        ->notEmpty()
-                        ->nullable(true)
-                ;
+                $errorMessage = $errorObject->property('message')->string()->notEmpty()->nullable(true);
                 if ($errorMessage !== null) {
-                    $errorParts[] = $errorMessage;
+                    $messageParts[] = $errorMessage;
                 }
 
-                if (count($errorParts) !== 0) {
-                    $errorStatus = $errorObject->property('status')->int()->nullable(true);
-                    if ($errorStatus !== null) {
-                        $errorParts[] = 'status ('.$errorStatus.')';
+                // https://datatracker.ietf.org/doc/html/rfc7807#section-3.1 status
+                $errorStatus = $errorObject->property('status')->int()->nullable(true) ?? $response->getStatusCode();
+                // https://datatracker.ietf.org/doc/html/rfc7807#section-3.1 instance
+                $errorInstance = $errorObject->property('instance')->string()->notEmpty()->nullable(true) ?? $promise->request->url();
+                // https://datatracker.ietf.org/doc/html/rfc7807#section-3.1 title
+                $errorTitle = $errorObject->property('title')->string()->notEmpty()->nullable(true);
+                // https://datatracker.ietf.org/doc/html/rfc7807#section-3.1 detail
+                $errorDetail = $errorObject->property('detail')->string()->notEmpty()->nullable(true);
+
+                if (count($messageParts) === 0) {
+                    if ($errorTitle !== null) {
+                        $messageParts[] = 'Error title: "'.$errorTitle.'"';
                     }
-                    $errorInstance = $errorObject->property('instance')->string()->notEmpty()->nullable(true);
-                    if ($errorInstance !== null) {
-                        $errorParts[] = 'instance: "'.$errorInstance.'"';
+                    if ($errorDetail !== null) {
+                        $messageParts[] = 'Error detail: "'.$errorDetail.'"';
                     }
+                }
+
+                if (count($messageParts) !== 0) {
+                    $messageParts[] = 'status (' . $errorStatus . ')';
+                    $messageParts[] = 'instance: "' . $errorInstance . '"';
 
-                    $message = implode(' ', $errorParts);
+                    $message = implode(' ', $messageParts);
                 }
 
                 $response = $response->withBody((new HttpFactory())->createStream($responseContent));
                 $response = new Response($promise->request, $response);
 
                 match ($response->getStatusCode()) {
-                    HttpCodes::HTTP_BAD_REQUEST => throw new BadRequestApiException($message, $promise->request, $response, $exception),
-                    HttpCodes::HTTP_UNAUTHORIZED => throw new UnauthorizedApiException($message, $promise->request, $response, $exception),
-                    HttpCodes::HTTP_FORBIDDEN => throw new ForbiddenApiException($message, $promise->request, $response, $exception),
-                    HttpCodes::HTTP_NOT_FOUND => throw new NotFoundApiException($message, $promise->request, $response, $exception),
-                    HttpCodes::HTTP_CONFLICT => throw new ConflictApiException($message, $promise->request, $response, $exception),
-                    HttpCodes::HTTP_GONE => throw new GoneApiException($message, $promise->request, $response, $exception),
-                    HttpCodes::HTTP_INTERNAL_SERVER_ERROR => throw new InternalServerErrorApiException($message, $promise->request, $response, $exception),
-                    default => throw new ResponseApiException($message, $promise->request, $response, $exception),
+                    HttpCodes::HTTP_BAD_REQUEST => throw new BadRequestApiException($message, $errorStatus, $errorInstance, $errorType, $errorTitle, $errorDetail, $promise->request, $response, $exception),
+                    HttpCodes::HTTP_UNAUTHORIZED => throw new UnauthorizedApiException($message, $errorStatus, $errorInstance, $errorType, $errorTitle, $errorDetail, $promise->request, $response, $exception),
+                    HttpCodes::HTTP_FORBIDDEN => throw new ForbiddenApiException($message, $errorStatus, $errorInstance, $errorType, $errorTitle, $errorDetail, $promise->request, $response, $exception),
+                    HttpCodes::HTTP_NOT_FOUND => throw new NotFoundApiException($message, $errorStatus, $errorInstance, $errorType, $errorTitle, $errorDetail, $promise->request, $response, $exception),
+                    HttpCodes::HTTP_CONFLICT => throw new ConflictApiException($message, $errorStatus, $errorInstance, $errorType, $errorTitle, $errorDetail, $promise->request, $response, $exception),
+                    HttpCodes::HTTP_GONE => throw new GoneApiException($message, $errorStatus, $errorInstance, $errorType, $errorTitle, $errorDetail, $promise->request, $response, $exception),
+                    HttpCodes::HTTP_INTERNAL_SERVER_ERROR => throw new InternalServerErrorApiException($message, $errorStatus, $errorInstance, $errorType, $errorTitle, $errorDetail, $promise->request, $response, $exception),
+                    default => throw new ResponseApiException($message, $errorStatus, $errorInstance, $errorType, $errorTitle, $errorDetail, $promise->request, $response, $exception),
                 };
             }
 
-            throw new ApiException($message, $promise->request, null, $exception);
+            throw new ApiException($message, 0, $promise->request->url(), null, null, null, $promise->request, $response, $exception);
         } catch (TransferException $exception) {
-            throw new ApiException($exception->getMessage(), $promise->request, null, $exception);
+            throw new ApiException($exception->getMessage(), 0, $promise->request->url(), null, null, null, $promise->request, null, $exception);
         }
 
         $this->deprecationsLogger?->logDeprecation($promise->apiName, $promise->request, $response);
diff --git a/src/Service/Client/DeprecationsLogger.php b/src/Service/Client/DeprecationsLogger.php
@@ -4,7 +4,6 @@
 
 namespace SimpleAsFuck\ApiToolkit\Service\Client;
 
-use Psr\Http\Message\RequestFactoryInterface;
 use Psr\Http\Message\ResponseInterface;
 use Psr\Log\LoggerInterface;
 use SimpleAsFuck\ApiToolkit\Model\Client\Request;
@@ -16,7 +15,6 @@ class DeprecationsLogger
     public function __construct(
         private readonly Config $config,
         private readonly LoggerInterface $logger,
-        private readonly RequestFactoryInterface $requestFactory
     ) {
     }
 
@@ -59,8 +57,7 @@ public function logDeprecation(string $apiName, Request $request, ResponseInterf
         }
 
         if (count($deprecatedContext) !== 0) {
-            $request = $request->createPsr($this->requestFactory);
-            $this->logger->warning('Api: '.$apiName.' method: '.$request->getMethod().' uri: "'.$request->getUri()->__toString().'" call is deprecated', $deprecatedContext);
+            $this->logger->warning('Api: '.$apiName.' method: '.$request->method().' url: "'.$request->url().'" call is deprecated', $deprecatedContext);
         }
     }
 }
diff --git a/test/Service/Client/ApiClientTest.php b/test/Service/Client/ApiClientTest.php
@@ -63,7 +63,7 @@ public static function dataProviderWaitRawFail(): array
             [400, 'Exception message', '', new RequestException('Exception message', $request, $response)],
             [
                 400,
-                'Json message',
+                'Json message status (400) instance: "/"',
                 '{"message":"Json message"}',
                 new RequestException(
                     'Exception message',
@@ -73,7 +73,7 @@ public static function dataProviderWaitRawFail(): array
             ],
             [
                 400,
-                'Json title',
+                'Error title: "Json title" status (400) instance: "/"',
                 '{"title":"Json title"}',
                 new RequestException(
                     'Exception message',
@@ -83,14 +83,34 @@ public static function dataProviderWaitRawFail(): array
             ],
             [
                 400,
-                'Error type: "/test/error" Json title status (401) instance: "/test/url"',
+                'Json message status (400) instance: "/"',
+                '{"title":"Json title","message":"Json message"}',
+                new RequestException(
+                    'Exception message',
+                    $request,
+                    $response->withBody($httpFactory->createStream('{"title":"Json title","message":"Json message"}'))
+                ),
+            ],
+            [
+                401,
+                'Error type: "/test/error" status (401) instance: "/test/url"',
                 '{"title":"Json title","type":"/test/error","status":401,"instance":"/test/url"}',
                 new RequestException(
                     'Exception message',
                     $request,
                     $response->withBody($httpFactory->createStream('{"title":"Json title","type":"/test/error","status":401,"instance":"/test/url"}'))
                 ),
             ],
+            [
+                403,
+                'Error type: "/test/error" Json message status (403) instance: "/"',
+                '{"type":"/test/error","message":"Json message"}',
+                new RequestException(
+                    'Exception message',
+                    $request,
+                    $response->withStatus(403)->withBody($httpFactory->createStream('{"type":"/test/error","message":"Json message"}'))
+                ),
+            ],
         ];
     }
 }
diff --git a/test/Service/Client/DeprecationsLoggerTest.php b/test/Service/Client/DeprecationsLoggerTest.php

Original file line number	Diff line number	Diff line change
`@@ -18,6 +18,15 @@ public function __construct(`
`18`	`18`
`19`	`19`	`public function create(string $message): \Exception`
`20`	`20`	`{`
`21`		`- return new \SimpleAsFuck\ApiToolkit\Model\Client\ParseResponseException($message, $this->request, $this->response);`
	`21`	`+ return new \SimpleAsFuck\ApiToolkit\Model\Client\ParseResponseException(`
	`22`	`+ $message,`
	`23`	`+ $this->response->getStatusCode(),`
	`24`	`+ $this->request->url(),`
	`25`	`+ null,`
	`26`	`+ null,`
	`27`	`+ null,`
	`28`	`+ $this->request,`
	`29`	`+ $this->response`
	`30`	`+ );`
`22`	`31`	`}`
`23`	`32`	`}`
Original file line number	Diff line number	Diff line change
`@@ -4,7 +4,6 @@`
`4`	`4`
`5`	`5`	`namespace SimpleAsFuck\ApiToolkit\Service\Client;`
`6`	`6`
`7`		`-use Psr\Http\Message\RequestFactoryInterface;`
`8`	`7`	`use Psr\Http\Message\ResponseInterface;`
`9`	`8`	`use Psr\Log\LoggerInterface;`
`10`	`9`	`use SimpleAsFuck\ApiToolkit\Model\Client\Request;`
`@@ -16,7 +15,6 @@ class DeprecationsLogger`
`16`	`15`	`public function __construct(`
`17`	`16`	`private readonly Config $config,`
`18`	`17`	`private readonly LoggerInterface $logger,`
`19`		`- private readonly RequestFactoryInterface $requestFactory`
`20`	`18`	`) {`
`21`	`19`	`}`
`22`	`20`
`@@ -59,8 +57,7 @@ public function logDeprecation(string $apiName, Request $request, ResponseInterf`
`59`	`57`	`}`
`60`	`58`
`61`	`59`	`if (count($deprecatedContext) !== 0) {`
`62`		`- $request = $request->createPsr($this->requestFactory);`
`63`		`- $this->logger->warning('Api: '.$apiName.' method: '.$request->getMethod().' uri: "'.$request->getUri()->__toString().'" call is deprecated', $deprecatedContext);`
	`60`	`+ $this->logger->warning('Api: '.$apiName.' method: '.$request->method().' url: "'.$request->url().'" call is deprecated', $deprecatedContext);`
`64`	`61`	`}`
`65`	`62`	`}`
`66`	`63`	`}`