From 970716f5e59aa5d68abb99bce5d18da8a5ff695a Mon Sep 17 00:00:00 2001 From: Sourov Biswas Date: Sun, 20 Sep 2026 11:40:05 +0600 Subject: [PATCH] Show the app's own error pages, and drop a guard that guarded nothing Two bits of drift from ProductSync, which had both already. A 403, 404 or 500 dropped the user out of the app onto Laravel's plain stock page -- the sidebar, the theme and every way back gone, in an app they were signed into a moment earlier. They render inside the app now: 403 and 404 always, 500 and 503 only where debug mode is off, so the debug screen still wins locally. A 419 is not worth a page at all, so an expired session flashes a toast and sends the user back to the form they were already looking at. If the app cannot render -- database down, assets not built -- the handler falls back to the stock page rather than failing twice, which is what it did while I was writing the tests for it. An ErrorBoundary goes around the app for the other half of the problem: a render error unmounts the React tree and leaves a white screen, which says even less than the stock page does. The other bit is a `verified` middleware on every workspace route that did nothing. EnsureEmailIsVerified only bites when the user implements MustVerifyEmail, and this User never did, so it passed everyone through while reading as though it protected them. Implementing the contract is not the fix: there is no verification flow here to send anyone to, and 3AG Accounts now refuses to issue a code for an unverified address, with the SSO callback refusing an explicit email_verified of false behind it. Also point .env.example at accounts.test, which it has needed since the move to Herd left it naming an artisan serve port nothing listens on. Co-Authored-By: Claude Opus 5 --- .env.example | 2 +- bootstrap/app.php | 41 ++++++++++ resources/js/app.tsx | 12 ++- resources/js/components/error-boundary.tsx | 72 ++++++++++++++++++ resources/js/pages/error-page.tsx | 88 ++++++++++++++++++++++ routes/settings.php | 2 +- routes/web.php | 6 +- tests/Feature/ErrorPageTest.php | 66 ++++++++++++++++ 8 files changed, 280 insertions(+), 9 deletions(-) create mode 100644 resources/js/components/error-boundary.tsx create mode 100644 resources/js/pages/error-page.tsx create mode 100644 tests/Feature/ErrorPageTest.php diff --git a/.env.example b/.env.example index a0cc62c..8139c0e 100644 --- a/.env.example +++ b/.env.example @@ -5,7 +5,7 @@ APP_DEBUG=true APP_URL=http://localhost:8000 # 3AG Accounts OIDC (identity provider) -ACCOUNTS_URL=http://127.0.0.1:8000 +ACCOUNTS_URL=https://accounts.test ACCOUNTS_CLIENT_ID= ACCOUNTS_CLIENT_SECRET= ACCOUNTS_REDIRECT_URI="${APP_URL}/auth/accounts/callback" diff --git a/bootstrap/app.php b/bootstrap/app.php index da912a0..1f7b646 100644 --- a/bootstrap/app.php +++ b/bootstrap/app.php @@ -6,8 +6,11 @@ use Illuminate\Foundation\Application; use Illuminate\Foundation\Configuration\Exceptions; use Illuminate\Foundation\Configuration\Middleware; +use Illuminate\Http\JsonResponse; use Illuminate\Http\Middleware\AddLinkHeadersForPreloadedAssets; use Illuminate\Http\Request; +use Inertia\Inertia; +use Symfony\Component\HttpFoundation\Response; return Application::configure(basePath: dirname(__DIR__)) ->withRouting( @@ -29,4 +32,42 @@ $exceptions->shouldRenderJsonWhen( fn (Request $request) => $request->is('api/*') || $request->expectsJson(), ); + + $exceptions->respond(function (Response $response, Throwable $exception, Request $request) { + $status = $response->getStatusCode(); + + // JSON callers keep their own error handling. + if ($response instanceof JsonResponse) { + return $response; + } + + // An expired session is not worth an error page: send the user back + // to the form so reloading and resubmitting is the obvious next step. + if ($status === 419) { + Inertia::flash('toast', ['type' => 'error', 'message' => __('Your session expired. Please try again.')]); + + return back(); + } + + // 403 and 404 carry no debug detail, so they always get the app's page. + // Server errors keep the debug screen wherever debug mode is on. + $showsErrorPage = in_array($status, [403, 404], true) + || (in_array($status, [500, 503], true) && ! config('app.debug')); + + if (! $showsErrorPage) { + return $response; + } + + try { + return Inertia::render('error-page', ['status' => $status]) + ->toResponse($request) + ->setStatusCode($status); + } catch (Throwable $renderException) { + // If the app cannot render (database down, missing build), fall + // back to Laravel's plain page rather than failing twice. + report($renderException); + + return $response; + } + }); })->create(); diff --git a/resources/js/app.tsx b/resources/js/app.tsx index e9f7685..ac1d74c 100644 --- a/resources/js/app.tsx +++ b/resources/js/app.tsx @@ -1,4 +1,5 @@ import { createInertiaApp } from '@inertiajs/react'; +import ErrorBoundary from '@/components/error-boundary'; import { Toaster } from '@/components/ui/sonner'; import { TooltipProvider } from '@/components/ui/tooltip'; import { initializeTheme } from '@/hooks/use-appearance'; @@ -13,6 +14,7 @@ void createInertiaApp({ layout: (name) => { switch (true) { case name === 'welcome': + case name === 'error-page': return null; case name.startsWith('auth/'): return AuthLayout; @@ -26,10 +28,12 @@ void createInertiaApp({ strictMode: true, withApp(app) { return ( - - {app} - - + + + {app} + + + ); }, progress: { diff --git a/resources/js/components/error-boundary.tsx b/resources/js/components/error-boundary.tsx new file mode 100644 index 0000000..4338bbb --- /dev/null +++ b/resources/js/components/error-boundary.tsx @@ -0,0 +1,72 @@ +import { RefreshCw, TriangleAlert } from 'lucide-react'; +import { Component, type ErrorInfo, type ReactNode } from 'react'; +import { Button } from '@/components/ui/button'; + +type Props = { children: ReactNode }; +type State = { error: Error | null }; + +/** + * Catch render errors instead of letting them blank the page. + * + * React unmounts the whole tree when a render throws, which leaves a white + * screen and no clue what happened. Anything that gets this far is a bug, + * so the point is to say so plainly and offer a way out. + */ +export default class ErrorBoundary extends Component { + state: State = { error: null }; + + static getDerivedStateFromError(error: Error): State { + return { error }; + } + + componentDidCatch(error: Error, info: ErrorInfo) { + // Keep the stack in the console for whoever is looking at it. + console.error('Unhandled render error', error, info.componentStack); + } + + render() { + const { error } = this.state; + + if (!error) { + return this.props.children; + } + + return ( +
+
+ +
+ +
+

+ Something went wrong on this page +

+

+ The page could not finish loading. Reloading usually + clears it; if it keeps happening, the details below are + worth reporting. +

+
+ +
+ + +
+ + 
+                    {error.message}
+
+
+ ); + } +} diff --git a/resources/js/pages/error-page.tsx b/resources/js/pages/error-page.tsx new file mode 100644 index 0000000..a507696 --- /dev/null +++ b/resources/js/pages/error-page.tsx @@ -0,0 +1,88 @@ +import { Head, Link } from '@inertiajs/react'; +import { + ArrowLeft, + Ban, + FileQuestion, + ServerCrash, + Wrench, +} from 'lucide-react'; +import AppLogoIcon from '@/components/app-logo-icon'; +import { Button } from '@/components/ui/button'; +import { home } from '@/routes'; + +type Status = 403 | 404 | 500 | 503; + +const messages: Record< + Status, + { icon: typeof Ban; title: string; description: string } +> = { + 403: { + icon: Ban, + title: 'You don’t have access to this page', + description: + 'It may belong to an organization you aren’t a member of, or your role doesn’t allow it. Ask an organization owner if you think you should have access.', + }, + 404: { + icon: FileQuestion, + title: 'We couldn’t find that page', + description: + 'The link may be old, or the item was moved or deleted. Check the address, or head back and pick up from there.', + }, + 500: { + icon: ServerCrash, + title: 'Something went wrong on our side', + description: + 'The error has been logged. Try again in a moment; if it keeps happening, let us know what you were doing.', + }, + 503: { + icon: Wrench, + title: 'We’ll be right back', + description: + 'SalesReport is being updated. This usually takes a minute or two, so try again shortly.', + }, +}; + +export default function ErrorPage({ status }: { status: Status }) { + const { + icon: Icon, + title, + description, + } = messages[status] ?? messages[500]; + + return ( + <> + + +
+
+
+ +
+ +

+ Error {status} +

+ +

+ {title} +

+

+ {description} +

+ +
+ + +
+
+
+ + ); +} diff --git a/routes/settings.php b/routes/settings.php index ca4c27a..f381423 100644 --- a/routes/settings.php +++ b/routes/settings.php @@ -13,7 +13,7 @@ Route::get('settings/profile', [ProfileController::class, 'edit'])->name('profile.edit'); }); -Route::middleware(['auth', 'verified'])->group(function () { +Route::middleware(['auth'])->group(function () { Route::delete('settings/profile', [ProfileController::class, 'destroy'])->name('profile.destroy'); Route::inertia('settings/appearance', 'settings/appearance')->name('appearance.edit'); diff --git a/routes/web.php b/routes/web.php index 064a348..34168fd 100644 --- a/routes/web.php +++ b/routes/web.php @@ -32,11 +32,11 @@ ->name('logout'); Route::get('onboarding', OnboardingController::class) - ->middleware(['auth', 'verified']) + ->middleware(['auth']) ->name('onboarding'); Route::prefix('{current_organization}') - ->middleware(['auth', 'verified', EnsureOrganizationMembership::class]) + ->middleware(['auth', EnsureOrganizationMembership::class]) ->scopeBindings() ->group(function () { Route::get('dashboard', DashboardController::class)->name('dashboard'); @@ -62,7 +62,7 @@ }); Route::get('invitations', [OrganizationInvitationController::class, 'index']) - ->middleware(['auth', 'verified']) + ->middleware(['auth']) ->name('invitations.index'); Route::middleware(['auth'])->group(function () { diff --git a/tests/Feature/ErrorPageTest.php b/tests/Feature/ErrorPageTest.php new file mode 100644 index 0000000..b407231 --- /dev/null +++ b/tests/Feature/ErrorPageTest.php @@ -0,0 +1,66 @@ +get('/this-page-does-not-exist') + ->assertNotFound() + ->assertInertia(fn (Assert $page) => $page + ->component('error-page') + ->where('status', 404), + ); +}); + +test('an organization the user does not belong to renders the forbidden page', function () { + $user = User::factory()->create(); + $other = Organization::factory()->create(); + + $this->actingAs($user) + ->get(route('dashboard', ['current_organization' => $other->slug])) + ->assertForbidden() + ->assertInertia(fn (Assert $page) => $page + ->component('error-page') + ->where('status', 403), + ); +}); + +test('server errors render the error page when debug mode is off', function () { + config(['app.debug' => false]); + + Route::get('/boom', fn () => throw new RuntimeException('Boom'))->middleware('web'); + + $this->get('/boom') + ->assertInternalServerError() + ->assertInertia(fn (Assert $page) => $page + ->component('error-page') + ->where('status', 500), + ); +}); + +test('server errors keep the debug screen when debug mode is on', function () { + config(['app.debug' => true]); + + Route::get('/boom', fn () => throw new RuntimeException('Boom'))->middleware('web'); + + $this->get('/boom') + ->assertInternalServerError() + ->assertDontSee('"component":"error-page"', false); +}); + +test('json requests still get json errors', function () { + $this->getJson('/this-page-does-not-exist') + ->assertNotFound() + ->assertJsonStructure(['message']); +}); + +test('an expired session sends the user back with a message', function () { + Route::post('/expired', fn () => abort(419))->middleware('web'); + + $this->from('/settings/profile') + ->post('/expired') + ->assertRedirect('/settings/profile') + ->assertSessionHas('inertia.flash_data.toast.type', 'error'); +});