useCancelSubscription treats every buildCancelTransaction failure as "no signature required" and cancels anyway

[abayomicornelius]

Problem

src/hooks/useSubscriptions.ts's useCancelSubscription:

export function useCancelSubscription() {
  const { signTransaction } = useWallet()
  const queryClient = useQueryClient()

  return useMutation<Subscription, Error, string>({
    mutationFn: async (subscriptionId: string) => {
      // Some cancellations require an on-chain authorization (e.g. revoking a
      // pre-signed standing order); others are purely a backend-side flag
      // flip. Try the build step, but proceed without a signature if the
      // backend says none is needed.
      let signedXdr: string | undefined
      try {
        const built = await subscriptionApi.buildCancelTransaction(subscriptionId)
        if (built?.xdr) {
          signedXdr = await signTransaction(built.xdr)
        }
      } catch {
        signedXdr = undefined
      }
      return subscriptionApi.cancel(subscriptionId, signedXdr)
    },
    onSuccess: () => {
      queryClient.invalidateQueries({ queryKey: subscriptionKeys.all })
    },
  })
}

The comment describes a legitimate distinction: some subscriptions need an on-chain revocation transaction, others are a pure backend flag flip signaled by the build endpoint returning no xdr. But the catch {} conflates that legitimate "no xdr in the response" case with any exception at all — including the build endpoint being down, a 500, a network timeout, or the user rejecting the Freighter signature prompt (signTransaction throwing because the user clicked "Reject" in the extension).

Why it matters

• If buildCancelTransaction 500s (backend bug, outage) the code proceeds exactly as if the backend had explicitly said "no signature needed," and calls subscriptionApi.cancel(subscriptionId, undefined). If this particular subscription actually does require an on-chain revocation to stop the standing order (per the comment's own example), the backend may mark it "cancelled" in its database while the actual on-chain authorization remains live — meaning the recurring payment keeps executing even though the UI now shows the subscription as cancelled and offers no way to cancel it again (canCancel in SubscriptionItem.tsx is status === 'active' || status === 'paused', so a subscription marked 'cancelled' loses its Cancel button entirely).
• If the user rejects the Freighter signature prompt (a normal, expected user action — "I changed my mind about cancelling"), signTransaction throws, is caught, and the code proceeds to cancel anyway with signedXdr: undefined — the opposite of what the user just decided.

Reproduction

• Mock subscriptionApi.buildCancelTransaction to resolve with an xdr, then mock signTransaction to reject (simulating the user clicking "Reject" in Freighter). Observe subscriptionApi.cancel is still called and the mutation resolves successfully, cancelling the subscription against the user's explicit refusal to sign.

Suggested fix

• Distinguish "build endpoint responded with no xdr" (legitimate no-signature-needed path) from "build endpoint threw" (unknown — must not silently proceed) at the type level: have buildCancelTransaction return a discriminated result ({ requiresSignature: false } vs { requiresSignature: true, xdr: string }) rather than relying on falsy-XDR-or-thrown-exception duck typing.
• Do not catch errors from signTransaction at all here — user rejection should abort the cancel mutation with a clear "Cancellation was not signed" error, not proceed silently.
• Only allow the "cancel without signature" path when the build step explicitly reports it's unnecessary, never as a byproduct of catching an unrelated exception.

Edge cases

• Subscriptions created before this fix shipped may already be in this inconsistent state (marked cancelled server-side, still active on-chain) — a reconciliation/audit script may be needed, out of scope for the frontend fix itself but worth noting for backend follow-up.

Testing strategy

• Unit test: buildCancelTransaction resolves without xdr → cancel called with undefined, mutation succeeds (happy path, unchanged).
• Unit test: buildCancelTransaction rejects (500) → mutation should reject/error, cancel must not be called.
• Unit test: buildCancelTransaction resolves with xdr, signTransaction rejects (user declines) → mutation should reject/error, cancel must not be called.

Related issues in this batch

Shares the "swallow error, proceed as if things are fine" root cause with useBatchPayment's Horizon fallback and useCreateSubscription's one-time-payment fallback in this same batch.