Skip to content

finalizeRelayFailureRetryRow's unguarded DB writes contradict retryFailedRelays's documented Never throws contract #8332

Description

@JSONbored

Context

src/orb/relay.ts's retryFailedRelays doc comment states: "Never throws — a bad DB row or a persistently-down container is dropped (with an alertable log, below) after exhaustion."

Its per-row finalize step, finalizeRelayFailureRetryRow (same file, ~line 104-125), issues D1 writes with no try/catch:

async function finalizeRelayFailureRetryRow(
  env: Env,
  row: { delivery_id: string; event_name: string; installation_id: number },
  outcome: RelayForwardOutcome,
): Promise<void> {
  if (isRelayFailureRetryTerminal(outcome, row.event_name)) {
    await env.DB.prepare("DELETE FROM orb_relay_failures WHERE delivery_id = ?").bind(row.delivery_id).run();
    return;
  }
  await env.DB
    .prepare("UPDATE orb_relay_failures SET attempts = attempts + 1, last_attempt_at = datetime('now') WHERE delivery_id = ?")
    .bind(row.delivery_id)
    .run();
  ...
}

retryFailedRelays calls this from retryRow (an inner function run via Promise.all in batches, ~line 501-503):

const retryRow = async (row) => {
  const outcome = await forwardOrbEvent(env, { ... }, opts?.fetchImpl);
  await finalizeRelayFailureRetryRow(env, row, outcome);
};

for (let i = 0; i < results.length; i += RELAY_RETRY_CONCURRENCY) {
  await Promise.all(results.slice(i, i + RELAY_RETRY_CONCURRENCY).map(retryRow));
}

If finalizeRelayFailureRetryRow's DB write throws (a transient D1 error) right after forwardOrbEvent already succeeded, the Promise.all rejects, retryFailedRelays throws out of the function — directly contradicting its own "Never throws" doc comment — and the failure row is never deleted despite the event having already been successfully forwarded. The practical consequence: the same event gets redelivered to the container on the next retry tick, since the row still shows as pending retry. No test in test/integration/orb-relay.test.ts exercises a DB write failure inside the finalize step (only DB failures during the earlier SELECT queries, around line 1099, are tested today).

Requirements

  • Wrap finalizeRelayFailureRetryRow's two env.DB calls (the DELETE and the UPDATE) in a try/catch that logs the failure (following this file's existing logging conventions, e.g. the structured console.error pattern used elsewhere for alertable conditions in this same file) and swallows the error rather than letting it propagate — consistent with how other best-effort DB operations elsewhere in relay.ts are already guarded.
  • The swallow must not silently hide the double-delivery risk: the log entry must include enough context (delivery_id, event_name, the outcome that had already succeeded) for the maintainer to recognize a duplicate-forward risk from the log if this fires.

Deliverables

  • finalizeRelayFailureRetryRow's DB writes wrapped in try/catch with an alertable log on failure, matching this file's existing logging conventions.
  • A regression test in test/integration/orb-relay.test.ts that makes env.DB's DELETE/UPDATE call inside the finalize step throw immediately after a successful forward, asserting retryFailedRelays resolves (does not throw) and the failure is logged.

Test Coverage Requirements

This repo's Codecov patch gate requires 99%+ coverage of changed lines and branches. Cover both the new catch branch and confirm the existing successful-finalize (DELETE and UPDATE) branches remain covered.

Expected Outcome

retryFailedRelays's "Never throws" contract is actually enforced end-to-end, including its finalize step — a transient D1 write failure immediately after a successful forward no longer aborts the whole retry batch or risks a duplicate redelivery of an already-forwarded event.

Links & Resources

  • src/orb/relay.tsfinalizeRelayFailureRetryRow (~line 104-125) and retryFailedRelays (~line 456-503, doc comment ~454)
  • test/integration/orb-relay.test.ts — existing DB-failure coverage at the SELECT stage, ~line 1099

Metadata

Metadata

Assignees

No one assigned

    Labels

    gittensor:bugGittensor-scored bug fix — scores a 0.05x multiplier.help wantedExtra attention is needed

    Projects

    No projects

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions