Skip to content

fix(sdk): stop signature step polling on failure or refund - #1144

Merged
pedromcunha-uneven merged 2 commits into
relayprotocol:mainfrom
erkancamli:fix/sdk-signature-check-final-status
Nov 7, 2026
Merged

pedromcunha-uneven merged 2 commits into
relayprotocol:mainfrom
erkancamli:fix/sdk-signature-check-final-status

Conversation

@erkancamli

Copy link
Copy Markdown
Contributor

In signatureStep.ts status polling, a failure throws Error(res.data.details || 'Transaction failed'), but the catch only rethrows messages containing "Transaction failed" or "Failed to check". Any other details string is treated as a network error and retried until maxAttempts, and then surfaces as Failed to get an ok response after N attempt(s), losing the real reason. refund, which /intents/status/v3 can return, has no branch at all, so it also polls until the timeout.

transaction.ts and the websocket handlers already treat both as final (Transaction failed: Refunded). With websocket off by default, every signature based quote with a check endpoint goes through this polling path.

The change adds a small CheckStatusError that failure and a new refund branch throw and the catch rethrows. Request errors, including 4xx such as 429, are still retried as before. A refund now marks the step as refunded like the other paths do.

Tests: three cases in signatureStep.test.ts. The failure with details and the refund cases fail on main (got 'Failed to get an ok response after 5 …') and pass now; the third confirms request errors are still retried. pnpm typecheck passes. The rest of the sdk suite passes except client.test.ts > Should configure chains dynamically, which calls the live chains API and fails the same way on main in my environment (no outbound access to it).

@vercel

vercel Bot commented Sep 30, 2026

Copy link
Copy Markdown
Contributor

@erkancamli is attempting to deploy a commit to the Uneven Labs Team on Vercel.

A member of the Team first needs to authorize it.

@greptile-apps

greptile-apps Bot commented Sep 30, 2026 •

Copy link
Copy Markdown

RetriggerConfidence Score: 5/5

[Medium risk] Changes signature transaction polling behavior on failure.

The PR appears safe to merge; the previously reported stale refund status is fixed.

Summary

The PR stops signature-step polling when the status check reports failure or refund, preserves failure details, and adds polling tests. The refund path now marks the step item as refunded.

Reviews (3) · Last reviewed commit: "fix(sdk): mark the signature step item a..."

A failure whose details did not contain 'Transaction failed' was retried as a network error until maxAttempts, and a refund status had no branch, so both polled until the timeout and lost the real reason.
The refund branch rejected without setting the item's checkStatus, so the emitted state reported a refund while the item kept its previous status. The transaction and websocket paths set it before rejecting.
@erkancamli
erkancamli force-pushed the fix/sdk-signature-check-final-status branch from c92249d to b9b9cf9 Compare October 2, 2026 19:59
@erkancamli

Copy link
Copy Markdown
Contributor Author

@pedromcunha thanks for the approval. Is anything else needed from my side before this can be merged? Happy to rebase onto the latest main if that helps.

@erkancamli

Copy link
Copy Markdown
Contributor Author

@pedromcunha friendly follow-up: this still applies cleanly on current main, and nothing has touched these files since your approval. #1143 and #1145 are in the same state. Happy to rebase all three if that makes merging easier.

@pedromcunha-uneven
pedromcunha-uneven merged commit be8501c into relayprotocol:main Nov 7, 2026
1 of 2 checks passed
@pedromcunha-uneven

Copy link
Copy Markdown
Collaborator

LGTM, thanks

@@ -362,7 +368,12 @@ export async function handleSignatureStepItem({
client.log(['Transaction completed successfully'], LogLevel.Verbose)
return // Success - exit polling
} else if (res?.data?.status === 'failure') {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can we set stepItem.checkStatus = 'failure' here before throwing? The refund branch now does this so it matches the transaction and websocket paths, and the same reason applies to failure. Right now the item stays on pending (or undefined) after a failure. The assertion at signatureStep.test.ts:139 would change to 'failure'.

@@ -362,7 +368,12 @@ export async function handleSignatureStepItem({
client.log(['Transaction completed successfully'], LogLevel.Verbose)
return // Success - exit polling
} else if (res?.data?.status === 'failure') {

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

FYI: the websocket path doesn't treat failure as final. It waits 2s because the backend can go failure → pending → refund (websocketHandlers.ts:74-89). Polling now stops at the first failure, so that sequence ends with refunded: false. This isn't a regression (on main it timed out) and it matches transaction.ts, so fine to leave. It just means the description's "websocket handlers already treat both as final" isn't quite right.

@erkancamli erkancamli Nov 8, 2026 •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch, thanks. Opened #1175 for it: the failure branch now sets stepItem.checkStatus = 'failure' before throwing, with the tests updated.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants