feat: implement unified failure-state UX system for privacy and blockchain edge cases - #167
Open
Simultech369 wants to merge 1 commit into
Open
Conversation
✅ Deploy Preview for zicket ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
Contributor
📝 WalkthroughWalkthroughIntroduces a typed failure-state system with centralized error parsing, configurable failure metadata, reusable banner and modal components, application integrations for checkout and wallet errors, and an interactive demonstration page. ChangesFailure-state UX system
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant CheckoutOrWallet
participant useFailureState
participant FailureStateBanner
participant FailureStateModal
CheckoutOrWallet->>useFailureState: triggerFailure(error or code)
useFailureState->>useFailureState: resolve code and build failure details
useFailureState->>FailureStateBanner: render failureState
useFailureState->>FailureStateModal: open with failureState
FailureStateBanner->>CheckoutOrWallet: invoke retry
FailureStateModal->>useFailureState: invoke retry or diagnostics
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/components/explore/EventCheckout/TicketInfo.tsx`:
- Around line 104-118: The failure handlers in TicketInfo.tsx (the checkout
useEffect) and ConnectWalletPrompt.tsx (the wallet failure handler) overwrite
serialized source errors with static technicalDetails. Append each sanitized
payment, transaction, wallet, or wallet SDK error to its contextual diagnostic
string while preserving the existing context and avoiding raw unsanitized error
data.
- Around line 445-450: Use a shared close-and-retry callback that clears or
resets the failure state before invoking the retry action. In
app/components/explore/EventCheckout/TicketInfo.tsx lines 445-450, route the
failure banner’s onRetry through this reset-then-retry path; in lines 483-499,
reuse the same callback for both the wallet banner and FailureStateModal. In
app/components/organizer/ConnectWalletPrompt.tsx lines 91-108, clear/reset the
failure state before calling handleConnectWallet.
In `@app/demo/failure-state/page.tsx`:
- Around line 48-50: Update the triggerFailure call’s txHash fallback so
FailureStateModal receives a transaction hash only when sampleTxHash is a full
confirmed signature; remove the placeholder blockchain hash containing ellipses
and otherwise pass null.
In `@components/FailureStateModal.tsx`:
- Around line 120-126: Update the DOWNLOAD_DIAGNOSTICS branch in
FailureStateModal so it never falls back to handleCopyTechDetails; either
perform a local Blob-based diagnostics download when onDownloadDiagnostics is
absent or enforce that this action is only configured when the callback exists.
Preserve the existing callback behavior when onDownloadDiagnostics is provided.
In `@hooks/useFailureState.ts`:
- Around line 120-124: Track the delayed cleanup created by clearFailure in a
ref, clear any pending timeout at the start of triggerFailure, and replace the
ref with the new timeout when closing. Preserve the existing 200 ms delay and
modal state behavior while preventing an earlier dismissal from clearing a newly
triggered failure.
- Around line 14-17: Update the error-string serialization in useFailureState,
including the corresponding logic near the later failure-handling path, to use a
guarded JSON serializer that catches stringify failures and always returns a
string fallback when serialization yields undefined. Preserve direct handling of
string errors and message-bearing objects while ensuring cyclic,
BigInt-containing, and otherwise unsupported values cannot make the failure
handler throw.
In `@lib/failureStateConfigs.ts`:
- Around line 27-37: Update the BLOCKCHAIN_TIMEOUT configuration in
failureStateConfigs.ts by setting retryable to false, while preserving the
existing CONTINUE_BACKGROUND and DOWNLOAD_DIAGNOSTICS actions.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro Plus
Run ID: 8271f44d-4349-4697-85cc-8da2927ac7e8
📒 Files selected for processing (8)
app/components/explore/EventCheckout/TicketInfo.tsxapp/components/organizer/ConnectWalletPrompt.tsxapp/demo/failure-state/page.tsxcomponents/FailureStateBanner.tsxcomponents/FailureStateModal.tsxhooks/useFailureState.tslib/failureStateConfigs.tstypes/failureState.ts
Comment on lines
+104
to
+118
| useEffect(() => { | ||
| if (hasPaymentFailed && paymentError) { | ||
| triggerFailure(paymentError, { | ||
| customMessage: paymentError, | ||
| technicalDetails: `Checkout payment failed for event ${eventId}.`, | ||
| }); | ||
| } else if (txState.status === "failed" && txState.error) { | ||
| triggerFailure(txState.error, { | ||
| txHash: txState.txHash, | ||
| technicalDetails: `Transaction failed after ${txState.attempts} polling attempts.`, | ||
| }); | ||
| } else if (walletState.error) { | ||
| triggerFailure(walletState.error, { | ||
| technicalDetails: `Wallet connection error during checkout.`, | ||
| }); |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟡 Minor | ⚡ Quick win
Keep source errors in the shared diagnostic payload. Supplying static technicalDetails overrides useFailureState’s normal error serialization, so users and support lose the actual failure cause.
app/components/explore/EventCheckout/TicketInfo.tsx#L104-L118: append the sanitized payment, transaction, or wallet error to the contextual diagnostic string.app/components/organizer/ConnectWalletPrompt.tsx#L28-L34: append the sanitized wallet SDK error to the contextual diagnostic string.
📍 Affects 2 files
app/components/explore/EventCheckout/TicketInfo.tsx#L104-L118(this comment)app/components/organizer/ConnectWalletPrompt.tsx#L28-L34
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/components/explore/EventCheckout/TicketInfo.tsx` around lines 104 - 118,
The failure handlers in TicketInfo.tsx (the checkout useEffect) and
ConnectWalletPrompt.tsx (the wallet failure handler) overwrite serialized source
errors with static technicalDetails. Append each sanitized payment, transaction,
wallet, or wallet SDK error to its contextual diagnostic string while preserving
the existing context and avoiding raw unsanitized error data.
Comment on lines
+445
to
+450
| {/* Unified Failure State Banner for payment failure */} | ||
| {hasPaymentFailed && failureState && txState.status === "idle" && ( | ||
| <FailureStateBanner | ||
| failureState={failureState} | ||
| onRetry={handleRetry} | ||
| /> |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Use a shared close-and-retry path. FailureStateModal delegates RETRY without closing, while these callbacks only reset source state; retries therefore run beneath an active modal and retain stale failure state.
app/components/explore/EventCheckout/TicketInfo.tsx#L445-L450: wire banner retry through a failure-state reset before checkout retry.app/components/explore/EventCheckout/TicketInfo.tsx#L483-L499: use the same close-and-retry callback for the wallet banner and modal.app/components/organizer/ConnectWalletPrompt.tsx#L91-L108: clear/reset failure state before invokinghandleConnectWallet.
📍 Affects 2 files
app/components/explore/EventCheckout/TicketInfo.tsx#L445-L450(this comment)app/components/explore/EventCheckout/TicketInfo.tsx#L483-L499app/components/organizer/ConnectWalletPrompt.tsx#L91-L108
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/components/explore/EventCheckout/TicketInfo.tsx` around lines 445 - 450,
Use a shared close-and-retry callback that clears or resets the failure state
before invoking the retry action. In
app/components/explore/EventCheckout/TicketInfo.tsx lines 445-450, route the
failure banner’s onRetry through this reset-then-retry path; in lines 483-499,
reuse the same callback for both the wallet banner and FailureStateModal. In
app/components/organizer/ConnectWalletPrompt.tsx lines 91-108, clear/reset the
failure state before calling handleConnectWallet.
Comment on lines
+48
to
+50
| triggerFailure(code, { | ||
| txHash: sampleTxHash || (code.includes('BLOCKCHAIN') ? '5K3M...9ZpW' : null), | ||
| technicalDetails: mockTechDetails, |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Do not generate a dead Solscan URL.
The fallback hash contains ..., but FailureStateModal treats any non-empty value as a real transaction and renders an explorer link. Omit it unless the demo supplies a full, confirmed signature.
Proposed fix
- txHash: sampleTxHash || (code.includes('BLOCKCHAIN') ? '5K3M...9ZpW' : null),
+ txHash: sampleTxHash ?? null,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| triggerFailure(code, { | |
| txHash: sampleTxHash || (code.includes('BLOCKCHAIN') ? '5K3M...9ZpW' : null), | |
| technicalDetails: mockTechDetails, | |
| triggerFailure(code, { | |
| txHash: sampleTxHash ?? null, | |
| technicalDetails: mockTechDetails, |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/demo/failure-state/page.tsx` around lines 48 - 50, Update the
triggerFailure call’s txHash fallback so FailureStateModal receives a
transaction hash only when sampleTxHash is a full confirmed signature; remove
the placeholder blockchain hash containing ellipses and otherwise pass null.
Comment on lines
+120
to
+126
| case 'DOWNLOAD_DIAGNOSTICS': | ||
| if (onDownloadDiagnostics) { | ||
| onDownloadDiagnostics(); | ||
| } else { | ||
| handleCopyTechDetails(); | ||
| } | ||
| break; |
Contributor
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Keep “download diagnostics” behavior truthful.
Without onDownloadDiagnostics, this action copies text to the clipboard while the UI promises an export/download. Implement a local Blob download fallback, or require the callback whenever this action is configured.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@components/FailureStateModal.tsx` around lines 120 - 126, Update the
DOWNLOAD_DIAGNOSTICS branch in FailureStateModal so it never falls back to
handleCopyTechDetails; either perform a local Blob-based diagnostics download
when onDownloadDiagnostics is absent or enforce that this action is only
configured when the callback exists. Preserve the existing callback behavior
when onDownloadDiagnostics is provided.
Comment on lines
+14
to
+17
| const errString = typeof error === 'string' | ||
| ? error | ||
| : (error as { message?: string; code?: number | string })?.message || JSON.stringify(error); | ||
|
|
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Make unknown-error serialization non-throwing.
JSON.stringify throws for cyclic or BigInt-containing errors and can return undefined for values such as functions. That makes the failure handler throw while handling an error. Use a guarded serializer with a string fallback here and at Line 97.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@hooks/useFailureState.ts` around lines 14 - 17, Update the error-string
serialization in useFailureState, including the corresponding logic near the
later failure-handling path, to use a guarded JSON serializer that catches
stringify failures and always returns a string fallback when serialization
yields undefined. Preserve direct handling of string errors and message-bearing
objects while ensuring cyclic, BigInt-containing, and otherwise unsupported
values cannot make the failure handler throw.
Comment on lines
+120
to
+124
| const clearFailure = useCallback(() => { | ||
| setIsOpen(false); | ||
| // Slight delay before clearing object to prevent layout jump during modal close transition | ||
| setTimeout(() => setFailureState(null), 200); | ||
| }, []); |
Contributor
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Cancel a pending clear when a new failure is triggered.
A failure triggered within 200 ms of dismissal is set successfully, then the previous timeout clears it, leaving an open modal with no state. Store the timeout in a ref, clear it in triggerFailure, and replace it when closing.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@hooks/useFailureState.ts` around lines 120 - 124, Track the delayed cleanup
created by clearFailure in a ref, clear any pending timeout at the start of
triggerFailure, and replace the ref with the new timeout when closing. Preserve
the existing 200 ms delay and modal state behavior while preventing an earlier
dismissal from clearing a newly triggered failure.
Comment on lines
+27
to
+37
| BLOCKCHAIN_TIMEOUT: { | ||
| code: 'BLOCKCHAIN_TIMEOUT', | ||
| category: 'blockchain', | ||
| severity: 'error', | ||
| title: 'Blockchain Confirmation Timeout', | ||
| userMessage: 'The network is experiencing high congestion. Your transaction was broadcasted but is taking longer than expected to confirm.', | ||
| retryable: true, | ||
| defaultActions: [ | ||
| { type: 'CONTINUE_BACKGROUND', label: 'Check Status in Background', variant: 'gradient', primary: true }, | ||
| { type: 'DOWNLOAD_DIAGNOSTICS', label: 'Export Error Log', variant: 'outline' }, | ||
| ], |
Contributor
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Do not offer transaction retry after broadcast.
This state says the transaction was already broadcast, but retryable: true exposes the banner’s generic Retry action. A checkout handler could submit a duplicate transaction instead of polling its status. Set this to false; the existing background-status action is the safe recovery path.
Proposed fix
- retryable: true,
+ retryable: false,📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| BLOCKCHAIN_TIMEOUT: { | |
| code: 'BLOCKCHAIN_TIMEOUT', | |
| category: 'blockchain', | |
| severity: 'error', | |
| title: 'Blockchain Confirmation Timeout', | |
| userMessage: 'The network is experiencing high congestion. Your transaction was broadcasted but is taking longer than expected to confirm.', | |
| retryable: true, | |
| defaultActions: [ | |
| { type: 'CONTINUE_BACKGROUND', label: 'Check Status in Background', variant: 'gradient', primary: true }, | |
| { type: 'DOWNLOAD_DIAGNOSTICS', label: 'Export Error Log', variant: 'outline' }, | |
| ], | |
| BLOCKCHAIN_TIMEOUT: { | |
| code: 'BLOCKCHAIN_TIMEOUT', | |
| category: 'blockchain', | |
| severity: 'error', | |
| title: 'Blockchain Confirmation Timeout', | |
| userMessage: 'The network is experiencing high congestion. Your transaction was broadcasted but is taking longer than expected to confirm.', | |
| retryable: false, | |
| defaultActions: [ | |
| { type: 'CONTINUE_BACKGROUND', label: 'Check Status in Background', variant: 'gradient', primary: true }, | |
| { type: 'DOWNLOAD_DIAGNOSTICS', label: 'Export Error Log', variant: 'outline' }, | |
| ], |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@lib/failureStateConfigs.ts` around lines 27 - 37, Update the
BLOCKCHAIN_TIMEOUT configuration in failureStateConfigs.ts by setting retryable
to false, while preserving the existing CONTINUE_BACKGROUND and
DOWNLOAD_DIAGNOSTICS actions.
Author
|
I am cleaning up older open PRs on my account. If this is still reviewable or useful to the project, I am happy to keep it open and make changes. Otherwise, I will close it after a short grace period to reduce queue noise. |
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
1 participant
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.
Closes #164
Summary
Implemented a unified failure-state UX system for wallet, blockchain, privacy/ZK, network, and partial confirmation edge cases.
Changes
useFailureStatefor error classification, modal state, and diagnostic export.FailureStateModalwith Solscan links and expandable diagnostics.FailureStateBannerfor checkout, wallet, relayer, and confirmation failures.TicketInfoand organizer wallet connection flow./demo/failure-state.Verification
npm run buildpasses.Summary by CodeRabbit