Fix OTP verification error handling to properly propagate errors to UI #1463

devin-ai-integration · 2025-10-21T18:39:25Z

Description

Fixes a critical error handling bug where OTP verification failures in the non-custodial signer were not being displayed to users. When users entered an incorrect OTP or hit rate limits, the SDK would silently restart the auth flow without showing any error message, causing the UI to appear hung or stuck in a loop.

Root Cause:

verifyOtp() and sendMessageWithOtp() in ncs-signer.ts were only rejecting an internal promise but not throwing errors to the caller
The React UI provider's try-catch blocks couldn't capture these errors due to the async execution context

Changes:

Modified sendMessageWithOtp() to throw errors after rejecting the internal promise
Ensured verifyOtp() properly throws errors to callers
Updated React UI CrossmintWalletProvider handlers to re-throw errors so they reach BaseCodeInput for display
Added explicit Promise<void> return types for clarity

Impact: Users will now see appropriate error messages when OTP verification fails, instead of experiencing silent failures.

Test plan

⚠️ IMPORTANT - Manual testing required: Due to pre-existing build errors in the repository and Node version constraints, I was unable to run automated tests or build verification. The changes need thorough manual testing:

Critical to test:
- Enter incorrect OTP → should display error message "Invalid code. Please try again."
- Trigger rate limit by requesting OTP multiple times → should display error with rate limit message
- Enter correct OTP → should work as before
- Test both email and phone signer flows
- Verify React Native flows still work correctly
Error propagation chain to verify:
1. ncs-signer.ts throws error
2. CrossmintWalletProvider re-throws error
3. BaseCodeInput catches and displays error
Backward compatibility: Existing apps using the SDK should continue to work without changes

Package updates

The following packages are affected and will need changesets:

@crossmint/client-sdk-wallets - Core error handling fix
@crossmint/client-sdk-react-ui - UI error propagation

Need to run pnpm change:add to create appropriate changesets with:

Category: improvements, security
Product Area: wallets, security

Human Review Checklist

🔴 High Priority Items:

Test error display: Manually verify that entering wrong OTP shows error message in UI (currently untested)
Double rejection safety: Review ncs-signer.ts lines 209-211 and 245-246 - is it safe to both reject the promise AND throw the error?
React Native: Should packages/client/ui/react-native/src/providers/CrossmintWalletProvider.tsx also be updated? I assumed no changes needed since it uses context pattern, but verify this.
Async context: Verify the error actually propagates through the async call chain (ncs-signer → provider → BaseCodeInput)

⚠️ Known Limitations:

Could not build or run tests due to Node version mismatch (repo requires >=20.19.4)
Changes based on code analysis only, not runtime verification
No new automated tests added (TEE communication mocking is complex)

Session Info: https://app.devin.ai/sessions/d4fa7bab226846e9b736967f6438328e
Requested by: jorge@paella.dev (@jorge2393)

- Modified verifyOtp() to explicitly return Promise<void> and properly throw errors - Modified sendMessageWithOtp() to throw errors instead of only rejecting internal promise - Updated React UI CrossmintWalletProvider to re-throw errors so they reach BaseCodeInput - Errors now properly display to users when OTP verification fails or rate limits occur - Maintains backward compatibility with existing onAuthRequired callback signature Co-Authored-By: jorge@paella.dev <jorge@paella.dev>

devin-ai-integration · 2025-10-21T18:39:28Z

Original prompt from jorge@paella.dev

Problem Statement 
The OTP verification flow in the Crossmint SDK's non-custodial signer has a critical error handling bug where verifyOtp() and sendEmailWithOtp()/sendPhoneWithOtp() fail silently when errors occur.

Current Behavior:

When an incorrect OTP is provided, the TEE correctly rejects it with HTTP 401
The error is logged internally but never propagated to the calling application
The SDK automatically restarts the auth flow without informing the app
Users see no error feedback and the UI appears to hang or loop
Expected Behavior:

verifyOtp() should throw/reject when OTP verification fails
sendEmailWithOtp()/sendPhoneWithOtp() should throw/reject when OTP sending fails (e.g., rate limits)
Application code should be able to catch these errors and show appropriate user feedback
Root Cause 
The issue is in packages/wallets/src/signers/non-custodial/ncs-signer.ts:

In verifyOtp() (lines 220-269): ncs-signer.ts:220-269
Line 267 rejects the internal _authPromise
Line 268 throws the error
However, the throw happens in a context where the UI layer's try-catch doesn't capture it
The promise rejection is internal and doesn't propagate to the caller
In sendMessageWithOtp() (lines 183-211): ncs-signer.ts:183-211
Lines 206-209 reject the auth promise on error
But there's no throw statement, so the function appears to succeed
In UI providers: CrossmintWalletProvider.tsx:109-117
Try-catch blocks exist but don't catch the errors due to the async execution context issue
Required Changes 
1. Fix verifyOtp() Error Propagation 
In packages/wallets/src/signers/non-custodial/ncs-signer.ts, modify the verifyOtp() method to ensure errors are properly thrown to the caller:

The error should be thrown before or instead of just rejecting the internal promise
Consider making the method properly async and ensuring the thrown error propagates through the promise chain
The _authPromise?.reject() call should happen, but the error must also reach the caller
2. Fix sendMessageWithOtp() Err... (2235 chars truncated...)

changeset-bot · 2025-10-21T18:39:29Z

🦋 Changeset detected

Latest commit: 4f2f73e

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

devin-ai-integration · 2025-10-21T18:39:29Z

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

Disable automatic comment and CI monitoring

vercel · 2025-10-21T18:39:32Z

The latest updates on your projects. Learn more about Vercel for GitHub.

1 Skipped Deployment

Project	Deployment	Preview	Comments	Updated (UTC)
smart-wallet-auth-demo	Ignored			Oct 21, 2025 7:19pm

Co-Authored-By: jorge@paella.dev <jorge@paella.dev>

jcurbelo · 2025-10-21T19:08:00Z

packages/wallets/src/signers/non-custodial/ncs-signer.ts

    }

-    private async sendMessageWithOtp() {
+    private async sendMessageWithOtp(): Promise<void> {


let's remove the Promise<void> return type to stay consistent with the rest of the codebase style

jcurbelo · 2025-10-21T19:08:11Z

packages/wallets/src/signers/non-custodial/ncs-signer.ts

    }

-    private async verifyOtp(encryptedOtp: string) {
+    private async verifyOtp(encryptedOtp: string): Promise<void> {


same here: let's remove the Promise<void> return type to stay consistent with the rest of the codebase style

jcurbelo · 2025-10-21T19:10:10Z

packages/client/ui/react-ui/src/providers/CrossmintWalletProvider.tsx

            setEmailSignerDialogStep("otp");
        } catch (error) {
            console.error("Failed to send email OTP", error);
-            rejectRef.current(new Error("Failed to send email OTP"));
+            throw error;
        }
    };

    const emailsigners_handleOTPSubmit = async (otp: string) => {
        try {
            await verifyOtpRef.current(otp);
            setEmailSignerDialogOpen(false);
        } catch (error) {
            console.error("Failed to verify OTP", error);
-            rejectRef.current(new Error("Failed to verify OTP"));
+            throw error;
        }
    };

    // Phone authentication handlers (for TEE handshake)
    const phonesigners_handleSendPhoneOTP = async () => {
        try {
            await sendPhoneWithOtpRef.current();
            setPhoneSignerDialogStep("otp");
        } catch (error) {
            console.error("Failed to send phone OTP", error);
-            rejectRef.current(new Error("Failed to send phone OTP"));
+            throw error;
        }
    };

    const phonesigners_handleOTPSubmit = async (otp: string) => {
        try {
            await verifyPhoneOtpRef.current(otp);
            setPhoneSignerDialogOpen(false);
        } catch (error) {
            console.error("Failed to verify phone OTP", error);
-            rejectRef.current(new Error("Failed to verify phone OTP"));
+            throw error;
        }
    };



what's the difference between throwing here and calling rejectRef.current(new Error("Failed to send email OTP"))? does the latter effectively throw as well?

jcurbelo · 2025-10-21T19:12:07Z

packages/wallets/src/signers/non-custodial/ncs-signer.ts

            this._needsAuth = true;
-            this._authPromise?.reject(err as Error);
-            throw err;
+            const error = err as Error;


no need to create a new variable

devin-ai-integration · 2025-10-21T19:19:03Z

Good question! The key difference is in the error propagation chain:

Before (calling rejectRef.current() only):

Error occurs in ncs-signer.ts
CrossmintWalletProvider catches it and calls rejectRef.current()
This rejects the internal auth promise in ncs-signer.ts
BUT the error doesn't propagate to BaseCodeInput's try-catch
User sees no error message

After (throwing the error):

Error occurs in ncs-signer.ts
CrossmintWalletProvider catches it and re-throws
BaseCodeInput's try-catch (lines 178-186) catches the thrown error
BaseCodeInput sets error state and displays "Invalid code. Please try again."
User sees the error message

The rejectRef.current() is used to signal the auth flow that something went wrong, but it doesn't propagate the error up to the UI layer where BaseCodeInput can display it. We need both: reject the internal promise AND throw so the error reaches the UI component's error handler.

In response to: #1463 (comment)

…iable Co-Authored-By: jorge@paella.dev <jorge@paella.dev>

Add changeset for OTP error handling fix

37b1954

Co-Authored-By: jorge@paella.dev <jorge@paella.dev>

jcurbelo reviewed Oct 21, 2025

View reviewed changes

Address PR feedback: remove explicit return types and unnecessary var…

4f2f73e

…iable Co-Authored-By: jorge@paella.dev <jorge@paella.dev>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix OTP verification error handling to properly propagate errors to UI #1463

Fix OTP verification error handling to properly propagate errors to UI #1463

Uh oh!

devin-ai-integration bot commented Oct 21, 2025

Uh oh!

devin-ai-integration bot commented Oct 21, 2025

Uh oh!

changeset-bot bot commented Oct 21, 2025 •

edited

Loading

Uh oh!

devin-ai-integration bot commented Oct 21, 2025

Uh oh!

vercel bot commented Oct 21, 2025 •

edited

Loading

Uh oh!

jcurbelo Oct 21, 2025

Uh oh!

jcurbelo Oct 21, 2025

Uh oh!

jcurbelo Oct 21, 2025

Uh oh!

jcurbelo Oct 21, 2025

Uh oh!

devin-ai-integration bot commented Oct 21, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Fix OTP verification error handling to properly propagate errors to UI #1463

Are you sure you want to change the base?

Fix OTP verification error handling to properly propagate errors to UI #1463

Uh oh!

Conversation

devin-ai-integration bot commented Oct 21, 2025

Description

Test plan

Package updates

Human Review Checklist

Uh oh!

devin-ai-integration bot commented Oct 21, 2025

Uh oh!

changeset-bot bot commented Oct 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

🦋 Changeset detected

Uh oh!

devin-ai-integration bot commented Oct 21, 2025

🤖 Devin AI Engineer

Uh oh!

vercel bot commented Oct 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

jcurbelo Oct 21, 2025

Choose a reason for hiding this comment

Uh oh!

jcurbelo Oct 21, 2025

Choose a reason for hiding this comment

Uh oh!

jcurbelo Oct 21, 2025

Choose a reason for hiding this comment

Uh oh!

jcurbelo Oct 21, 2025

Choose a reason for hiding this comment

Uh oh!

devin-ai-integration bot commented Oct 21, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

changeset-bot bot commented Oct 21, 2025 •

edited

Loading

vercel bot commented Oct 21, 2025 •

edited

Loading