libra/docs/tasks/23.md

id, title, status, date, parent

id	title	status	date	parent
23	23 Fix AdminNotificationService Carbon TypeError Masking Exceptions	Done	2026-09-28	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):

        $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:

        $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

• app/Services/AdminNotificationService.php's $throttleMinutes assignment casts the config value with (int)
• 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)
• 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.