From b5bfb18f114fc3177e9fbf5ea307f6dfddec796d Mon Sep 17 00:00:00 2001 From: Morris Jencen Chavez Date: Mon, 14 Sep 2026 19:20:03 +0000 Subject: [PATCH 1/2] fix: make queue_worker health check reflect worker liveness, not scheduler MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The /health/detailed queue_worker status was derived from a Redis heartbeat key written directly by the queue:heartbeat artisan command, which runs on the scheduler's cron. Since the scheduler and queue worker are separate processes/containers, this only proved the scheduler was alive — if the queue worker crashed or its process died while the scheduler kept ticking, the heartbeat stayed fresh and /health/detailed kept reporting queue_worker as ok, giving false confidence while triggered webhooks silently queued up and were never delivered. queue:heartbeat now dispatches a WriteQueueHeartbeat job onto the same webhooks queue SendWebhook runs on, instead of writing the Redis key itself. The key is only written once an actual queue worker pulls the job off that queue and executes it, so staleness now reflects whether a worker is alive and processing jobs rather than whether the scheduler container is ticking. Fixes #108 --- ARCHITECTURE.md | 9 ++-- CLAUDE.md | 2 +- DEPLOYMENT.md | 2 +- WEBHOOK_README.md | 2 +- app/Console/Commands/QueueHeartbeat.php | 10 ++-- app/Jobs/WriteQueueHeartbeat.php | 36 +++++++++++++ routes/console.php | 5 +- tests/Feature/QueueHeartbeatTest.php | 67 +++++++++++++++++++++++++ 8 files changed, 122 insertions(+), 11 deletions(-) create mode 100644 app/Jobs/WriteQueueHeartbeat.php create mode 100644 tests/Feature/QueueHeartbeatTest.php diff --git a/ARCHITECTURE.md b/ARCHITECTURE.md index d7e8744..64c5e90 100644 --- a/ARCHITECTURE.md +++ b/ARCHITECTURE.md @@ -74,7 +74,7 @@ graph TB - **Session Management**: User session data - **Cache Layer**: Application-level caching - **Queue Backend**: Background job processing -- **Scheduler Heartbeat**: `queue:heartbeat` key written every minute to verify the scheduler container is alive +- **Queue Worker Heartbeat**: `queue:heartbeat` key, written by a job dispatched onto the `webhooks` queue every minute, so it's only updated when an actual queue worker (not just the scheduler) is alive and processing jobs ### 3. Background Processing @@ -83,13 +83,14 @@ graph TB // Job Types - SendWebhook: Handle webhook HTTP POST with HMAC signing, timing, and retry logic - ProcessWebhookRetries (command): Pick up failed deliveries ready for retry -- QueueHeartbeat (command): Write alive timestamp to Redis every minute +- QueueHeartbeat (command): Dispatch a WriteQueueHeartbeat job onto the `webhooks` queue every minute +- WriteQueueHeartbeat (job): Write alive timestamp to Redis when a queue worker processes it ``` #### Scheduler - **Laravel Scheduler**: Cron-like job scheduling via `routes/console.php` - **Retry Logic**: `webhooks:process-retries` runs every minute — exponential backoff delays configured via `WEBHOOK_BACKOFF_DELAYS` -- **Health Heartbeat**: `queue:heartbeat` runs every minute; the `/health` endpoint reports `stale` if no heartbeat within 2 minutes +- **Health Heartbeat**: `queue:heartbeat` runs every minute, dispatching a job onto the `webhooks` queue; `/health/detailed` reports `stale` if a queue worker hasn't processed one within 2 minutes ## 📊 Data Models & Relationships @@ -310,7 +311,7 @@ GET /health // HTTP 200 if healthy, 503 if any service is degraded ``` -`queue_worker` is `stale` (and the response is 503) if the scheduler heartbeat in Redis is older than 2 minutes, indicating the scheduler container is down. +`queue_worker` is `stale` (and the response is 503) if the queue worker heartbeat in Redis is older than 2 minutes, indicating a queue worker isn't processing jobs — whether because the worker process died or the scheduler stopped dispatching the heartbeat job. ### Error Handling - **Graceful Degradation**: Fallback mechanisms for service failures diff --git a/CLAUDE.md b/CLAUDE.md index f919020..cf22557 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -66,7 +66,7 @@ The scheduler (`routes/console.php`) runs `webhooks:process-retries` every minut ### Health check -`GET /health` is unauthenticated (for load balancer/orchestrator probes) and returns only `{"status", "timestamp"}` with a 200/503 code — no service breakdown, to avoid disclosing internal infrastructure details to anonymous callers. `GET /health/detailed` requires `auth:sanctum` and returns the full breakdown: `database`, `redis`, and `queue_worker` status, plus loaded PHP extensions. `queue_worker` is `stale` (503) when the Redis key `queue:heartbeat` is older than 2 minutes. The `QueueHeartbeat` artisan command writes this key every minute via the scheduler. +`GET /health` is unauthenticated (for load balancer/orchestrator probes) and returns only `{"status", "timestamp"}` with a 200/503 code — no service breakdown, to avoid disclosing internal infrastructure details to anonymous callers. `GET /health/detailed` requires `auth:sanctum` and returns the full breakdown: `database`, `redis`, and `queue_worker` status, plus loaded PHP extensions. `queue_worker` is `stale` (503) when the Redis key `queue:heartbeat` is older than 2 minutes. The scheduler runs the `QueueHeartbeat` artisan command every minute, which dispatches a `WriteQueueHeartbeat` job onto the `webhooks` queue; that job (not the command itself) writes the key, so staleness reflects whether an actual queue worker is alive and processing jobs, not just whether the scheduler container is ticking. ### Key models diff --git a/DEPLOYMENT.md b/DEPLOYMENT.md index 39f00d8..973d893 100644 --- a/DEPLOYMENT.md +++ b/DEPLOYMENT.md @@ -398,7 +398,7 @@ Authorization: Bearer } ``` -`queue_worker` is `unknown` if the application has never been fully started (no heartbeat key in Redis yet). The heartbeat is written by `php artisan queue:heartbeat`, which the scheduler container runs every minute. +`queue_worker` is `unknown` if the application has never been fully started (no heartbeat key in Redis yet). The scheduler container runs `php artisan queue:heartbeat` every minute, which dispatches a job onto the `webhooks` queue; the heartbeat key is only written once an actual queue worker processes that job, so staleness reflects worker liveness, not just the scheduler being up. ### Docker Health Check Add to Dockerfile: diff --git a/WEBHOOK_README.md b/WEBHOOK_README.md index f0bf2c7..546c9c4 100644 --- a/WEBHOOK_README.md +++ b/WEBHOOK_README.md @@ -366,7 +366,7 @@ sudo supervisorctl start webhook-worker:* ## Commands - `php artisan webhooks:process-retries` — Process failed webhook deliveries that are ready for retry -- `php artisan queue:heartbeat` — Write a heartbeat timestamp to Redis (run by the scheduler every minute; used by the `/health` endpoint to verify the scheduler is alive) +- `php artisan queue:heartbeat` — Dispatch a job onto the `webhooks` queue that writes a heartbeat timestamp to Redis when a queue worker processes it (run by the scheduler every minute; used by the `/health/detailed` endpoint to verify a queue worker, not just the scheduler, is alive) ## Development diff --git a/app/Console/Commands/QueueHeartbeat.php b/app/Console/Commands/QueueHeartbeat.php index 9474e3e..4db8bed 100644 --- a/app/Console/Commands/QueueHeartbeat.php +++ b/app/Console/Commands/QueueHeartbeat.php @@ -2,17 +2,21 @@ namespace App\Console\Commands; +use App\Jobs\WriteQueueHeartbeat; use Illuminate\Console\Command; -use Illuminate\Support\Facades\Redis; class QueueHeartbeat extends Command { protected $signature = 'queue:heartbeat'; - protected $description = 'Write a heartbeat timestamp to Redis so the health check can verify the scheduler is running'; + protected $description = 'Dispatch a job onto the webhooks queue so the health check can verify a live queue worker (not just the scheduler) is running'; public function handle(): void { - Redis::set('queue:heartbeat', now()->timestamp); + // Dispatched rather than written directly: the heartbeat key must + // only update once an actual queue worker processes this job, so + // staleness reflects worker liveness instead of just the scheduler + // (which runs this command) being alive. See WriteQueueHeartbeat. + WriteQueueHeartbeat::dispatch(); } } diff --git a/app/Jobs/WriteQueueHeartbeat.php b/app/Jobs/WriteQueueHeartbeat.php new file mode 100644 index 0000000..4b49abf --- /dev/null +++ b/app/Jobs/WriteQueueHeartbeat.php @@ -0,0 +1,36 @@ +onQueue('webhooks'); + } + + public function handle(): void + { + Redis::set('queue:heartbeat', now()->timestamp); + } +} diff --git a/routes/console.php b/routes/console.php index e4b01c6..63f8696 100644 --- a/routes/console.php +++ b/routes/console.php @@ -15,5 +15,8 @@ // double-dispatch the same Delivery to the customer's endpoint. Schedule::command('webhooks:process-retries')->everyMinute()->withoutOverlapping(); -// Write a heartbeat so the /health endpoint can verify the scheduler is alive +// Dispatch a heartbeat job onto the webhooks queue every minute. The +// heartbeat key is only written once a live queue worker actually +// processes the job, so /health/detailed can tell a dead worker apart +// from a dead scheduler instead of conflating the two (see #108). Schedule::command('queue:heartbeat')->everyMinute(); diff --git a/tests/Feature/QueueHeartbeatTest.php b/tests/Feature/QueueHeartbeatTest.php new file mode 100644 index 0000000..1ba0568 --- /dev/null +++ b/tests/Feature/QueueHeartbeatTest.php @@ -0,0 +1,67 @@ + $job->queue === 'webhooks' + ); + } + + public function test_queue_heartbeat_command_does_not_write_the_heartbeat_key_itself(): void + { + // The command only enqueues the job; if it wrote the Redis key + // directly, the heartbeat would stay "fresh" even while the actual + // queue worker process is dead, reproducing the original bug. + Queue::fake(); + Redis::shouldReceive('set')->never(); + + Artisan::call('queue:heartbeat'); + } + + public function test_write_queue_heartbeat_job_writes_the_current_timestamp_to_redis(): void + { + $now = now(); + $this->travelTo($now); + + Redis::shouldReceive('set') + ->once() + ->with('queue:heartbeat', $now->timestamp); + + (new WriteQueueHeartbeat)->handle(); + } + + public function test_write_queue_heartbeat_job_runs_on_the_webhooks_queue(): void + { + // SendWebhook also runs on the `webhooks` queue, so a worker that + // has stopped processing deliveries also stops processing this job + // — which is exactly what makes the heartbeat a real liveness + // signal for the worker that matters. + $job = new WriteQueueHeartbeat; + + $this->assertSame('webhooks', $job->queue); + } +} From e97bb1729503c251d406388334dc8082895e4002 Mon Sep 17 00:00:00 2001 From: Claude Date: Tue, 15 Sep 2026 03:16:39 +0000 Subject: [PATCH 2/2] fix: apply psr12 constructor call style in QueueHeartbeatTest CI's Lint (Pint) check runs with --preset=psr12, which requires parentheses on `new ClassName()` expressions. Fixes the two constructor calls in the new heartbeat test to match. --- tests/Feature/QueueHeartbeatTest.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/Feature/QueueHeartbeatTest.php b/tests/Feature/QueueHeartbeatTest.php index 1ba0568..b1e9f82 100644 --- a/tests/Feature/QueueHeartbeatTest.php +++ b/tests/Feature/QueueHeartbeatTest.php @@ -51,7 +51,7 @@ public function test_write_queue_heartbeat_job_writes_the_current_timestamp_to_r ->once() ->with('queue:heartbeat', $now->timestamp); - (new WriteQueueHeartbeat)->handle(); + (new WriteQueueHeartbeat())->handle(); } public function test_write_queue_heartbeat_job_runs_on_the_webhooks_queue(): void @@ -60,7 +60,7 @@ public function test_write_queue_heartbeat_job_runs_on_the_webhooks_queue(): voi // has stopped processing deliveries also stops processing this job // — which is exactly what makes the heartbeat a real liveness // signal for the worker that matters. - $job = new WriteQueueHeartbeat; + $job = new WriteQueueHeartbeat(); $this->assertSame('webhooks', $job->queue); }