Rethink the way the error handler works

UPGRADE.md

@@ -52,7 +52,7 @@ If you are relying on Quill.js specifics (like css classes), use `'type' => 'qui
 Previously `withVideo` was true by default, if you relied on this you have to update these media fields to
 `'withVideo' => true`.
 
-#### SVG's are now no longer passing thorough glide
+#### SVG's are now no longer passing through glide
 
 These are now rendered directly, you can change this by updating config `twill.glide.original_media_for_extensions` to an empty array `[]`
 

config/twill.php

@@ -25,7 +25,7 @@
 
     /*
     |--------------------------------------------------------------------------
-    | Application strict url handeling
+    | Application strict url handling
     |--------------------------------------------------------------------------
     |
     | Setting this value to true will enable strict domain handling.

docs/content/1_docs/2_getting-started/3_configuration.md

@@ -79,31 +79,6 @@ return [
 ];
 ```
 
-Twill registers its own exception handler in all controllers. If you need to customize it (to report errors on a 3rd party service like Sentry or Rollbar for example), you can opt-out from it in your `config/twill.php` file:
-
-```php
-<?php
-
-return [
-    'bind_exception_handler' => false,
-];
-```
-
-And then extend it from your own `app/Exceptions/Handler.php` class:
-
-```php
-<?php
-
-namespace App\Exceptions;
-
-use A17\Twill\Exceptions\Handler as ExceptionHandler;
-use Exception;
-use Illuminate\Auth\AuthenticationException;
-
-class Handler extends ExceptionHandler
-...
-```
-
 If you would like to provide custom tables names, use the following configuration options:
 
 ```php

src/Exceptions/Handler.php

@@ -2,51 +2,16 @@
 
 namespace A17\Twill\Exceptions;
 
+use Illuminate\Contracts\Container\Container;
 use Illuminate\Foundation\Exceptions\Handler as ExceptionHandler;
-use Illuminate\Support\Facades\Request;
-use Illuminate\Support\Str;
-use Illuminate\Validation\ValidationException;
-use Symfony\Component\HttpKernel\Exception\HttpExceptionInterface;
 
+/** @deprecated It is not needed anymore and will be removed in v4 */
 class Handler extends ExceptionHandler
 {
-    /**
-     * Get the view used to render HTTP exceptions.
-     *
-     * @return string
-     */
-    protected function getHttpExceptionView(HttpExceptionInterface $e)
+    public function __construct(Container $container)
     {
-        $usesAdminPath = !empty(config('twill.admin_app_path'));
-        $adminAppUrl = config('twill.admin_app_url', config('app.url'));
+        parent::__construct($container);
 
-        $isSubdomainAdmin = !$usesAdminPath && Str::contains(Request::url(), $adminAppUrl);
-        $isSubdirectoryAdmin = $usesAdminPath && Str::startsWith(Request::path(), config('twill.admin_app_path'));
-
-        return $this->getTwillErrorView($e->getStatusCode(), !$isSubdomainAdmin && !$isSubdirectoryAdmin);
-    }
-
-    /**
-     * Get the Twill error view used to render a specified HTTP status code.
-     *
-     * @param  integer $statusCode
-     * @return string
-     */
-    protected function getTwillErrorView($statusCode, $frontend = false)
-    {
-        if ($frontend) {
-            $view = config('twill.frontend.views_path') . ".errors.$statusCode";
-
-            return view()->exists($view) ? $view : "errors::{$statusCode}";
-        }
-
-        $view = "twill.errors.$statusCode";
-
-        return view()->exists($view) ? $view : "twill::errors.$statusCode";
-    }
-
-    protected function invalidJson($request, ValidationException $exception)
-    {
-        return response()->json($exception->errors(), $exception->status);
+        trigger_deprecation('area17/twill', '3.4', 'The Twill Exception handler is deprecated and will be removed in v4, go back to extending the laravel ExceptionHandler');
     }
 }

src/Http/Controllers/Admin/Controller.php

@@ -20,9 +20,6 @@ class Controller extends BaseController
 
     public function __construct()
     {
-        if (Config::get('twill.bind_exception_handler', true)) {
-            App::singleton(ExceptionHandler::class, TwillHandler::class);
-        }
     }
 
     /**

src/Http/Controllers/Front/Controller.php

@@ -19,10 +19,6 @@ class Controller extends BaseController
 
     public function __construct()
     {
-        if (Config::get('twill.bind_exception_handler', true)) {
-            App::singleton(ExceptionHandler::class, TwillHandler::class);
-        }
-
         $this->seo = new Seo();
 
         $this->seo->title = Config::get('twill.seo.site_title');

src/TwillRoutes.php

@@ -9,6 +9,8 @@
 use Illuminate\Support\Facades\Route;
 use Illuminate\Support\Str;
 
+use function request;
+
 class TwillRoutes
 {
     /**
@@ -402,4 +404,18 @@ public function getAuthRedirectPath(): string
         return config('twill.auth_login_redirect_path')
             ?? rtrim(config('twill.admin_app_url') ?: '', '/') . '/' . ltrim(config('twill.admin_app_path') ?? 'admin', '/');
     }
+
+    public function isTwillRequest(): bool
+    {
+        $adminAppUrl = str_replace(['http://', 'https://'], '', config('twill.admin_app_url', config('app.url')));
+        $requestHost = request()->getHttpHost();
+
+        $matchesDomain = config('twill.support_subdomain_admin_routing') ?
+                Str::startsWith($requestHost, config('twill.admin_app_subdomain', 'admin') . '.') && Str::endsWith($requestHost, '.' . $adminAppUrl)
+                : !config('twill.admin_app_strict') || $requestHost === $adminAppUrl;
+
+        $matchesPath = empty(config('twill.admin_app_path')) || request()->is(config('twill.admin_app_path', '') . '*');
+
+        return $matchesDomain && $matchesPath;
+    }
 }

src/TwillServiceProvider.php

@@ -28,6 +28,7 @@
 use A17\Twill\Commands\Update;
 use A17\Twill\Commands\UpdateExampleCommand;
 use A17\Twill\Commands\UpdateMorphMapReferences;
+use A17\Twill\Facades\TwillRoutes;
 use A17\Twill\Http\ViewComposers\CurrentUser;
 use A17\Twill\Http\ViewComposers\FilesUploaderConfig;
 use A17\Twill\Http\ViewComposers\Localization;
@@ -42,14 +43,20 @@
 use Astrotomic\Translatable\TranslatableServiceProvider;
 use Cartalyst\Tags\TagsServiceProvider;
 use Exception;
+use Illuminate\Contracts\Debug\ExceptionHandler;
+use Illuminate\Contracts\Foundation\CachesConfiguration;
 use Illuminate\Database\Eloquent\Relations\Relation;
 use Illuminate\Foundation\AliasLoader;
+use Illuminate\Foundation\Exceptions\Handler;
 use Illuminate\Support\Facades\Blade;
 use Illuminate\Support\Facades\View;
 use Illuminate\Support\ServiceProvider;
 use Illuminate\Support\Str;
+use Illuminate\Support\ViewErrorBag;
+use Illuminate\Validation\ValidationException;
 use PragmaRX\Google2FAQRCode\Google2FA as Google2FAQRCode;
 use Spatie\Activitylog\ActivitylogServiceProvider;
+use Symfony\Component\HttpKernel\Exception\HttpExceptionInterface;
 
 class TwillServiceProvider extends ServiceProvider
 {
@@ -79,8 +86,6 @@ class TwillServiceProvider extends ServiceProvider
      */
     public function boot(): void
     {
-        $this->requireHelpers();
-
         $this->publishConfigs();
         $this->publishMigrations();
         $this->publishAssets();
@@ -116,8 +121,13 @@ private function requireHelpers(): void
      */
     public function register(): void
     {
-        $this->mergeConfigs();
+        $this->requireHelpers();
 
+        if (! ($this->app instanceof CachesConfiguration && $this->app->configurationIsCached())) {
+            $this->mergeConfigs();
+        }
+
+        $this->registerErrorHandlers();
         $this->registerProviders();
         $this->registerAliases();
         $this->registerFacades();
@@ -150,8 +160,6 @@ public function register(): void
             'blocks' => config('twill.models.block', Block::class),
             'groups' => config('twill.models.group', Group::class),
         ]);
-
-        config(['twill.version' => $this->version()]);
     }
 
     private function registerFacades(): void
@@ -214,47 +222,6 @@ private function registerAliases(): void
      */
     private function publishConfigs(): void
     {
-        if (config('twill.enabled.users-management')) {
-            config([
-                'auth.providers.twill_users' => [
-                    'driver' => 'eloquent',
-                    'model' => twillModel('user'),
-                ],
-            ]);
-
-            config([
-                'auth.guards.twill_users' => [
-                    'driver' => 'session',
-                    'provider' => 'twill_users',
-                ],
-            ]);
-
-            if (blank(config('auth.passwords.twill_users'))) {
-                config([
-                    'auth.passwords.twill_users' => [
-                        'provider' => 'twill_users',
-                        'table' => config('twill.password_resets_table', 'twill_password_resets'),
-                        'expire' => 60,
-                        'throttle' => 60,
-                    ],
-                ]);
-            }
-        }
-
-        config(
-            ['activitylog.enabled' => config('twill.enabled.dashboard') ? true : config('twill.enabled.activitylog')]
-        );
-        config(['activitylog.subject_returns_soft_deleted_models' => true]);
-
-        config(
-            [
-                'analytics.service_account_credentials_json' => config(
-                    'twill.dashboard.analytics.service_account_credentials_json',
-                    storage_path('app/analytics/service-account-credentials.json')
-                ),
-            ]
-        );
-
         $this->publishes([__DIR__ . '/../config/twill-publish.php' => config_path('twill.php')], 'config');
         $this->publishes([__DIR__ . '/../config/translatable.php' => config_path('translatable.php')], 'config');
     }
@@ -299,6 +266,49 @@ private function mergeConfigs(): void
         }
 
         $this->mergeConfigFrom(__DIR__ . '/../config/services.php', 'services');
+
+        if (config('twill.enabled.users-management')) {
+            config([
+                'auth.providers.twill_users' => [
+                    'driver' => 'eloquent',
+                    'model' => twillModel('user'),
+                ],
+            ]);
+
+            config([
+                'auth.guards.twill_users' => [
+                    'driver' => 'session',
+                    'provider' => 'twill_users',
+                ],
+            ]);
+
+            if (blank(config('auth.passwords.twill_users'))) {
+                config([
+                    'auth.passwords.twill_users' => [
+                        'provider' => 'twill_users',
+                        'table' => config('twill.password_resets_table', 'twill_password_resets'),
+                        'expire' => 60,
+                        'throttle' => 60,
+                    ],
+                ]);
+            }
+        }
+
+        config(
+            ['activitylog.enabled' => config('twill.enabled.dashboard') ? true : config('twill.enabled.activitylog')]
+        );
+        config(['activitylog.subject_returns_soft_deleted_models' => true]);
+
+        config(
+            [
+                'analytics.service_account_credentials_json' => config(
+                    'twill.dashboard.analytics.service_account_credentials_json',
+                    storage_path('app/analytics/service-account-credentials.json')
+                ),
+            ]
+        );
+
+        config(['twill.version' => $this->version()]);
     }
 
     private function setLocalDiskUrl($type): void
@@ -625,4 +635,34 @@ public function check2FA(): void
             );
         }
     }
+
+    private function registerErrorHandlers(): void
+    {
+        $handler = app(ExceptionHandler::class);
+        if ($handler instanceof Handler) {
+            $handler->renderable(function (HttpExceptionInterface $e) {
+                $statusCode = $e->getStatusCode();
+                if (TwillRoutes::isTwillRequest()) {
+                    $view = "twill.errors.$statusCode";
+
+                    $view = view()->exists($view) ? $view : "twill::errors.$statusCode";
+                } else {
+                    $view = config('twill.frontend.views_path') . ".errors.$statusCode";
+
+                    $view = view()->exists($view) ? $view : null;
+                }
+                return $view ? response()->view($view, [
+                    'errors' => new ViewErrorBag(),
+                    'exception' => $e,
+                ], $e->getStatusCode(), $e->getHeaders()) : null;
+            });
+
+            $handler->renderable(function (ValidationException $exception) {
+                if (TwillRoutes::isTwillRequest() && request()->expectsJson()) {
+                    return response()->json($exception->errors(), $exception->status);
+                }
+                return null;
+            });
+        }
+    }
 }

tests/integration/Exceptions/ExceptionHandlerTest.php

@@ -0,0 +1,39 @@
+<?php
+
+namespace A17\Twill\Tests\Integration\Exceptions;
+
+use A17\Twill\Tests\Integration\TestCase;
+use Illuminate\Http\Request;
+use Illuminate\Support\Facades\Route;
+use Illuminate\View\View;
+
+class ExceptionHandlerTest extends TestCase
+{
+    public function test404InAdmin()
+    {
+        $res = $this->get('/twill/foobar');
+        $res->assertStatus(404);
+        $this->assertEquals($res->original->name(), "twill::errors.404");
+    }
+
+    public function test404InFrontend()
+    {
+        $res = $this->get('/foobar');
+        $res->assertStatus(404);
+        $this->assertTrue(is_string($res->original));
+    }
+
+    public function testValidationException()
+    {
+        Route::post('/twill/validation-exception', function (Request $request) {
+            $request->validate([
+                'dummy' => 'required',
+            ]);
+        });
+
+        $res = $this->postJson('/twill/validation-exception');
+        $res->assertStatus(422);
+        // Response is directly an array of exceptions and doesn't have a response key
+        $res->assertJsonValidationErrors('dummy', null);
+    }
+}