By default log failed requests to a file + test improvments

- Do not catch PHPUnits exceptions
- Add base tests for Laravel to ensure all classes can be created by container

Author: pionl

composer.json

@@ -27,6 +27,8 @@
         "league/flysystem": "^3.14",
         "mockery/mockery": "^1.5.1",
         "nyholm/psr7": "^1.8.0",
+        "orchestra/testbench": "^7.26",
+        "php-http/mock-client": "^1.6",
         "phpstan/phpstan": "1.10.26",
         "phpstan/phpstan-deprecation-rules": "^1.1.3",
         "phpstan/phpstan-mockery": "^1.1.1",
@@ -40,6 +42,9 @@
         "wrkflow/larastrict": "Improve your Laravel code base with strict conventions."
     },
     "scripts": {
+        "post-autoload-dump": [
+            "@php vendor/bin/testbench package:discover --ansi"
+        ],
         "check": "composer lint && composer test",
         "lint:check": "./vendor/bin/ecs",
         "lint:fix": "./vendor/bin/ecs --fix",

phpunit.xml

@@ -17,4 +17,9 @@
             <directory>./tests/</directory>
         </testsuite>
     </testsuites>
+    <php>
+        <env name="APP_KEY" value="AckfSECXIvnK5r28GVIWUAxmbBSjTsmF"/>
+        <env name="DB_CONNECTION" value="testing"/>
+    </php>
+
 </phpunit>

src/Actions/SendRequestAction.php

@@ -97,7 +97,7 @@ public function execute(
     }
 
     /**
-     * @param class-string<AbstractResponse>                           $responseClass
+     * @param class-string<AbstractResponse> $responseClass
      */
     private function sendRequest(
         AbstractEnvironment $environment,
@@ -127,6 +127,8 @@ private function sendRequest(
                 ->client()
                 ->sendRequest($request);
         } catch (Exception $exception) {
+            $this->rethrowIfPHPUnitException($exception);
+
             $event = new RequestConnectionFailedEvent(
                 id: $requestId,
                 request: $request,
@@ -167,7 +169,7 @@ private function getRequestDuration(float $timeStart): float
      * @template TResponse of AbstractResponse
      *
      * @param class-string<TResponse> $responseClass
-     * @param int|null                $expectedResponseStatusCode                           Will raise and failed
+     * @param int|null                $expectedResponseStatusCode Will raise and failed
      *                                                                                      exception if response
      *                                                                                      status code is different
      *
@@ -211,6 +213,8 @@ private function handleResponse(
 
             throw $api->createFailedResponseException($statusCode, $response);
         } catch (Exception $exception) {
+            $this->rethrowIfPHPUnitException($exception);
+
             // Wrap any custom exception to response exception because we have a response
             if ($exception instanceof ResponseException === false) {
                 $exception = new ResponseException($response, $exception->getMessage(), $exception);
@@ -241,4 +245,17 @@ private function buildRequest(
         $request = $this->buildHeadersAction->execute($mergedHeaders, $request);
         return $this->withBody($api, $body, $request);
     }
+
+    /**
+     * If we are using assert mocking, we do not want to log the exception and
+     * rethrow it.
+     */
+    private function rethrowIfPHPUnitException(Exception $exception): void
+    {
+        // PHPUnit can be missing.
+        $class = '\\' . \PHPUnit\Exception::class;
+        if ($exception instanceof $class) {
+            throw $exception;
+        }
+    }
 }

src/Laravel/Configs/api_sdk.php

@@ -20,7 +20,7 @@
          * - Log multiple domains: 'domain.com:debug,otherdomain.com:info',
          * - Log all except given domain: '*:info,domain.com:'
          */
-        ApiSdkConfig::KeyLoggingType => env('API_SDK_LOGGING', '*:info'),
+        ApiSdkConfig::KeyLoggingType => env('API_SDK_LOGGING', '*:info_file'),
         /**
          * List of available loggers key-ed by their name and their implementation.
          *

src/Laravel/LaravelServiceProvider.php

@@ -19,7 +19,6 @@
 use League\Flysystem\FilesystemOperator;
 use LogicException;
 use Psr\EventDispatcher\EventDispatcherInterface;
-use Psr\Log\LoggerInterface;
 use WrkFlow\ApiSdkBuilder\Actions\MakeApiFactory;
 use WrkFlow\ApiSdkBuilder\Contracts\ApiFactoryContract;
 use WrkFlow\ApiSdkBuilder\Contracts\SDKContainerFactoryContract;
@@ -30,20 +29,22 @@
 use WrkFlow\ApiSdkBuilder\Laravel\Configs\ApiSdkConfig;
 use WrkFlow\ApiSdkBuilder\Log\Actions\BuildRequestHttpFileAction;
 use WrkFlow\ApiSdkBuilder\Log\Actions\ClearFileLogsAction;
-use WrkFlow\ApiSdkBuilder\Log\Actions\GetExtensionFromContentTypeAction;
 use WrkFlow\ApiSdkBuilder\Log\Contracts\BuildRequestLogFileActionContract;
 use WrkFlow\ApiSdkBuilder\Log\Contracts\DebugLoggerContract;
 use WrkFlow\ApiSdkBuilder\Log\Contracts\FileLoggerContract;
 use WrkFlow\ApiSdkBuilder\Log\Contracts\FileLogPathServiceContract;
 use WrkFlow\ApiSdkBuilder\Log\Contracts\InfoLoggerContract;
+use WrkFlow\ApiSdkBuilder\Log\Contracts\InfoOrFailFileLoggerContract;
 use WrkFlow\ApiSdkBuilder\Log\Entities\LoggerConfigEntity;
 use WrkFlow\ApiSdkBuilder\Log\Loggers\DebugLogger;
 use WrkFlow\ApiSdkBuilder\Log\Loggers\FileLogger;
 use WrkFlow\ApiSdkBuilder\Log\Loggers\InfoLogger;
+use WrkFlow\ApiSdkBuilder\Log\Loggers\InfoOrFailFileLogger;
 use WrkFlow\ApiSdkBuilder\Log\Services\FileLogPathService;
 
 class LaravelServiceProvider extends ServiceProvider
 {
+    final public const KeyFilesystemOperator = 'api_sdk_filesystem_operator';
     private const ConfigPath = __DIR__ . '/Configs/api_sdk.php';
     private const ConfigKey = 'api_sdk';
 
@@ -70,8 +71,9 @@ public function boot(): void
         $clearAt = $config->getTimeForClearSchedule();
 
         if ($clearAt !== null) {
-            /** @var Schedule $schedule */
             $schedule = $this->app->make(Schedule::class);
+            assert($schedule instanceof Schedule);
+
             $schedule->command(ClearFileLogsCommand::class)
                 ->dailyAt($clearAt);
         }
@@ -105,8 +107,8 @@ protected function bindApi(): void
         $this->app->singleton(
             abstract: ApiFactoryContract::class,
             concrete: static function (Container $container): ApiFactoryContract {
-                /** @var MakeApiFactory $makeApiFactory */
                 $makeApiFactory = $container->make(MakeApiFactory::class);
+                assert($makeApiFactory instanceof MakeApiFactory);
 
                 return $makeApiFactory->execute(loggerConfig: $container->make(LoggerConfigEntity::class));
             }
@@ -119,27 +121,33 @@ protected function bindLogs(): void
             abstract: BuildRequestLogFileActionContract::class,
             concrete: BuildRequestHttpFileAction::class
         );
+        $this->app->singleton(abstract: ClearFileLogsAction::class, concrete: ClearFileLogsAction::class);
         $this->app->singleton(abstract: FileLogPathServiceContract::class, concrete: FileLogPathService::class);
         $this->app->singleton(abstract: DebugLoggerContract::class, concrete: DebugLogger::class);
         $this->app->singleton(abstract: FileLoggerContract::class, concrete: FileLogger::class);
         $this->app->singleton(abstract: InfoLoggerContract::class, concrete: InfoLogger::class);
+        $this->app->singleton(abstract: InfoOrFailFileLoggerContract::class, concrete: InfoOrFailFileLogger::class);
 
-        $this->app->singleton(
-            abstract: FileLogger::class,
-            concrete: static fn (Application $application) => new FileLogger(
-                filesystemOperator: static::getFileSystemOperator($application),
-                logger: $application->make(LoggerInterface::class),
-                fileLogPathService: $application->make(FileLogPathService::class),
-                getExtensionFromContentTypeAction: $application->make(GetExtensionFromContentTypeAction::class),
-                buildRequestLogFileAction: $application->make(BuildRequestLogFileActionContract::class),
-            )
+        // Not sure if we provided FilesystemOperator to whole Laravel application
+        // if it would break some package usage... So lets be explicit.
+        $this->app->bind(
+            abstract: self::KeyFilesystemOperator,
+            concrete: static fn (Application $application) => static::getFileSystemOperator($application)
         );
-
-        $this->app->bind(ClearFileLogsAction::class, static fn (Application $application) => new ClearFileLogsAction(
-            filesystemOperator: static::getFileSystemOperator($application),
-            fileLogPathService: $application->make(FileLogPathService::class),
-            logger: $application->make(LoggerInterface::class),
-        ));
+        $this->app
+            ->when(FileLogger::class)
+            ->needs(FilesystemOperator::class)
+            ->give(self::KeyFilesystemOperator);
+
+        $this->app
+            ->when(FileLogger::class)
+            ->needs(FilesystemOperator::class)
+            ->give(self::KeyFilesystemOperator);
+
+        $this->app
+            ->when(ClearFileLogsAction::class)
+            ->needs(FilesystemOperator::class)
+            ->give(self::KeyFilesystemOperator);
     }
 
     protected function getEventDispatcher(): Dispatcher

src/Log/Constants/LoggerConstants.php

@@ -7,17 +7,24 @@
 use WrkFlow\ApiSdkBuilder\Log\Contracts\DebugLoggerContract;
 use WrkFlow\ApiSdkBuilder\Log\Contracts\FileLoggerContract;
 use WrkFlow\ApiSdkBuilder\Log\Contracts\InfoLoggerContract;
+use WrkFlow\ApiSdkBuilder\Log\Contracts\InfoOrFailFileLoggerContract;
+use WrkFlow\ApiSdkBuilder\Log\Contracts\LoggerContract;
 
 class LoggerConstants
 {
+    /**
+     * @var array<string, array<class-string<LoggerContract>>>
+     */
     final public const DefaultLoggersMap = [
         self::NoLog => [],
         self::Info => [InfoLoggerContract::class],
         self::Debug => [DebugLoggerContract::class],
         self::File => [InfoLoggerContract::class, FileLoggerContract::class],
+        self::InfoOrFailFile => [InfoOrFailFileLoggerContract::class],
     ];
     final public const NoLog = '';
     final public const Info = 'info';
+    final public const InfoOrFailFile = 'info_file';
     final public const Debug = 'debug';
     final public const File = 'file';
 }

src/Log/Contracts/InfoOrFailFileLoggerContract.php

@@ -0,0 +1,9 @@
+<?php
+
+declare(strict_types=1);
+
+namespace WrkFlow\ApiSdkBuilder\Log\Contracts;
+
+interface InfoOrFailFileLoggerContract extends LoggerContract
+{
+}

src/Log/Loggers/InfoLogger.php

@@ -64,9 +64,9 @@ private function debug(
             ? 'FAILED'
             : (string) $response->getStatusCode();
 
-        // example.com GET 200 /api/v1/clients/1 [20s]
-        // example.com GET 500 /api/v1/clients/1 [20s]
-        // example.com GET FAILED /api/v1/clients/1 [20s]
+        // GET 200 example.com /api/v1/clients/1 [20s]
+        // GET example.com 500 /api/v1/clients/1 [20s]
+        // GET example.com FAILED /api/v1/clients/1 [20s]
         $this->logger->info(
             message: sprintf(
                 '%s %s %s %s [%ds]',

src/Log/Loggers/InfoOrFailFileLogger.php

@@ -0,0 +1,39 @@
+<?php
+
+declare(strict_types=1);
+
+namespace WrkFlow\ApiSdkBuilder\Log\Loggers;
+
+use WrkFlow\ApiSdkBuilder\Events\RequestConnectionFailedEvent;
+use WrkFlow\ApiSdkBuilder\Events\RequestFailedEvent;
+use WrkFlow\ApiSdkBuilder\Events\ResponseReceivedEvent;
+use WrkFlow\ApiSdkBuilder\Log\Contracts\FileLoggerContract;
+use WrkFlow\ApiSdkBuilder\Log\Contracts\InfoLoggerContract;
+use WrkFlow\ApiSdkBuilder\Log\Contracts\InfoOrFailFileLoggerContract;
+use WrkFlow\ApiSdkBuilder\Log\Entities\LoggerConfigEntity;
+
+class InfoOrFailFileLogger implements InfoOrFailFileLoggerContract
+{
+    public function __construct(
+        private readonly InfoLoggerContract $logger,
+        private readonly FileLoggerContract $fileLogger
+    ) {
+    }
+
+    public function responseReceivedEvent(ResponseReceivedEvent $event, LoggerConfigEntity $config): void
+    {
+        $this->logger->responseReceivedEvent($event, $config);
+    }
+
+    public function requestFailed(RequestFailedEvent $event, LoggerConfigEntity $config): void
+    {
+        $this->logger->requestFailed($event, $config);
+        $this->fileLogger->requestFailed($event, $config);
+    }
+
+    public function requestConnectionFailed(RequestConnectionFailedEvent $event, LoggerConfigEntity $config): void
+    {
+        $this->logger->requestConnectionFailed($event, $config);
+        $this->fileLogger->requestConnectionFailed($event, $config);
+    }
+}

tests/Actions/BuildHeadersActionTest.php

@@ -2,7 +2,7 @@
 
 declare(strict_types=1);
 
-namespace Wrkflow\ApiSdkBuilderTests\Actions;
+namespace WrkFlow\ApiSdkBuilderTests\Actions;
 
 use Nyholm\Psr7\Request;
 use PHPUnit\Framework\TestCase;