Add late output error test (#1719)

Author: mvorisek

demos/_unit-test/callback_2.php renamed to demos/_unit-test/callback-nested.php

@@ -10,6 +10,7 @@
 
 use Atk4\Ui\Button;
 use Atk4\Ui\Crud;
+use Atk4\Ui\Exception;
 use Atk4\Ui\Header;
 use Atk4\Ui\Loader;
 use Atk4\Ui\UserAction\ExecutorFactory;
@@ -25,26 +26,39 @@
 );
 
 $loader = Loader::addTo($app);
+$loader->cb->setUrlTrigger('trigger_main_loader');
 $loader->loadEvent = false;
 
-$loader->set(function ($p) use ($m) {
-    $loader_1 = Loader::addTo($p);
-    $loader_1->loadEvent = false;
-
+$loader->set(function (Loader $p) use ($m) {
     Header::addTo($p, ['Loader-1', 'size' => 4]);
 
-    $loader_1->set(function ($p) use ($m) {
+    if (isset($_GET['err_main_loader'])) {
+        throw new Exception('Exception from Main Loader');
+    }
+
+    $loaderSub = Loader::addTo($p);
+    $loaderSub->cb->setUrlTrigger('trigger_sub_loader');
+    $loaderSub->loadEvent = false;
+
+    $loaderSub->set(function (Loader $p) use ($m) {
         Header::addTo($p, ['Loader-2', 'size' => 4]);
-        $loader_3 = Loader::addTo($p);
 
-        $loader_3->set(function ($p) use ($m) {
+        if (isset($_GET['err_sub_loader'])) {
+            throw new Exception('Exception from Sub Loader');
+        } elseif (isset($_GET['err_sub_loader2'])) {
+            throw new \Error('Exception II from Sub Loader');
+        }
+
+        $loaderSubSub = Loader::addTo($p);
+
+        $loaderSubSub->set(function (Loader $p) use ($m) {
             Header::addTo($p, ['Loader-3', 'size' => 4]);
 
             $c = Crud::addTo($p, ['ipp' => 4]);
             $c->setModel($m, [$m->fieldName()->name]);
         });
     });
-    \Atk4\Ui\Button::addTo($p, ['Load2'])->js('click', $loader_1->jsLoad());
+    \Atk4\Ui\Button::addTo($p, ['Load2'])->js('click', $loaderSub->jsLoad());
 });
 
 \Atk4\Ui\Button::addTo($app, ['Load1'])->js('click', $loader->jsLoad());

demos/_unit-test/exception.php

@@ -18,7 +18,7 @@
 $modal->name = 'm_test';
 
 $modal->set(function () {
-    throw new \Exception('TEST!');
+    throw new \Exception('Test throw exception!');
 });
 
 $button = \Atk4\Ui\Button::addTo($app, ['Test modal exception']);
@@ -28,7 +28,7 @@
 $modal2 = \Atk4\Ui\Modal::addTo($app, ['cb' => $cb1]);
 
 $modal2->set(function () {
-    trigger_error('error triggered');
+    trigger_error('Test trigger error!');
 });
 
 $button2 = \Atk4\Ui\Button::addTo($app, ['Test modal error']);

demos/_unit-test/late-output-error.php

@@ -0,0 +1,37 @@
+<?php
+
+declare(strict_types=1);
+
+namespace Atk4\Ui\Demos;
+
+use Atk4\Ui\CallbackLater;
+
+/** @var \Atk4\Ui\App $app */
+require_once __DIR__ . '/../init-app.php';
+
+$cb = CallbackLater::addTo($app);
+$cb->setUrlTrigger('err_headers_already_sent');
+
+$modal = \Atk4\Ui\Modal::addTo($app, ['cb' => $cb]);
+$modal->set(function () {
+    header('x-unmanaged-header: test');
+    flush();
+});
+
+$cb2 = CallbackLater::addTo($app);
+$cb2->setUrlTrigger('err_unexpected_output_detected');
+
+$modal2 = \Atk4\Ui\Modal::addTo($app, ['cb' => $cb2]);
+$modal2->set(function () {
+    // unexpected output can be detected only when output buffering is enabled and not flushed
+    if (ob_get_level() === 0) {
+        ob_start();
+    }
+    echo 'unmanaged output';
+});
+
+$button = \Atk4\Ui\Button::addTo($app, ['Test LateOutputError: Headers already sent']);
+$button->on('click', $modal->show());
+
+$button2 = \Atk4\Ui\Button::addTo($app, ['Test LateOutputError: Unexpected output detected']);
+$button2->on('click', $modal2->show());

phpstan.neon.dist

@@ -700,9 +700,6 @@ parameters:
         -
             path: 'src/View.php'
             message: '~^If condition is always true\.$~'
-        -
-            path: 'tests/DemosTest.php'
-            message: '~^Else branch is unreachable because ternary operator condition is always true\.$~'
 
         # TODO these rules are generated, this ignores should be fixed in the code
         # for level = 5
@@ -2038,27 +2035,6 @@ parameters:
         -
             path: 'src/Wizard.php'
             message: '~^Method Atk4\\Ui\\Wizard::addFinish\(\) has no return type specified\.$~'
-        -
-            path: 'tests/CallbackTest.php'
-            message: '~^Property Atk4\\Ui\\Tests\\CallbackTest::\$var has no type specified\.$~'
-        -
-            path: 'tests/CallbackTest.php'
-            message: '~^Method Atk4\\Ui\\Tests\\CallbackTest::callPull230\(\) has no return type specified\.$~'
-        -
-            path: 'tests/DemosHttpTest.php'
-            message: '~^Property Atk4\\Ui\\Tests\\DemosHttpTest::\$_process has no type specified\.$~'
-        -
-            path: 'tests/DemosHttpTest.php'
-            message: '~^Property Atk4\\Ui\\Tests\\DemosHttpTest::\$_processSessionDir has no type specified\.$~'
-        -
-            path: 'tests/DemosHttpTest.php'
-            message: '~^Property Atk4\\Ui\\Tests\\DemosHttpTest::\$host has no type specified\.$~'
-        -
-            path: 'tests/DemosHttpTest.php'
-            message: '~^Property Atk4\\Ui\\Tests\\DemosHttpTest::\$port has no type specified\.$~'
-        -
-            path: 'tests/DemosTest.php'
-            message: '~^Strict comparison using \!\=\= between bool and null will always evaluate to true\.$~'
         -
             path: 'tests/PaginatorTest.php'
             message: '~^Property Atk4\\Ui\\Tests\\PaginatorTest::\$p has no type specified\.$~'

src/App.php

@@ -123,6 +123,7 @@ class App
 
     /** @var string[] Extra HTTP headers to send on exit. */
     protected $response_headers = [
+        self::HEADER_STATUS_CODE => '200',
         'cache-control' => 'no-store', // disable caching by default
     ];
 
@@ -216,6 +217,7 @@ static function (int $severity, string $msg, string $file, int $line): bool {
                 },
                 \E_ALL
             );
+            $this->outputResponseUnsafe('', [self::HEADER_STATUS_CODE => 500]);
         }
 
         // Always run app on shutdown

src/Loader.php

@@ -21,7 +21,7 @@ class Loader extends View
     public $shim;
 
     /**
-     * Specify which event will cause Loader to begen fetching it's actual data. In some cases
+     * Specify which event will cause Loader to begin fetching it's actual data. In some cases
      * you would want to wait. You can set a custom JavaScript event name then trigger() it.
      *
      * Default value is `true` which means loading will take place as soon as possible. Setting this
@@ -35,7 +35,7 @@ class Loader extends View
     public $ui = 'ui segment';
 
     /** @var Callback for triggering */
-    protected $cb;
+    public $cb;
 
     /** @var array Url arguments. */
     public $urlArgs = [];

tests-behat/callback.feature

@@ -10,7 +10,7 @@ Feature: Callback
     Then I should not see "TestName"
 
   Scenario:
-    Given I am on "_unit-test/callback_2.php"
+    Given I am on "_unit-test/callback-nested.php"
     Then I press button "Load1"
     Then I should see "Loader-1"
     Then I press button "Load2"

tests/CallbackTest.php

@@ -225,23 +225,21 @@ public function testVirtualPageCustomTrigger(): void
         $this->assertSame(25, $var);
     }
 
-    public $var;
-
-    public function callPull230()
-    {
-        $this->var = 26;
-    }
+    /** @var int */
+    private $varPull230;
 
     public function testPull230(): void
     {
         $vp = VirtualPage::addTo($this->app);
 
         $this->simulateCallbackTriggering($vp);
 
-        $vp->set(\Closure::fromCallable([$this, 'callPull230']));
+        $vp->set(function () {
+            $this->varPull230 = 26;
+        });
 
         $this->expectOutputRegex('/^..DOCTYPE/');
         $this->app->run();
-        $this->assertSame(26, $this->var);
+        $this->assertSame(26, $this->varPull230);
     }
 }

tests/DemosHttpTest.php

@@ -4,6 +4,7 @@
 
 namespace Atk4\Ui\Tests;
 
+use Atk4\Ui\Callback;
 use GuzzleHttp\Client;
 use Symfony\Component\Process\Process;
 
@@ -14,7 +15,9 @@
  */
 class DemosHttpTest extends DemosTest
 {
+    /** @var Process|null */
     private static $_process;
+    /** @var string|null */
     private static $_processSessionDir;
 
     /** @var bool set the app->call_exit in demo */
@@ -23,7 +26,9 @@ class DemosHttpTest extends DemosTest
     /** @var bool set the app->catch_exceptions in demo */
     protected $app_catch_exceptions = true;
 
+    /** @var string */
     protected $host = '127.0.0.1';
+    /** @var int */
     protected $port = 9687;
 
     public static function tearDownAfterClass(): void
@@ -90,4 +95,33 @@ protected function getPathWithAppVars(string $path): string
 
         return parent::getPathWithAppVars($path);
     }
+
+    private function getLateOutputErrorPath(string $trigger): string
+    {
+        return '_unit-test/late-output-error.php?' . Callback::URL_QUERY_TRIGGER_PREFIX . $trigger . '=ajax&'
+            . Callback::URL_QUERY_TARGET . '=' . $trigger . '&__atk_json=1';
+    }
+
+    public function testDemoLateOutputErrorHeadersAlreadySent(): void
+    {
+        $response = $this->getResponseFromRequest5xx($this->getLateOutputErrorPath('err_headers_already_sent'));
+
+        $this->assertSame(500, $response->getStatusCode());
+        $this->assertSame(
+            "\n" . '!! FATAL UI ERROR: Headers already sent, more headers cannot be set at this stage !!' . "\n",
+            $response->getBody()->getContents()
+        );
+    }
+
+    public function testDemoLateOutputErrorUnexpectedOutputDetected(): void
+    {
+        $response = $this->getResponseFromRequest5xx($this->getLateOutputErrorPath('err_unexpected_output_detected'));
+
+        $this->assertSame(500, $response->getStatusCode());
+        $this->assertSame(
+            'unmanaged output'
+                . "\n" . '!! FATAL UI ERROR: Unexpected output detected !!' . "\n",
+            $response->getBody()->getContents()
+        );
+    }
 }