From 758869920c456519a280ab6c67d424a651d5d2e0 Mon Sep 17 00:00:00 2001 From: Marc Rejohn Castillano Date: Fri, 20 Mar 2026 20:50:45 +0800 Subject: [PATCH] =?UTF-8?q?Fix=20push=20notifications=20never=20delivered?= =?UTF-8?q?=20=E2=80=94=203=20root=20causes?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 1. CRITICAL: Double idempotency check silently discarded every notification. process_scheduled_notifications called try_mark_notification_pushed first, then send_fcm called it again and saw "already pushed" → skipped FCM send. Fix: Remove the check from process_scheduled_notifications; send_fcm's own check is sufficient (and needed for direct Flutter client calls). 2. SEND_FCM_URL env var crash: If not configured, edge function crashed on module load. Fix: Derive from SUPABASE_URL (always auto-set) as fallback. 3. Timing windows too narrow: ±30s windows with 60s cron intervals could miss events between runs. Fix: Widen to ±90s. ON CONFLICT DO NOTHING prevents duplicates, making wider windows safe. Co-Authored-By: Claude Opus 4.6 --- .../process_scheduled_notifications/index.ts | 23 ++++++------------- ...0260321_extend_scheduled_notifications.sql | 16 ++++++------- 2 files changed, 15 insertions(+), 24 deletions(-) diff --git a/supabase/functions/process_scheduled_notifications/index.ts b/supabase/functions/process_scheduled_notifications/index.ts index 7e11f803..e138f320 100644 --- a/supabase/functions/process_scheduled_notifications/index.ts +++ b/supabase/functions/process_scheduled_notifications/index.ts @@ -2,7 +2,8 @@ import { createClient } from 'npm:@supabase/supabase-js@2' // Minimal Deno Edge Function to process queued scheduled_notifications // Environment variables required: -// SUPABASE_URL, SUPABASE_SERVICE_ROLE_KEY, SEND_FCM_URL +// SUPABASE_URL, SUPABASE_SERVICE_ROLE_KEY +// Optional: SEND_FCM_URL (defaults to ${SUPABASE_URL}/functions/v1/send_fcm) const corsHeaders = { 'Access-Control-Allow-Origin': '*', @@ -10,7 +11,8 @@ const corsHeaders = { } const supabase = createClient(Deno.env.get('SUPABASE_URL')!, Deno.env.get('SUPABASE_SERVICE_ROLE_KEY')!) -const SEND_FCM_URL = Deno.env.get('SEND_FCM_URL')! +const SEND_FCM_URL = Deno.env.get('SEND_FCM_URL') + || `${Deno.env.get('SUPABASE_URL')!}/functions/v1/send_fcm` const BATCH_SIZE = Number(Deno.env.get('PROCESSOR_BATCH_SIZE') || '50') // deterministic UUIDv5-like from a name using SHA-1 @@ -58,20 +60,9 @@ async function processBatch() { const idSource = `${scheduleId || ''}-${r.task_id || ''}-${r.it_service_request_id || ''}-${r.pass_slip_id || ''}-${userId}-${notifyType}-${r.epoch || 0}` const notificationId = await uuidFromName(idSource) - // Attempt to mark idempotent push - const { data: markData, error: markErr } = await supabase.rpc('try_mark_notification_pushed', { p_notification_id: notificationId }) - if (markErr) { - console.warn('try_mark_notification_pushed error', markErr) - // do not mark processed; increment retry - await supabase.from('scheduled_notifications').update({ retry_count: r.retry_count + 1, last_error: String(markErr) }).eq('id', rowId) - continue - } - - if (markData === false) { - console.log('Notification already pushed, skipping', notificationId) - await supabase.from('scheduled_notifications').update({ processed: true, processed_at: new Date().toISOString() }).eq('id', rowId) - continue - } + // Idempotency is handled by send_fcm via try_mark_notification_pushed. + // Do NOT call it here — doing so would mark the notification as pushed + // before send_fcm sees it, causing send_fcm to skip the actual FCM send. // Prepare message based on notify_type let title = '' diff --git a/supabase/migrations/20260321_extend_scheduled_notifications.sql b/supabase/migrations/20260321_extend_scheduled_notifications.sql index deafca92..1d5afab4 100644 --- a/supabase/migrations/20260321_extend_scheduled_notifications.sql +++ b/supabase/migrations/20260321_extend_scheduled_notifications.sql @@ -71,8 +71,8 @@ BEGIN FOR rec IN SELECT id AS schedule_id, user_id, start_time AS start_at FROM public.duty_schedules - WHERE start_time BETWEEN now() + interval '15 minutes' - interval '30 seconds' - AND now() + interval '15 minutes' + interval '30 seconds' + WHERE start_time BETWEEN now() + interval '15 minutes' - interval '90 seconds' + AND now() + interval '15 minutes' + interval '90 seconds' AND status = 'active' LOOP -- Skip if user already checked in for this duty @@ -92,7 +92,7 @@ BEGIN FOR rec IN SELECT id AS schedule_id, user_id, end_time AS end_at FROM public.duty_schedules - WHERE end_time BETWEEN now() - interval '30 seconds' AND now() + interval '30 seconds' + WHERE end_time BETWEEN now() - interval '90 seconds' AND now() + interval '90 seconds' AND status IN ('arrival', 'late') LOOP INSERT INTO public.scheduled_notifications (schedule_id, user_id, notify_type, scheduled_for) @@ -235,8 +235,8 @@ BEGIN JOIN public.it_service_request_assignments isra ON isra.request_id = isr.id WHERE isr.status IN ('scheduled', 'in_progress_dry_run') AND isr.event_date IS NOT NULL - AND isr.event_date BETWEEN now() + interval '60 minutes' - interval '30 seconds' - AND now() + interval '60 minutes' + interval '30 seconds' + AND isr.event_date BETWEEN now() + interval '60 minutes' - interval '90 seconds' + AND now() + interval '60 minutes' + interval '90 seconds' LOOP INSERT INTO public.scheduled_notifications (it_service_request_id, user_id, notify_type, scheduled_for) @@ -348,8 +348,8 @@ BEGIN FOR rec IN SELECT ds.id AS schedule_id, ds.user_id, ds.end_time FROM public.duty_schedules ds - WHERE ds.end_time BETWEEN now() + interval '15 minutes' - interval '30 seconds' - AND now() + interval '15 minutes' + interval '30 seconds' + WHERE ds.end_time BETWEEN now() + interval '15 minutes' - interval '90 seconds' + AND now() + interval '15 minutes' + interval '90 seconds' AND ds.status IN ('arrival', 'late') -- User has pending (non-paused) tasks AND EXISTS ( @@ -395,7 +395,7 @@ BEGIN AND ps.slip_end IS NULL AND ps.slip_start IS NOT NULL AND ps.slip_start + interval '45 minutes' - BETWEEN now() - interval '30 seconds' AND now() + interval '30 seconds' + BETWEEN now() - interval '90 seconds' AND now() + interval '90 seconds' ON CONFLICT DO NOTHING; END; $$;