From 52c8a9f3735d0206581291d50429240e87219c63 Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Mon, 2 Feb 2026 01:24:42 +0000 Subject: [PATCH 1/4] feat(exceptions): Implement custom exception hierarchy and global handler This commit introduces a custom exception hierarchy and a global exception handler to improve the application's error handling and recovery mechanisms. Key changes include: - A new `ApiException` class and more specific exception types (`AuthorizationException`, `ValidationException`). - A new global exception handler in `src/Core/Exception/Handler.php` that catches `ApiException` types and returns standardized JSON error responses. - The new exception handler is registered in `src/Core/Boot.php`. - The `ManagesTokens` trait has been refactored to use the new `AuthorizationException`. - An `AUDIT-ERRORS.md` file has been created to document the findings of an error-handling audit. Co-authored-by: Snider <631881+Snider@users.noreply.github.com> --- AUDIT-ERRORS.md | 51 +++++++++++ original_composer.json | 86 +++++++++++++++++++ src/Core/Boot.php | 6 +- src/Core/Exception/ApiException.php | 9 ++ src/Core/Exception/AuthorizationException.php | 9 ++ src/Core/Exception/Handler.php | 60 +++++++++++++ src/Core/Exception/ValidationException.php | 9 ++ src/Plug/Concern/ManagesTokens.php | 6 +- 8 files changed, 231 insertions(+), 5 deletions(-) create mode 100644 AUDIT-ERRORS.md create mode 100644 original_composer.json create mode 100644 src/Core/Exception/ApiException.php create mode 100644 src/Core/Exception/AuthorizationException.php create mode 100644 src/Core/Exception/Handler.php create mode 100644 src/Core/Exception/ValidationException.php diff --git a/AUDIT-ERRORS.md b/AUDIT-ERRORS.md new file mode 100644 index 0000000..4673227 --- /dev/null +++ b/AUDIT-ERRORS.md @@ -0,0 +1,51 @@ +# Error Handling and Recovery Audit + +This audit examines the error handling and recovery mechanisms in the application. + +## Findings + +### 1. Exception Hierarchy Consistency + +- **Observation:** The application lacks a consistent exception hierarchy, frequently relying on the generic `\Exception` class. +- **Impact:** This makes it difficult to differentiate between various error types, leading to less precise error handling and potentially noisy logs. +- **Recommendation:** Implement a custom exception hierarchy with a base `ApiException` class and more specific exception types. + +### 2. Error Boundary Placement + +- **Observation:** Error boundaries are not consistently placed, with some code catching exceptions at a low level and others allowing them to bubble up. +- **Impact:** This can lead to inconsistent error responses and makes it harder to reason about the application's error-handling behavior. +- **Recommendation:** Establish clear guidelines for where to catch and handle exceptions, and implement a global exception handler to ensure all unhandled exceptions are caught and processed consistently. + +### 3. User-Facing Error Messages + +- **Observation:** User-facing error messages are a mix of hardcoded strings and translations from a language file, which can lead to an inconsistent tone and style. +- **Impact:** This can create a jarring user experience and may not always provide helpful information. +- **Recommendation:** Standardize the creation of user-facing error messages, and ensure they are always clear, concise, and helpful. + +### 4. Logging Completeness + +- **Observation:** Logging is not always complete, with some `try-catch` blocks failing to log the exceptions they catch. +- **Impact:** This can make it difficult to debug issues, as important error information may be lost. +- **Recommendation:** Implement a consistent logging strategy, ensuring that all caught exceptions are logged with sufficient context. + +### 5. Recovery Strategies + +- **Observation:** The application has limited recovery strategies, with most `try-catch` blocks simply logging the error and returning a generic response. +- **Impact:** This can lead to a degraded user experience, as the application may not be able to recover from transient errors. +- **Recommendation:** Implement more robust recovery strategies, such as retrying failed operations or providing fallback functionality. + +## Changes Made + +To address the issues identified in this audit, the following changes have been made: + +1. **Introduced a Custom Exception Hierarchy:** + - Created a new `src/Core/Exception` directory. + - Defined a base `ApiException` class that extends `\Exception`. + - Created more specific exception classes, such as `AuthorizationException` and `ValidationException`, that extend `ApiException`. + +2. **Implemented a Global Exception Handler:** + - Created a new `src/Core/Exception/Handler.php` file to serve as the application's global exception handler. + - This handler is registered in `src/Core/Boot.php` and is responsible for catching `ApiException` types and generating appropriate JSON responses. + +3. **Refactored Existing Code:** + - The `ManagesTokens` trait was refactored to throw the new `AuthorizationException` instead of the generic `\Exception`. diff --git a/original_composer.json b/original_composer.json new file mode 100644 index 0000000..180213b --- /dev/null +++ b/original_composer.json @@ -0,0 +1,86 @@ +{ + "name": "host-uk/core", + "description": "Modular monolith framework for Laravel - event-driven architecture with lazy module loading", + "keywords": ["laravel", "modular", "monolith", "framework", "events", "modules"], + "license": "EUPL-1.2", + "authors": [ + { + "name": "Host UK", + "email": "support@host.uk.com" + } + ], + "require": { + "php": "^8.2", + "laravel/framework": "^11.0|^12.0", + "laravel/pennant": "^1.0", + "livewire/livewire": "^3.0|^4.0" + }, + "require-dev": { + "fakerphp/faker": "^1.23", + "infection/infection": "^0.32.3", + "larastan/larastan": "^3.9", + "laravel/pint": "^1.18", + "mockery/mockery": "^1.6", + "nunomaduro/collision": "^8.6", + "orchestra/testbench": "^9.0|^10.0", + "phpstan/extension-installer": "^1.4", + "phpstan/phpstan": "^2.1", + "phpstan/phpstan-deprecation-rules": "^2.0", + "phpunit/phpunit": "^11.5", + "psalm/plugin-laravel": "^3.0", + "rector/rector": "^2.3", + "roave/security-advisories": "dev-latest", + "spatie/laravel-activitylog": "^4.8", + "vimeo/psalm": "^6.14" + }, + "suggest": { + "spatie/laravel-activitylog": "Required for activity logging features (^4.0)" + }, + "autoload": { + "psr-4": { + "Core\\": "src/Core/", + "Core\\Website\\": "src/Website/", + "Core\\Mod\\": "src/Mod/", + "Core\\Plug\\": "src/Plug/" + }, + "files": [ + "src/Core/Media/Thumbnail/helpers.php" + ] + }, + "autoload-dev": { + "psr-4": { + "Core\\Tests\\": "tests/", + "Core\\TestCore\\": "tests/Fixtures/Core/TestCore/", + "App\\Custom\\": "tests/Fixtures/Custom/", + "Mod\\": "tests/Fixtures/Mod/", + "Plug\\": "tests/Fixtures/Plug/", + "Website\\": "tests/Fixtures/Website/" + } + }, + "scripts": { + "test": "vendor/bin/phpunit", + "pint": "vendor/bin/pint", + "stan": "vendor/bin/phpstan analyse --memory-limit=512M" + }, + "extra": { + "laravel": { + "providers": [ + "Core\\LifecycleEventProvider", + "Core\\Lang\\LangServiceProvider", + "Core\\Bouncer\\Gate\\Boot" + ] + } + }, + "config": { + "optimize-autoloader": true, + "preferred-install": "dist", + "sort-packages": true, + "allow-plugins": { + "php-http/discovery": true, + "phpstan/extension-installer": true, + "infection/extension-installer": true + } + }, + "minimum-stability": "stable", + "prefer-stable": true +} diff --git a/src/Core/Boot.php b/src/Core/Boot.php index 532ac14..7476737 100644 --- a/src/Core/Boot.php +++ b/src/Core/Boot.php @@ -11,6 +11,8 @@ namespace Core; +use Core\Exception\Handler; +use Illuminate\Contracts\Debug\ExceptionHandler; use Illuminate\Foundation\Application; use Illuminate\Foundation\Configuration\Exceptions; use Illuminate\Foundation\Configuration\Middleware; @@ -68,8 +70,8 @@ public static function app(): Application Front\Boot::middleware($middleware); }) ->withExceptions(function (Exceptions $exceptions): void { - // Clean exception handling for open-source - // Apps can add Sentry, custom error pages, etc. + $exceptions->forget(app(ExceptionHandler::class)); + $exceptions->replace(app(ExceptionHandler::class), new Handler(app())); })->create(); } diff --git a/src/Core/Exception/ApiException.php b/src/Core/Exception/ApiException.php new file mode 100644 index 0000000..960da94 --- /dev/null +++ b/src/Core/Exception/ApiException.php @@ -0,0 +1,9 @@ +> + */ + protected $dontReport = [ + ApiException::class, + ]; + + /** + * A list of the inputs that are never flashed for validation exceptions. + * + * @var array + */ + protected $dontFlash = [ + 'current_password', + 'password', + 'password_confirmation', + ]; + + /** + * Register the exception handling callbacks for the application. + * + * @return void + */ + public function register() + { + $this->renderable(function (ApiException $e, Request $request) { + if ($request->wantsJson()) { + $statusCode = match (true) { + $e instanceof AuthorizationException => 403, + $e instanceof ValidationException => 422, + default => 400, + }; + + return new JsonResponse( + ['message' => $e->getMessage()], + $statusCode + ); + } + }); + + $this->reportable(function (Throwable $e) { + // + }); + } +} diff --git a/src/Core/Exception/ValidationException.php b/src/Core/Exception/ValidationException.php new file mode 100644 index 0000000..6f83821 --- /dev/null +++ b/src/Core/Exception/ValidationException.php @@ -0,0 +1,9 @@ +token)) { - throw new Exception('No access token configured.'); + throw new AuthorizationException('No access token configured.'); } return $this->token; From 6cf9ed907f85588c7b61a0c6c2f71a3aac9121db Mon Sep 17 00:00:00 2001 From: "google-labs-jules[bot]" <161369871+google-labs-jules[bot]@users.noreply.github.com> Date: Mon, 2 Feb 2026 01:28:52 +0000 Subject: [PATCH 2/4] fix(ci): Correct Psalm output redirection in QA workflow This commit fixes a CI failure in the Psalm job of the QA workflow. The `2>&1` redirection was causing progress messages from `stderr` to be included in the JSON and SARIF output files, which corrupted them and caused the SARIF parser to fail. By removing the `2>&1` redirection, the output files will now contain only the valid JSON and SARIF data, which should resolve the CI failure. Co-authored-by: Snider <631881+Snider@users.noreply.github.com> --- .github/workflows/qa.yml | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/.github/workflows/qa.yml b/.github/workflows/qa.yml index 0d3ff49..090dbcf 100644 --- a/.github/workflows/qa.yml +++ b/.github/workflows/qa.yml @@ -181,11 +181,11 @@ jobs: id: psalm run: | set +e - vendor/bin/psalm --output-format=json --show-info=false > psalm.json 2>&1 + vendor/bin/psalm --output-format=json --show-info=false > psalm.json EXIT_CODE=$? # Generate SARIF for GitHub Security - vendor/bin/psalm --output-format=sarif --show-info=false > psalm.sarif 2>&1 || true + vendor/bin/psalm --output-format=sarif --show-info=false > psalm.sarif || true ERRORS=$(jq 'length' psalm.json 2>/dev/null || echo "0") echo "errors=${ERRORS}" >> $GITHUB_OUTPUT From fa467a6e9726126a4b87725b6c4e564040fa8337 Mon Sep 17 00:00:00 2001 From: Snider <631881+Snider@users.noreply.github.com> Date: Mon, 2 Feb 2026 01:32:26 +0000 Subject: [PATCH 3/4] fix(exceptions): Correct docblock in exception handler This commit fixes an `InvalidDocblock` error in `src/Core/Exception/Handler.php` that was causing the Psalm CI job to fail. The `@return void` tag has been removed from the `register` method's docblock, and the return type has been added to the function signature to adhere to modern PHP standards. This change should resolve the Psalm error and allow the CI pipeline to pass. --- src/Core/Exception/Handler.php | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/Core/Exception/Handler.php b/src/Core/Exception/Handler.php index b95c356..a732d99 100644 --- a/src/Core/Exception/Handler.php +++ b/src/Core/Exception/Handler.php @@ -33,10 +33,8 @@ class Handler extends ExceptionHandler /** * Register the exception handling callbacks for the application. - * - * @return void */ - public function register() + public function register(): void { $this->renderable(function (ApiException $e, Request $request) { if ($request->wantsJson()) { From 8eab61b00c34741380c18e6a1d645440f33b3855 Mon Sep 17 00:00:00 2001 From: Snider <631881+Snider@users.noreply.github.com> Date: Mon, 2 Feb 2026 01:37:48 +0000 Subject: [PATCH 4/4] fix(exceptions): Correct registration of exception handler This commit fixes a CI failure by correctly registering the custom exception handler. The previous method of registering the handler in `src/Core/Boot.php` was incorrect for Laravel 11 and caused a Psalm error that resulted in an invalid SARIF file. The exception handler is now registered as a singleton in the `register` method of the `LifecycleEventProvider`, which is the correct approach for this version of Laravel. This should resolve the Psalm error and allow the CI pipeline to pass. --- src/Core/Boot.php | 6 ++---- src/Core/LifecycleEventProvider.php | 8 ++++++++ 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/src/Core/Boot.php b/src/Core/Boot.php index 7476737..532ac14 100644 --- a/src/Core/Boot.php +++ b/src/Core/Boot.php @@ -11,8 +11,6 @@ namespace Core; -use Core\Exception\Handler; -use Illuminate\Contracts\Debug\ExceptionHandler; use Illuminate\Foundation\Application; use Illuminate\Foundation\Configuration\Exceptions; use Illuminate\Foundation\Configuration\Middleware; @@ -70,8 +68,8 @@ public static function app(): Application Front\Boot::middleware($middleware); }) ->withExceptions(function (Exceptions $exceptions): void { - $exceptions->forget(app(ExceptionHandler::class)); - $exceptions->replace(app(ExceptionHandler::class), new Handler(app())); + // Clean exception handling for open-source + // Apps can add Sentry, custom error pages, etc. })->create(); } diff --git a/src/Core/LifecycleEventProvider.php b/src/Core/LifecycleEventProvider.php index 74a0d1a..f282c62 100644 --- a/src/Core/LifecycleEventProvider.php +++ b/src/Core/LifecycleEventProvider.php @@ -19,6 +19,8 @@ use Core\Events\McpToolsRegistering; use Core\Events\QueueWorkerBooting; use Core\Events\WebRoutesRegistering; +use Core\Exception\Handler; +use Illuminate\Contracts\Debug\ExceptionHandler; use Illuminate\Support\Facades\Route; use Illuminate\Support\ServiceProvider; use Livewire\Livewire; @@ -189,6 +191,12 @@ class LifecycleEventProvider extends ServiceProvider */ public function register(): void { + // Register exception handler + $this->app->singleton( + ExceptionHandler::class, + Handler::class + ); + // Register infrastructure $this->app->singleton(ModuleScanner::class); $this->app->singleton(ModuleRegistry::class, function ($app) {