--- id: 23 title: 23 Fix AdminNotificationService Carbon TypeError Masking Exceptions status: Done date: 2026-09-28 parent: null --- ## Description Investigating a client report of a missing workshop-registration email led to discovering a real bug: `App\Services\AdminNotificationService::notifyError()` throws its own `TypeError` whenever it runs, which **masks the original exception** it was trying to report and prevents the admin notification from ever being sent. Root cause: `config('libra.notifications.throttle_minutes', 15)` resolves to `env('ERROR_NOTIFICATION_THROTTLE_MINUTES', 15)` (see `config/libra.php` line 14). Production's `.env` has `ERROR_NOTIFICATION_THROTTLE_MINUTES=15` set — Laravel's `env()` helper does not cast a numeric-looking string to an int, so this returns the **string** `"15"`, not the int `15`. That string is then passed to `now()->addMinutes($throttleMinutes)` in `AdminNotificationService::notifyError()` (line 32), and Carbon 3.11 (this project's installed version, per `composer.lock`) requires `addMinutes()`'s argument to be `int|float` — passing a string throws `TypeError: Carbon\Carbon::rawAddUnit(): Argument #3 ($value) must be of type int|float, string given`. This is a live production bug, confirmed via `storage/logs/laravel.log` on h1 (`/home/libra/public_html/storage/logs/laravel.log`, entries timestamped `2026-09-27 21:35:26` and `21:40:31`) — every exception that reaches `Handler->report()` and is judged notify-worthy by `AdminNotificationService::shouldNotifyAdmin()` (e.g. any `Symfony\Component\Mailer\Exception\TransportExceptionInterface`, or a critical `QueryException`) currently crashes this method instead of sending the notification, and the crash happens **before** `Cache::put()` on line 32 ever executes, so the throttle-dedup cache key is never written — meaning this crash also can't self-heal or throttle, it fires on every single qualifying exception, indefinitely, until fixed. **File to modify:** `app/Services/AdminNotificationService.php` Current (lines 25-32): ```php $errorKey = $this->getErrorKey($exception); $throttleMinutes = config('libra.notifications.throttle_minutes', 15); if (Cache::has($errorKey)) { return; } Cache::put($errorKey, true, now()->addMinutes($throttleMinutes)); ``` Change to cast the config value to an int before use: ```php $errorKey = $this->getErrorKey($exception); $throttleMinutes = (int) config('libra.notifications.throttle_minutes', 15); if (Cache::has($errorKey)) { return; } Cache::put($errorKey, true, now()->addMinutes($throttleMinutes)); ``` This is the only line that changes. No other method, no other file. ## Acceptance Criteria - [x] `app/Services/AdminNotificationService.php`'s `$throttleMinutes` assignment casts the config value with `(int)` - [x] A test constructs `AdminNotificationService` and calls `notifyError()` with a throwable that makes `shouldNotifyAdmin()` return true (e.g. a mock/stub implementing `Symfony\Component\Mailer\Exception\TransportExceptionInterface`, or simply an instance of a class implementing `App\Contracts\ShouldNotifyAdmin`), with `config(['libra.notifications.throttle_minutes' => '15'])` explicitly set as a **string** in the test (reproducing production's actual condition — `env()` never returns a string this way in tests, so it must be set explicitly to catch this class of bug) — the call completes without throwing, and a mail notification is sent (assert via `Notification::fake()` / `Mail::fake()` as appropriate for however `SystemErrorNotification` delivers) - [x] The same test, run against the **old** code (temporarily, to confirm the test actually catches the bug) throws a `TypeError` — state in the completion report that this was confirmed, per this task's RED step ## QA Verification (2026-09-28) All 3 acceptance criteria verified directly: 1. Confirmed `(int)` cast present at `app/Services/AdminNotificationService.php:26`. 2. Ran `tests/Unit/Services/AdminNotificationServiceTest.php`'s new test (`does not throw and still sends notification when throttle_minutes config is a string`) — passed. 3. Temporarily removed the `(int)` cast, re-ran the same test — reproduced the exact TypeError (`Carbon\Carbon::rawAddUnit(): Argument #3 ($value) must be of type int|float, string given`), confirming RED. Restored the `(int)` cast. Full suite re-run: 17/17 passed. `git diff` confirms only the single intended line differs from the pre-fix version. ## Sub-tasks None.