Skip to content

Confiabilidade/Leaderboard: persistMatch descarta os erros de insert — corrida terminada some do ranking em silêncio e deixa matches órfã (rooms/[code]/route.ts:271-282) #112

Closed

Description

@caioross

Contexto

A #56 consertou o error descartado das actions de sala — inclusive o flip do finish, que
hoje devolve 500 honesto quando o update falha (src/app/api/rooms/[code]/route.ts:175-181).
Mas a persistência que roda logo depois desse flip (:186) ficou no padrão antigo, e é
justamente ela que decide se a corrida existe para o mundo:

src/app/api/rooms/[code]/route.ts:271-282

// Persist a finished match + scores for the leaderboard (best-effort).
async function persistMatch(sb: SupabaseClient, room: RoomRow, results: ResultRow[]) {
  try {
    const matchRow = buildMatchRow(room, results, new Date().toISOString());
    if (!matchRow) return;
    const { data: match } = await sb.from("matches").insert(matchRow).select("id").single();  // :276
    if (!match) return;
    await sb.from("scores").insert(buildScoreRows(match.id, room, results));                  // :278
  } catch (e) {
    console.warn("[persistMatch]", (e as Error).message);
  }
}

Dois furos, ambos mudos:

  1. :276 descarta error. O supabase-js não lança em erro de query — devolve
    { data: null, error }. Então uma falha (constraint NOT VALID da migration 0004, RLS,
    rede, timeout) faz match virar null, cair no return de :277 e não passar nem
    pelo catch
    . Zero linhas de log. A partida simplesmente não aconteceu.
  2. :278 descarta o retorno inteiro — nem data, nem error. Se o insert de scores
    falhar, a linha de matches já foi gravada: sobra uma partida órfã que aparece em
    "Partidas recentes" (getRecentMatches, src/lib/supabase.ts) com player_count e
    vencedor, enquanto nenhum dos jogadores entra no ranking — porque o leaderboard é
    uma view sobre scores (0001_coderacer_init.sql:40-52). O placar mente e ninguém sabe.

Em qualquer dos dois casos a rota responde ok: true (:188) e o cliente mostra a corrida
encerrada com sucesso.

Escopo honesto — o que este pedido NÃO é: o finish não deve passar a falhar por
causa da persistência. Quando persistMatch roda, a linha da sala já flipou e o broadcast já
saiu (:185-186); devolver 500 aqui faria o cliente acreditar que a corrida não terminou —
regressão pior que o bug. O pedido é observabilidade + integridade do par matches/scores,
não transação distribuída.

Acceptance criteria

  1. O error do insert de matches (:276) é lido e registrado com console.error
    (não um warn que nunca dispara), identificando a sala. Falhou → não tenta gravar scores.
  2. O error do insert de scores (:278) é lido e registrado da mesma forma.
  3. A matches não fica órfã. Se o insert de scores falhar, o par não pode sobreviver
    pela metade — o Resolvedor escolhe entre (a) deletar, na mesma requisição, a linha de
    matches que essa própria chamada acabou de criar (pelo id retornado em :276), ou
    (b) gravar o par de forma que a ausência de scores seja impossível. Justifique a
    escolha no PR.
    Nota de escopo: a variante (a) é lógica de aplicação sobre uma linha
    recém-criada no mesmo request — não é migração destrutiva, então não cai no §7.1; o
    quórum do §7.2 (abaixo) é o crivo correto.
  4. A resposta do finish continua { ok: true } mesmo com falha de persistência. Nenhuma
    mudança de comportamento visível no cliente.
  5. Cobertura dos caminhos de erro: a Testes/Persistência: extrair mapeamento puro de persistMatch (results→matches/scores) e cobrir com Vitest #50 já extraiu buildMatchRow/buildScoreRows
    puros e testados — o que falta é o wrapper. Cubra "matches falha" e "scores falha" em
    vitest e/ou scripts/validate-persistence.mjs, com um sb dublê que devolve { error }.
  6. Gate verde: pnpm typecheck && pnpm build, vitest, validate-metrics,
    validate-persistence.

Dica de abordagem

O arquivo já tem o padrão pronto a copiar: o finish em :175-181 mostra exatamente como
ler o error, logar com contexto e decidir. A diferença é o que se faz depois — aqui a
decisão é logar e seguir, nunca propagar 500.

Atenção a um detalhe do .single() em :276: ele devolve erro também quando a query não
retorna exatamente uma linha, então o log precisa distinguir "falhou a escrita" de "escreveu
e não voltou linha" para não virar ruído.

Classificação: §7.2 — área de quórum (toca src/app/api/rooms/**). PR non-draft com a
linha exata Solicito quórum (HANDBOOK §7).

Activity

  1. added
    P2Prioridade normal
    area:multiplayerSalas, Realtime, presença, leaderboard, persistência
    area:infraCI/CD, build, tooling, deploy, deps
    on Jul 30, 2026
  2. caioross commented on Jul 31, 2026

    @caioross
    OwnerAuthor

    🏛️ Parecer do Conselho · 2026-07-31

    Veredito: APROVADA PARA EXECUÇÃO — com (a) recomendado e justificado pelo schema, e uma pré-condição estrutural que a AC 5 exige sem dizer.

    Auditado contra origin/main @ 098372e (o clone do dono está atrás da origin).

    • Engenharia: O diagnóstico da issue está certo linha a linha; confirmei :276/:278 e o catch que nunca dispara.
      Pré-condição que a AC 5 implica e não escreve: cobrir os caminhos de erro com um sb dublê exige importar persistMatch, e hoje ela é uma função privada dentro de src/app/api/rooms/[code]/route.ts:271. O vitest não a importa sem arrastar next/server, e exportá-la de um route file do App Router é ambíguo (o Next trata exports desses módulos como configuração de rota). O validate-persistence.mjs também não serve: ele é declaradamente um espelho sem import de TS (scripts/validate-persistence.mjs:66-69) — espelhar um wrapper de I/O ali testaria a cópia, não o código. → Extraia persistMatch para src/lib/ (ex.: src/lib/persistMatch.ts), recebendo sb por parâmetro — ela já recebe. A rota passa a importar e a testabilidade cai de graça. Sem esse passo a AC 5 não fecha honestamente.
      AC 3 — recomendo (a), e o schema decide: scores.match_id referencia matches(id) on delete cascade (0001_coderacer_init.sql:24), então apagar a matches recém-criada é um statement só, sem risco de scores órfão na direção oposta; e como o insert roda com service_role (que ignora RLS), não é preciso policy de DELETE nenhuma. Já (b) — "gravar o par de forma que a ausência de scores seja impossível" — exige transação, e o supabase-js não tem transação sobre PostgREST: viraria função Postgres + migração nova, mais superfície e mais deploy-ordering para um problema que (a) resolve em 4 linhas. (a) é a fatia honesta.
      Guarde o guarda: o delete compensatório também pode falhar. Se falhar, a linha órfã que a AC 3 proíbe existe — esse caso precisa de console.error próprio e explícito, nunca de um catch mudo. É o único jeito de a AC 3 ser verdadeira em vez de otimista.
      .single() (:276): o PostgREST devolve erro também quando a contagem ≠ 1 — o código costuma ser PGRST116; confirme no PR em vez de acreditar neste parecer, e ramifique o log por error.code para separar "não escreveu" de "escreveu e não voltou linha", como a issue pede. Não logue a linha inteira: ela carrega os nicks dos jogadores.
      Não é bug, não "conserte": sb.from("scores").insert([]) é inalcançável — buildMatchRow devolve null quando results está vazio (src/lib/room.ts:381) e a função retorna antes (route.ts:274).

    • Segurança/Anti-cheat: Nada afrouxa; a fronteira anti-cheat é o sanitizeResults de :163, muito antes daqui. Vale registrar o modo de falha que este log torna visível: getServerSupabase() cai para a chave anon quando SUPABASE_SERVICE_ROLE_KEY falta (src/lib/supabase.ts:12-15), e matches/scores têm RLS ligada com apenas policies de SELECT (0001:57-66) — nenhuma de INSERT. Num deploy sem service_role, toda persistMatch falharia, sempre, e o ranking ficaria permanentemente vazio enquanto cada corrida responde ok: true. Não é o caso da produção (a Anti-cheat/Leaderboard: recorde impossível (3596 WPM) no Ranking Global — sem teto de plausibilidade #28 registra recorde no Ranking Global, logo o insert funciona lá) — mas é exatamente a classe de falha total-e-silenciosa que o error descartado esconde, e é o melhor argumento para o log ser console.error com o código do erro junto.

    • UX/Game-feel: A AC 4 está certa: falhar o finish seria regressão pior. Mas o custo honesto tem nome — o jogador vê o pódio e o link ver ranking global (Results.tsx:157-162) apontando para um ranking em que ele não entrou. Esta fatia não resolve isso (resolver exige sinalizar o cliente, o que a AC 4 proíbe) e não deve tentar; fica como issue futura de UX. Sem mudança de copy, movimento ou reduced-motion aqui.

    • Performance: Fora da área sagrada — persistMatch é aguardada em route.ts:186, depois do flip e do broadcast, no finish do último jogador; nenhuma tecla em voo. O delete compensatório adiciona um round-trip só no caminho de falha: custo zero no caminho feliz. Não introduza retry nesta fatia: o broadcast já saiu em :185, então um retry sobre uma resposta perdida cujo insert de fato gravou produziria uma segunda matches para a mesma corrida — troca um bug silencioso por outro. Se alguém quiser idempotência de verdade, é outra issue (chave única por room_code + finished_at).

    • Produto: P2 correta — não há incidente conhecido e a produção persiste. O que sustenta a prioridade é a persona "O Cético Sênior", que julga "se o WPM bate com a realidade" (docs/UI-AAA-OVERHAUL.md §0.3): o ranking é o único artefato durável do produto, e hoje ele pode perder corridas sem deixar uma linha de log. O sub-caso do órfão é pior que perder: "Partidas recentes" (supabase.ts:124-143) exibe a partida com vencedor e player_count enquanto nenhum jogador entra no ranking — o placar mente com aparência de verdade.

    Plano sugerido:

    1. Extraia persistMatch de route.ts:271-282 para src/lib/persistMatch.ts, assinatura inalterada (sb, room, results). A rota só importa e chama em :186.
    2. Leia o error do insert de matches: const { data: match, error: matchErr } = await sb.... matchErr → console.error("[persistMatch:matches]", room.code, matchErr.code, matchErr.message) e return (nenhum scores). Sem matchErr e sem match → log próprio; é o ramo PGRST116.
    3. Leia o error do insert de scores. Se falhar: console.error("[persistMatch:scores]", room.code, match.id, ...) e apague a matches que esta chamada acabou de criar, por id — await sb.from("matches").delete().eq("id", match.id). Leia o erro desse delete também e logue como console.error se ele falhar (é o órfão que a AC 3 proíbe; ele não pode ser mudo).
    4. Mantenha o try/catch externo, mas troque console.warn por console.error — hoje ele só pegaria exceção de verdade, e essa merece o mesmo nível.
    5. finish intocado: route.ts:188 continua { ok: true } em qualquer desfecho.
    6. Testes em src/lib/persistMatch.test.ts com um sb dublê que devolve { data: null, error: {...} } por tabela: (i) matches falha → scores nunca é chamada; (ii) scores falha → delete é chamado com o id certo; (iii) delete também falha → não lança, e loga; (iv) caminho feliz → 2 inserts, 0 deletes; (v) results vazio → 0 inserts (regressão do buildMatchRow → null). Espie o console.error com vi.spyOn.
    7. Gate: pnpm typecheck && pnpm build, vitest, validate-metrics, validate-persistence. §7.2 — PR non-draft com Solicito quórum (HANDBOOK §7). Na PR, justifique (a) vs (b) citando o on delete cascade de 0001:24, como a AC 3 pede.
  3. added
    em-resolucaoClaim: um Resolvedor está trabalhando nisto
    and removed
    em-resolucaoClaim: um Resolvedor está trabalhando nisto
    on Aug 3, 2026
  4. caioross commented on Aug 3, 2026

    @caioross
    OwnerAuthor

    ✅ Resolvida na PR #127

    persistMatch saiu do route file para src/lib/persistMatch.ts (a AC 5 não fecha sem isso — o
    vitest não importa um module de rota do App Router), os error dos dois inserts passaram a ser
    lidos e logados com console.error + código do PostgREST, e o scores que falha agora dispara o
    delete da matches que a mesma chamada criou — variante (a) da AC 3, justificada pelo
    on delete cascade de 0001_coderacer_init.sql:24. O erro desse delete também é logado
    ([persistMatch:rollback]): a órfã que a AC 3 proíbe nunca é muda. finish intocado — segue
    { ok: true } (AC 4).

    Gate verde: typecheck, build, pnpm test 181/181 (8 testes novos cobrindo os caminhos de erro),
    validate-persistence 72/72, validate-metrics 45/45.

    PR non-draft com quórum (§7.2 — toca src/app/api/rooms/**). Segui o plano do Parecer do Conselho
    na íntegra; as três premissas que ele mandou reverificar foram confirmadas contra origin/main.

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

    P2Prioridade normalarea:infraCI/CD, build, tooling, deploy, depsarea:multiplayerSalas, Realtime, presença, leaderboard, persistência

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions