fix(broadcast): normalize web3.js polling-fallback rejection#304
Merged
Merged
Conversation
The diagnostic console.error added in PR #303 revealed that the prod broadcast endpoint catches a plain Object — not a TransactionFailedOnChainError or any Error subclass — when the tx confirms on-chain with a program error: [tx/broadcast] caught error: { isError: false, isTFOCE: false, constructorName: 'Object', name: undefined, message: undefined, stringified: '[object Object]' } Root cause is in @solana/web3.js (v1.98) Connection's `getTransactionConfirmationPromise`. Two code paths can settle the confirmation: - The WS-subscription `onSignature` callback resolves the promise with `{ __type: PROCESSED, response: { context, value: { err } } }` — so `confirmTransaction` then resolves with `{ value: { err } }` and our code reads `result.value.err`. - The `getSignatureStatus` polling fallback (fired in parallel) at line ~6601 of `node_modules/@solana/web3.js/lib/index.cjs.js` calls `reject(value.err)` directly with the bare `TransactionError` object when `value.err` is non-null. So `confirmTransaction` REJECTS with that bare plain object, and our `await` throws it as-is. When the polling-fallback path wins the race (slow WS subscription), the thrown value is a plain `{ InstructionError: [...] }` object, which fails every `instanceof` check in our route handler — including the new `TransactionFailedOnChainError` check — so we fall through to the generic `500 INTERNAL "unknown error"` envelope. This commit wraps `confirmTransaction` with a try/catch inside `confirmInspected` that normalizes a non-Error object rejection into a proper `TransactionFailedOnChainError(signature, err)`. Real Error subclasses (`TransactionExpiredBlockheightExceededError`, `SendTransactionError`, etc.) propagate unchanged so the route's existing 504/502 branches still fire correctly. Tests: - New test `normalizes polling-fallback rejection` mocks `confirmTransaction` to `Promise.reject(programErr)` and asserts the outer rejection is a `TransactionFailedOnChainError` with matching signature/err. - New test `preserves non-object rejection types` asserts the blockheight-expired path still surfaces as `TransactionExpiredBlockheightExceededError`. - Removed the diagnostic console.error from #303; it has served its purpose. 1652 agent / 10 sendWithRetry / 13 tx-broadcast tests pass. Typecheck clean across root + sdk + app + agent. Diag2 against live devnet confirms the local dist still throws `TransactionFailedOnChainError` correctly via the resolve path. Prod will be verified post-merge via the existing smoke script.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Diagnostic log from PR #303 revealed the real root cause of the prod 500 INTERNAL fallthrough that PR #302 was supposed to convert into 502 TX_FAILED_ON_CHAIN.
The thrown value is a plain Object, not a
TransactionFailedOnChainErroror anyErrorsubclass — so everyinstanceofcheck in the route fails and we fall through to500 INTERNAL "unknown error".Root cause
In
@solana/web3.jsv1.98 (Connection#getTransactionConfirmationPromise), two paths can settle the confirmation:onSignaturecallback) resolves the promise with{ __type: PROCESSED, response: { context, value: { err } } }, soconfirmTransactionresolves with{ value: { err } }and PR fix(broadcast): bail on confirmed-with-err to avoid CF 504 #302'sresult.value.errcheck fires correctly.getSignatureStatuspolling fallback (~line 6601 ofnode_modules/@solana/web3.js/lib/index.cjs.js) callsreject(value.err)— it rejects with the bareTransactionErrorobject (e.g.{ InstructionError: [0, { Custom: 3012 }] }), not a thrownError.When the polling fallback wins the race (slow WS subscription on Helius devnet),
confirmTransactionthrows the bare plain object. PR #302 only handled the resolve path. Hence: 500 INTERNAL.Fix
Wrap
confirmTransactioninconfirmInspectedwith a try/catch that normalizes any non-Error object rejection into a properTransactionFailedOnChainError(signature, err). RealErrorsubclasses (TransactionExpiredBlockheightExceededError,SendTransactionError) propagate unchanged.Also removes the diagnostic
console.errorfrom #303 (it served its purpose).Test plan
pnpm typecheckclean across root + sdk + app + agentpnpm vitest run tests/lib/sendWithRetry.test.ts tests/routes/tx-broadcast.test.ts— 23 passed (10 + 13)TransactionFailedOnChainErroris thrown correctly via the resolve path502 TX_FAILED_ON_CHAINenvelopeNew tests
normalizes polling-fallback rejection (bare TransactionError) into TransactionFailedOnChainError— mocksconfirmTransactiontoPromise.reject(programErr)and asserts outer rejection is the wrapped class with matching signature/err.preserves non-object rejection types (e.g. TransactionExpiredBlockheightExceededError) unchanged— guards against the normalization accidentally swallowing real Error instances.