App state resseting error fix (#43)

Co-authored-by: Tetiuev Pavel <jetexe2@gmail.com>

CHANGELOG.md

@@ -4,6 +4,25 @@ All notable changes to this project will be documented in this file.
 
 The format is based on [Keep a Changelog][keepachangelog] and this project adheres to [Semantic Versioning][semver].
 
+## v3.3.0
+
+### Added
+
+- Event `LoopErrorOccurredEvent` (triggered on request processing exception)
+- Listener `SendExceptionToStderrListener` for direct exception sending (as a string) into `stderr`
+- Listener `StopWorkerListener` for worker stopping
+
+### Changed
+
+- Default package configuration includes `LoopErrorOccurredEvent` event listeners: `SendExceptionToStderrListener` and `StopWorkerListener` [#42]
+- When "debug mode" (`app.debug`) is **not** enabled - client will get only `Internal server error` string instead exception with stacktrace [#42]
+
+### Fixed
+
+- Double response sending on request processing error (calling `$psr7_client->respond` and `$psr7_client->getWorker()->error` after that)
+
+[#42]:https://github.com/avto-dev/roadrunner-laravel/issues/42
+
 ## v3.2.1
 
 ### Changed

README.md

@@ -18,7 +18,7 @@ Easy way for connecting [RoadRunner][roadrunner] and [Laravel][laravel] applicat
 Require this package with composer using next command:
 
 ```shell script
-$ composer require avto-dev/roadrunner-laravel "^3.0"
+$ composer require avto-dev/roadrunner-laravel "^3.3"
 ```
 
 > Installed `composer` is required ([how to install composer][getcomposer]).
@@ -47,14 +47,15 @@ After that you can modify configuration files as you wish.
 
 After package installation you can use provided "binary" file as RoadRunner worker: `./vendor/bin/rr-worker`. This worker allows you to interact with incoming requests and outcoming responses using [laravel events system][laravel_events]. Also events contains:
 
-Event classname              | Application object | HTTP server request | HTTP request | HTTP response 
----------------------------- | :----------------: | :-----------------: | :----------: | :-----------:
-`BeforeLoopStartedEvent`     |          ✔         |                     |              |
-`BeforeLoopIterationEvent`   |          ✔         |          ✔          |              |
-`BeforeRequestHandlingEvent` |          ✔         |                     |       ✔      |
-`AfterRequestHandlingEvent`  |          ✔         |                     |       ✔      |       ✔
-`AfterLoopIterationEvent`    |          ✔         |                     |       ✔      |       ✔
-`AfterLoopStoppedEvent`      |          ✔         |                     |              |
+Event classname              | Application object | HTTP server request | HTTP request | HTTP response | Exception
+---------------------------- | :----------------: | :-----------------: | :----------: | :-----------: | :-------:
+`BeforeLoopStartedEvent`     |          ✔         |                     |              |               |
+`BeforeLoopIterationEvent`   |          ✔         |          ✔          |              |               |
+`BeforeRequestHandlingEvent` |          ✔         |                     |       ✔      |               |
+`AfterRequestHandlingEvent`  |          ✔         |                     |       ✔      |       ✔       |
+`AfterLoopIterationEvent`    |          ✔         |                     |       ✔      |       ✔       |
+`AfterLoopStoppedEvent`      |          ✔         |                     |              |               |
+`LoopErrorOccurredEvent`     |          ✔         |          ✔          |              |               |     ✔
 
 Simple `.rr.yaml` config example:
 

config/roadrunner.php

@@ -93,6 +93,11 @@
         Events\AfterLoopStoppedEvent::class => [
             //
         ],
+
+        Events\LoopErrorOccurredEvent::class => [
+            Listeners\SendExceptionToStderrListener::class,
+            Listeners\StopWorkerListener::class,
+        ],
     ],
 
     /*

src/Events/Contracts/WithException.php

@@ -0,0 +1,15 @@
+<?php
+
+namespace AvtoDev\RoadRunnerLaravel\Events\Contracts;
+
+use Throwable;
+
+interface WithException
+{
+    /**
+     * Get exception instance.
+     *
+     * @return Throwable
+     */
+    public function exception(): Throwable;
+}

src/Events/LoopErrorOccurredEvent.php

@@ -0,0 +1,65 @@
+<?php
+
+declare(strict_types = 1);
+
+namespace AvtoDev\RoadRunnerLaravel\Events;
+
+use Throwable;
+use Psr\Http\Message\ServerRequestInterface;
+use Illuminate\Contracts\Foundation\Application as ApplicationContract;
+
+final class LoopErrorOccurredEvent implements Contracts\WithApplication, Contracts\WithException, Contracts\WithServerRequest
+{
+    /**
+     * @var ApplicationContract
+     */
+    private $app;
+
+    /**
+     * @var Throwable
+     */
+    private $exception;
+
+    /**
+     * @var ServerRequestInterface
+     */
+    private $server_request;
+
+    /**
+     * Create a new event instance.
+     *
+     * @param ApplicationContract    $app
+     * @param ServerRequestInterface $server_request
+     * @param Throwable              $exception
+     */
+    public function __construct(ApplicationContract $app, ServerRequestInterface $server_request, Throwable $exception)
+    {
+        $this->app            = $app;
+        $this->server_request = $server_request;
+        $this->exception      = $exception;
+    }
+
+    /**
+     * {@inheritdoc}
+     */
+    public function application(): ApplicationContract
+    {
+        return $this->app;
+    }
+
+    /**
+     * {@inheritdoc}
+     */
+    public function exception(): Throwable
+    {
+        return $this->exception;
+    }
+
+    /**
+     * {@inheritdoc}
+     */
+    public function serverRequest(): ServerRequestInterface
+    {
+        return $this->server_request;
+    }
+}

src/Listeners/SendExceptionToStderrListener.php

@@ -0,0 +1,20 @@
+<?php
+
+declare(strict_types = 1);
+
+namespace AvtoDev\RoadRunnerLaravel\Listeners;
+
+use AvtoDev\RoadRunnerLaravel\Events\Contracts\WithException;
+
+class SendExceptionToStderrListener implements ListenerInterface
+{
+    /**
+     * {@inheritdoc}
+     */
+    public function handle($event): void
+    {
+        if ($event instanceof WithException) {
+            \fwrite(\STDERR, (string) $event->exception());
+        }
+    }
+}

src/Listeners/StopWorkerListener.php

@@ -0,0 +1,29 @@
+<?php
+
+declare(strict_types = 1);
+
+namespace AvtoDev\RoadRunnerLaravel\Listeners;
+
+use Spiral\RoadRunner\PSR7Client;
+use AvtoDev\RoadRunnerLaravel\Events\Contracts\WithApplication;
+
+/**
+ * Common usage - stop worker on unhandled error occurring.
+ *
+ * @link https://roadrunner.dev/docs/php-restarting
+ */
+class StopWorkerListener implements ListenerInterface
+{
+    /**
+     * {@inheritdoc}
+     */
+    public function handle($event): void
+    {
+        if ($event instanceof WithApplication) {
+            /** @var PSR7Client $psr7_client */
+            $psr7_client = $event->application()->make(PSR7Client::class);
+
+            $psr7_client->getWorker()->stop();
+        }
+    }
+}

src/Worker.php

@@ -4,6 +4,7 @@
 
 namespace AvtoDev\RoadRunnerLaravel;
 
+use Throwable;
 use RuntimeException;
 use Illuminate\Http\Request;
 use InvalidArgumentException;
@@ -58,6 +59,8 @@ public function start(bool $refresh_app = false): void
         $this->fireEvent($app, new Events\BeforeLoopStartedEvent($app));
 
         while ($req = $psr7_client->acceptRequest()) {
+            $responded = false;
+
             if ($refresh_app === true) {
                 $sandbox = $this->createApplication($this->base_path);
                 $this->bootstrapApplication($sandbox, $psr7_client);
@@ -69,6 +72,8 @@ public function start(bool $refresh_app = false): void
 
             /** @var HttpKernelContract $http_kernel */
             $http_kernel = $sandbox->make(HttpKernelContract::class);
+            /** @var ConfigRepository $config */
+            $config = $sandbox->make(ConfigRepository::class);
 
             try {
                 $this->fireEvent($sandbox, new Events\BeforeLoopIterationEvent($sandbox, $req));
@@ -80,11 +85,16 @@ public function start(bool $refresh_app = false): void
 
                 $psr7_response = $psr7_factory->createResponse($response);
                 $psr7_client->respond($psr7_response);
+                $responded = true;
                 $http_kernel->terminate($request, $response);
 
                 $this->fireEvent($sandbox, new Events\AfterLoopIterationEvent($sandbox, $request, $response));
-            } catch (\Throwable $e) {
-                $psr7_client->getWorker()->error((string) $e);
+            } catch (Throwable $e) {
+                if ($responded !== true) {
+                    $psr7_client->getWorker()->error($this->exceptionToString($e, $this->isDebugModeEnabled($config)));
+                }
+
+                $this->fireEvent($sandbox, new Events\LoopErrorOccurredEvent($sandbox, $req, $e));
             } finally {
                 unset($http_kernel, $response, $request, $sandbox);
 
@@ -95,6 +105,29 @@ public function start(bool $refresh_app = false): void
         $this->fireEvent($app, new Events\AfterLoopStoppedEvent($app));
     }
 
+    /**
+     * @param Throwable $e
+     * @param bool      $is_debug
+     *
+     * @return string
+     */
+    protected function exceptionToString(Throwable $e, bool $is_debug): string
+    {
+        return $is_debug
+            ? (string) $e
+            : 'Internal server error';
+    }
+
+    /**
+     * @param ConfigRepository $config
+     *
+     * @return bool
+     */
+    protected function isDebugModeEnabled(ConfigRepository $config): bool
+    {
+        return $config->get('app.debug', false) === true;
+    }
+
     /**
      * @param ApplicationContract $app
      *

tests/Events/LoopErrorOccurredTest.php

@@ -0,0 +1,47 @@
+<?php
+
+declare(strict_types = 1);
+
+namespace AvtoDev\RoadRunnerLaravel\Tests\Events;
+
+use Zend\Diactoros\ServerRequest;
+use AvtoDev\RoadRunnerLaravel\Events\LoopErrorOccurredEvent;
+use AvtoDev\RoadRunnerLaravel\Events\Contracts\WithException;
+use AvtoDev\RoadRunnerLaravel\Events\Contracts\WithApplication;
+use AvtoDev\RoadRunnerLaravel\Events\Contracts\WithServerRequest;
+
+/**
+ * @covers \AvtoDev\RoadRunnerLaravel\Events\LoopErrorOccurredEvent<extended>
+ */
+class LoopErrorOccurredTest extends AbstractEventTestCase
+{
+    /**
+     * @var string[]
+     */
+    protected $required_interfaces = [
+        WithApplication::class,
+        WithException::class,
+        WithServerRequest::class,
+    ];
+
+    /**
+     * @var string
+     */
+    protected $event_class = LoopErrorOccurredEvent::class;
+
+    /**
+     * {@inheritdoc}
+     */
+    public function testConstructor(): void
+    {
+        $event = new LoopErrorOccurredEvent(
+            $this->app,
+            $request = new ServerRequest,
+            $exception = new \Exception('foo')
+        );
+
+        $this->assertSame($this->app, $event->application());
+        $this->assertSame($exception, $event->exception());
+        $this->assertSame($request, $event->serverRequest());
+    }
+}

tests/Listeners/SendExceptionToStderrListenerTest.php

@@ -0,0 +1,25 @@
+<?php
+
+declare(strict_types = 1);
+
+namespace AvtoDev\RoadRunnerLaravel\Tests\Listeners;
+
+use AvtoDev\RoadRunnerLaravel\Listeners\SendExceptionToStderrListener;
+
+/**
+ * @covers \AvtoDev\RoadRunnerLaravel\Listeners\SendExceptionToStderrListener<extended>
+ */
+class SendExceptionToStderrListenerTest extends AbstractListenerTestCase
+{
+    public function testHandle(): void
+    {
+        $this->listenerFactory()->handle(new \stdClass);
+
+        $this->markTestIncomplete('There is no legal way for handle method testing.');
+    }
+
+    protected function listenerFactory()
+    {
+        return new SendExceptionToStderrListener;
+    }
+}