Next.js Server Action Fail-Open: error Read as Success

The short version

A Next.js server action discarded the error from a Supabase DB update and judged success purely by whether {data} existed. As a result, a transient DB failure during a signature hold left the customer page still saying “your signature is on hold” while internal Slack was told, in the opposite direction, “not held – already signed.” It was caught in pre-deploy review on 2026-09-16 and fixed the same day; there’s no record of this path ever running in production.

The symptom

Before the fix, the code handling a signature hold read the update like this:

const { data: heldRow } = await admin
  .from("contracts")
  .update({ review_status: "レビュー中" })
  .eq("id", link.contract_id)
  .or("application_status.is.null,application_status.neq.署名済")
  .select("id")
  .maybeSingle();
const held = !!heldRow;

error isn’t even destructured – it’s discarded outright. held depends only on whether heldRow is truthy, so “no row matched (already signed)” and “the update itself failed (a DB error)” both collapse to the same false.

held is then read in two places. On the customer-facing page:

{!isSigned && (
  <>
    <br />
    Your signature is on hold until we reply.
  </>
)}

This only checks isSigned; it never looks at whether the hold UPDATE actually succeeded. In the internal Slack notification:

(held
  ? "⏸ *Signature held.* The customer cannot sign until staff approves (agreed by memo → approval)\n"
  : "※ This contract is already signed, so no hold was placed (post-signing follow-up)\n")

If held is false for any reason, this unconditionally states “already signed, so no hold was placed” – including when heldRow came back empty because of a DB error.

Why

This page accepts pre-signing requests from customers who want contract edits or special terms; a successful hold is what lets staff review before signing. Swallowing error opened exactly the failure path this feature was meant to prevent.

• The customer page, looking only at isSigned, kept saying “on hold” with the sign button still clickable
• Internal Slack assumed the reason held was false was “already signed,” and staff read that as nothing to do

A single DB failure got translated into two separate false reassurances: “safely held” to the customer, “no action needed” to staff. supabase-js doesn’t throw on a transport or query failure – it returns {data: null, error} – so unless error is read explicitly, this kind of failure is swallowed silently.

Fixing it

error is now read, and the reason held came back false is split into “already signed (normal)” and “failed (needs attention)”:

const { data: heldRow, error: holdError } = await admin
  .from("contracts")
  .update({ review_status: "レビュー中" })
  .eq("id", link.contract_id)
  .or("application_status.is.null,application_status.neq.署名済")
  .select("id")
  .maybeSingle();
if (holdError) console.error("[p/token] signing hold failed:", holdError);
const held = !!heldRow && !holdError;
const holdMiss: "signed" | "error" | null = held ? null : holdError ? "error" : "signed";

The Slack message now branches on holdMiss across three cases:

held
  ? "⏸ *Signature held.* The customer cannot sign until staff approves (agreed by memo → approval)\n"
  : holdMiss === "error"
    ? "🔴 *Failed to place hold.* The customer can still sign right now. Set review_status to \"レビュー中\" immediately\n"
    : "※ This contract is already signed, so no hold was placed (post-signing follow-up)\n"

The customer-facing notice was also changed to depend only on whether a hold is actually active (isReviewing), not on isSigned:

{isReviewing && (
  <>
    <br />
    Your signature is on hold until we reply.
  </>
)}

hold_miss_reason is now written to the audit log as well, so the reason signing_held came back false can be traced from the log alone.

Lesson

The evidence behind this article does not show any mechanical check added to catch a swallowed error in general – the fix addressed this one call site directly, and whether the same pattern exists elsewhere isn’t something the evidence covers.

What’s structurally visible is that any API returning {data, error} becomes fail-open the moment error is left out of the destructure. This code looked only at whether a result existed, not why it didn’t – and then reused that single verdict for two destinations with very different audiences (a customer-facing message and an internal one), so one swallowed error turned into two separate, contradictory false assurances.