Skip to content

Draft timer expiry 500s the polling endpoint and wedges the draft (P0) #254

Description

@diese-tech

Problem

GET /api/draft/[id] is the polling endpoint every draft client hits every 3 seconds — captain boards, the admin panel, and the caster overlay (#249). When a pick timer expires it first tries an auto-pick from the org's shortlist; if no eligible shortlisted player exists, it falls back to advancePickOnTimeout() (src/lib/draft-data.ts:258), which calls RPC advance_pick_on_timeout through a hand-rolled as unknown as cast with a comment admitting it "is not yet part of the released shared contract."

That function does not exist in production. Verified directly against the live database:

SELECT p.oid::regprocedure, p.prosecdef, p.proconfig FROM pg_proc p
JOIN pg_namespace n ON n.oid = p.pronamespace
WHERE n.nspname='public' AND p.proname IN
  ('advance_pick_on_timeout','submit_draft_pick','undo_last_pick');
-- → submit_draft_pick(text,text,text,integer,integer)
-- → undo_last_pick(text)
-- advance_pick_on_timeout: ABSENT

Corroborated in-repo: zero occurrences anywhere in sal-database/supabase/migrations/ or its git history on any branch; absent from the released contract's generated types (site is pinned to db-v1.11.0); and sal-database/supabase/migrations/README.md + docs/baseline-evidence.md state explicitly that archived site migrations 019–025 — where 020_atomic_advance_on_timeout.sql lives — "were not recorded in production and their effects were not all present." baseline-evidence.md names "timeout handling" as deferred forward work.

advancePickOnTimeout does if (error) throw error with no fallback, so PostgREST's 42883 function does not exist propagates out of the GET handler as a 500. Because the room stays past its deadline, every polling client keeps 500ing until an admin manually intervenes — the draft wedges rather than degrading.

Trigger: a captain lets the clock run out with an empty shortlist, or with every shortlisted player already drafted. Ordinary, not exotic.

Why tests miss it: src/app/api/draft/[id]/route.test.ts mocks advancePickOnTimeout, so the missing function is invisible to the existing suite. This is also the only uncontracted .rpc cast in the codebase — all 18 other RPC call sites are contract-typed.

Fix

  1. Reimplement the timer-expiry skip using updateDraftRoomGuarded (src/lib/draft-data.ts:190) instead of the nonexistent RPC — the same guarded-update pattern already used by the admin skip route (src/app/api/admin/draft/[id]/skip/route.ts:24, the DE-02 fix for issue fix: guard draft config after start and make admin skip concurrency-safe #216). The .eq("current_pick_index", …) guard is what actually prevents the double-advance from [BUG P2] Draft auto-skip on timer expiry is not atomic — concurrent polls can double-advance the pick index #129; concurrent pollers race the update and only one matches a row.
  2. Wrap the whole timeout-resolution block in src/app/api/draft/[id]/route.ts in try/catch so any failure degrades to "return current state; next poll retries" — the same treatment getLeagueData() failure already gets a few lines above it. Report via reportError.
  3. Delete advancePickOnTimeout and its cast (draft-data.ts:258-279) — removes the codebase's last uncontracted RPC.

Tradeoff, stated honestly: this drops the RPC's row lock and its timer re-validation under that lock. The index guard still makes double-advance impossible (the original #129 bug). What's lost is protection against a caller acting on a stale read, but such a caller can only ever advance the slot it observed, and a concurrent legitimate pick wins that race via the same index guard.

Follow-ups (not blocking this fix)

  • Port a locked, timer-revalidating resolver into sal-database as a proper forward migration.
  • ADR-0003's unified slot-resolution function shared by picks, timeouts, captain skips, and admin skips (§353-374 of sal-database/docs/adr/0003-draft-room-lifecycle-authorization-and-failure-recovery.md).
  • A skipped slot currently leaves a pick_number gap with no row; ADR-0003 wants this recorded as a permanent roster vacancy.
  • Add a real-database RPC-parity test (enumerate every site .rpc() name, assert presence in pg_proc) so a missing function fails CI instead of production.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions