fix: show rejection alert in case the user rejects the connection

[ignaciosantise]

This pull request refactors the retry logic in the ConnectingView component to improve error handling and code clarity. The main change is moving the QR code retry logic from the error catch block to after the error modal is displayed, ensuring that retries are only attempted after user feedback.

Error handling and retry logic improvements:

• Moved the QR code retry logic to execute after the error modal is displayed, rather than immediately upon catching an error, making the flow clearer and more user-friendly. [1] [2]
• Removed redundant calls to WcController.clearUri() and the previous retry logic from the catch block, streamlining error handling.

---

Note

Refactors ConnectingView to clear the URI before reconnect and run QR retries after showing errors, and bumps hono to 4.10.3.

• AppKit ConnectingView:
  • Clear wc URI via WcController.clearUri() before initiating reconnect when retrying or pairing expired.
  • Remove clearUri from onRetry and catch block; move QR retry to execute after error notification and event tracking.
  • Preserve timed QR auto-retry using isAllowedRetry with updated retry timestamp handling.
• Examples / Dependencies:
  • Add overrides entry for hono 4.10.3 in examples/expo-multichain/package.json and update lockfile.

^{Written by Cursor Bugbot for commit b73fdb4. This will update automatically on new commits. Configure here.}

[changeset-bot]

⚠️ No Changeset found

Latest commit: b73fdb4

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

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

[claude]

Claude finished @ignaciosantise's task —— View job

---

Code Review Complete

Reviewing PR #502 with focus on code quality, security, and best practices.

---

Issues Found

1. Breaking Change: Removed WcController.clearUri() on Error (packages/appkit/src/views/w3m-connecting-view/index.tsx:58)

Severity: High

The removal of WcController.clearUri() from the error handler could lead to stale state issues:

// Before (line 59):
WcController.setWcError(true);
WcController.clearUri();  // ❌ REMOVED

// After (line 58):
WcController.setWcError(true);

Problem:

• When an error occurs, the wcUri, wcPairingExpiry, wcPromise, and wcLinking remain in state
• If the user navigates away or cancels, stale connection data persists
• On subsequent connection attempts, the component might use expired or invalid URIs

Impact:

• For QR code flows: Auto-retry will eventually generate a new URI, potentially masking the issue
• For wallet flows (non-QR): No auto-retry occurs, leaving stale state indefinitely
• State inconsistency between wcError: true and non-cleared connection state

Recommendation:
Restore the WcController.clearUri() call on errors, or document why it's safe to remove. If the intent is to preserve the URI for retry, this should only apply to specific error types (e.g., network errors), not user rejections or proposal expirations.

---

2. Inconsistent Retry Behavior Between QR and Wallet Flows (packages/appkit/src/views/w3m-connecting-view/index.tsx:76-81)

Severity: Medium

The retry logic only applies to QR code flows:

if (isQr && CoreHelperUtil.isAllowedRetry(currentRetryTime)) {
  const newRetryTime = Date.now();
  setLastRetry(newRetryTime);
  initializeConnection(true, newRetryTime);
}

Problem:

• Wallet connection errors (non-QR) never retry automatically
• For user rejections on wallet flows, users see the error but must manually retry via onRetry
• The manual retry flow calls WcController.clearUri() (line 35), but automatic retry doesn't

Questions:

1. Is automatic retry intentionally disabled for wallet flows?
2. Should user rejections trigger auto-retry at all, or should they always require manual retry?
3. If auto-retry is intended only for network/timeout errors, shouldn't we filter by error type?

---

3. User Rejection Message Not Surfaced in Event Payload (packages/appkit/src/views/w3m-connecting-view/index.tsx:68-74)

Severity: Low

The snackbar shows specific rejection messages, but the analytics event only captures the raw error message:

SnackController.showError(
  isUserRejected ? 'User rejected the request' : 'Something went wrong'
);

EventsController.sendEvent({
  type: 'track',
  event: isUserRejected ? 'USER_REJECTED' : 'CONNECT_ERROR',
  properties: {
    message: (error as Error)?.message ?? 'Unknown'  // Raw error message
  }
});

Issue:

• Analytics receives the technical error message (e.g., "User rejected the connection request")
• User-facing snackbar shows a friendlier message
• This inconsistency could make debugging harder when correlating user reports with analytics

Recommendation:
Consider adding a userFacingMessage field to the event properties for consistency, or document that message should always contain the technical error.

---

4. Race Condition Risk: Retry Timestamp Management (packages/appkit/src/views/w3m-connecting-view/index.tsx:76-81)

Severity: Low

The retry logic manages timestamps across async boundaries:

const currentRetryTime = retryTimestamp ?? lastRetry;
if (isQr && CoreHelperUtil.isAllowedRetry(currentRetryTime)) {
  const newRetryTime = Date.now();
  setLastRetry(newRetryTime);
  initializeConnection(true, newRetryTime);  // ⚠️ async call
}

Potential Issues:

1. Multiple simultaneous errors could trigger concurrent retries
2. The setLastRetry state update is async, so rapid errors might bypass isAllowedRetry
3. If initializeConnection fails quickly, we could end up in a retry loop

Why it mostly works:

• The retryTimestamp parameter helps track individual retry chains
• The QR interval check (line 115) is separate from error retries

Recommendation:
Add a isRetrying flag to prevent concurrent retry attempts:

const [isRetrying, setIsRetrying] = useState(false);

// In error handler:
if (isQr && !isRetrying && CoreHelperUtil.isAllowedRetry(currentRetryTime)) {
  setIsRetrying(true);
  // ... retry logic ...
  // Clear flag after retry completes or fails
}

---

Missing Context

5. No Changeset Added

The changeset-bot flagged that this PR has no changeset. Since this affects error handling behavior (a user-visible change), a patch-level changeset should be added.

---

Positive Changes

✅ Improved UX: Moving retry after user notifications ensures users see error feedback before retries happen
✅ Better Error Categorization: Distinguishing USER_REJECTED vs CONNECT_ERROR in analytics
✅ Cleaner Flow: Removing early return makes the error handler easier to follow

---

Summary

The main concern is the removal of WcController.clearUri() without clear justification. This could lead to state inconsistency, particularly for non-QR wallet flows. The other issues are minor but worth considering for robustness.

Recommendation: Either restore WcController.clearUri() or provide test coverage/documentation explaining why it's safe to remove.

---

Tasks:

• Read repository guidelines (CLAUDE.md)
• Read the modified file
• Analyze error handling and retry logic changes
• Provide comprehensive review feedback

---

[Copilot]

Pull Request Overview

This PR refactors the retry logic in the ConnectingView component by moving the automatic QR code retry mechanism from executing immediately in the catch block to executing after error modal display and event tracking. The change aims to improve user experience by showing error feedback before attempting automatic retries.

Key changes:

• Moved QR code retry logic from the catch block (before error handling) to after the error modal display and event tracking
• Removed the WcController.clearUri() call that was previously in the catch block

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[socket-security]

Review the following changes in direct dependencies. Learn more about Socket for GitHub.

Diff	Package	Supply Chain Security	Vulnerability	Quality	Maintenance	License
	@​walletconnect/​react-native-compat@​2.21.5 ⏵ 2.22.2	+1			+1	-19
	@​reown/​appkit-bitcoin-react-native@​0.0.0-use.local ⏵ 2.0.1	-24		-28	-2
	@​react-navigation/​bottom-tabs@​7.4.6
	@​react-navigation/​elements@​2.6.3
	@​reown/​appkit-wagmi-react-native@​0.0.0-use.local ⏵ 2.0.1	-16		-26	-2
	@​react-navigation/​native@​7.1.17
	@​reown/​appkit-solana-react-native@​0.0.0-use.local ⏵ 2.0.1	-19		-24	-2
	expo-router@​6.0.10
	expo@​52.0.47 ⏵ 54.0.12			+1
	expo-application@​6.0.2 ⏵ 7.0.7	+1		+1	-1
	expo-system-ui@​6.0.7
	@​expo/​metro-runtime@​4.0.1 ⏵ 6.1.2
	expo-constants@​17.0.8 ⏵ 18.0.9	+1		+1	+2
	expo-linking@​8.0.8
	@​babel/​core@​7.26.10 ⏵ 7.28.3
	@​types/​react@​18.2.79 ⏵ 19.1.13	+1		+1	-1
	@​react-native-async-storage/​async-storage@​2.1.2 ⏵ 2.2.0
	expo-symbols@​1.0.7
	expo-status-bar@​2.0.1 ⏵ 3.0.8			+1
	expo-blur@​15.0.7
	expo-clipboard@​7.0.1 ⏵ 8.0.7	+1		+1
	expo-haptics@​15.0.7
	expo-font@​13.0.4 ⏵ 14.0.8			+1	+2
	expo-web-browser@​15.0.8
	eslint-config-expo@​10.0.0
	expo-build-properties@​1.0.9
	@​tanstack/​react-query@​5.56.2 ⏵ 5.85.5				+2
	@​expo/​vector-icons@​14.0.4 ⏵ 15.0.2
	expo-image@​3.0.8
	eslint@​8.57.1 ⏵ 9.34.0	+1			+1
	expo-splash-screen@​31.0.10
	expo-dev-client@​6.0.13
	@​reown/​appkit-react-native@​2.0.1

View full report

[ignaciosantise]

@claude review

[claude]

Claude finished @ignaciosantise's task —— View job

---

🔄 Incremental Review: Code Review Complete

Reviewing PR #502 with focus on code quality, security, and best practices.

---

Issues Found

1. Breaking Change: Removed WcController.clearUri() on Error (packages/appkit/src/views/w3m-connecting-view/index.tsx:58)

Severity: High

The removal of WcController.clearUri() from the error handler could lead to stale state issues:

// Before (line 59):
WcController.setWcError(true);
WcController.clearUri();  // ❌ REMOVED

// After (line 58):
WcController.setWcError(true);

Problem:

• When an error occurs, the wcUri, wcPairingExpiry, wcPromise, and wcLinking remain in state
• If the user navigates away or cancels, stale connection data persists
• On subsequent connection attempts, the component might use expired or invalid URIs

Impact:

• For QR code flows: Auto-retry will eventually generate a new URI, potentially masking the issue
• For wallet flows (non-QR): No auto-retry occurs, leaving stale state indefinitely
• State inconsistency between wcError: true and non-cleared connection state

Recommendation:
Restore the WcController.clearUri() call on errors, or document why it's safe to remove. If the intent is to preserve the URI for retry, this should only apply to specific error types (e.g., network errors), not user rejections or proposal expirations.

---

2. Inconsistent Retry Behavior Between QR and Wallet Flows (packages/appkit/src/views/w3m-connecting-view/index.tsx:76-81)

Severity: Medium

The retry logic only applies to QR code flows:

if (isQr && CoreHelperUtil.isAllowedRetry(currentRetryTime)) {
  const newRetryTime = Date.now();
  setLastRetry(newRetryTime);
  initializeConnection(true, newRetryTime);
}

Problem:

• Wallet connection errors (non-QR) never retry automatically
• For user rejections on wallet flows, users see the error but must manually retry via onRetry
• The manual retry flow calls WcController.clearUri() (line 35), but automatic retry doesn't

Questions:

1. Is automatic retry intentionally disabled for wallet flows?
2. Should user rejections trigger auto-retry at all, or should they always require manual retry?
3. If auto-retry is intended only for network/timeout errors, shouldn't we filter by error type?

---

3. User Rejection Message Not Surfaced in Event Payload (packages/appkit/src/views/w3m-connecting-view/index.tsx:68-74)

Severity: Low

The snackbar shows specific rejection messages, but the analytics event only captures the raw error message:

SnackController.showError(
  isUserRejected ? 'User rejected the request' : 'Something went wrong'
);

EventsController.sendEvent({
  type: 'track',
  event: isUserRejected ? 'USER_REJECTED' : 'CONNECT_ERROR',
  properties: {
    message: (error as Error)?.message ?? 'Unknown'  // Raw error message
  }
});

Issue:

• Analytics receives the technical error message (e.g., "User rejected the connection request")
• User-facing snackbar shows a friendlier message
• This inconsistency could make debugging harder when correlating user reports with analytics

Recommendation:
Consider adding a userFacingMessage field to the event properties for consistency, or document that message should always contain the technical error.

---

4. Race Condition Risk: Retry Timestamp Management (packages/appkit/src/views/w3m-connecting-view/index.tsx:76-81)

Severity: Low

The retry logic manages timestamps across async boundaries:

const currentRetryTime = retryTimestamp ?? lastRetry;
if (isQr && CoreHelperUtil.isAllowedRetry(currentRetryTime)) {
  const newRetryTime = Date.now();
  setLastRetry(newRetryTime);
  initializeConnection(true, newRetryTime);  // ⚠️ async call
}

Potential Issues:

1. Multiple simultaneous errors could trigger concurrent retries
2. The setLastRetry state update is async, so rapid errors might bypass isAllowedRetry
3. If initializeConnection fails quickly, we could end up in a retry loop

Why it mostly works:

• The retryTimestamp parameter helps track individual retry chains
• The QR interval check (line 115) is separate from error retries

Recommendation:
Add a isRetrying flag to prevent concurrent retry attempts:

const [isRetrying, setIsRetrying] = useState(false);

// In error handler:
if (isQr && !isRetrying && CoreHelperUtil.isAllowedRetry(currentRetryTime)) {
  setIsRetrying(true);
  // ... retry logic ...
  // Clear flag after retry completes or fails
}

---

Missing Context

5. No Changeset Added

The changeset-bot flagged that this PR has no changeset. Since this affects error handling behavior (a user-visible change), a patch-level changeset should be added.

---

Positive Changes

✅ Improved UX: Moving retry after user notifications ensures users see error feedback before retries happen
✅ Better Error Categorization: Distinguishing USER_REJECTED vs CONNECT_ERROR in analytics
✅ Cleaner Flow: Removing early return makes the error handler easier to follow

---

Summary

The main concern is the removal of WcController.clearUri() without clear justification. This could lead to state inconsistency, particularly for non-QR wallet flows. The other issues are minor but worth considering for robustness.

Recommendation: Either restore WcController.clearUri() or provide test coverage/documentation explaining why it's safe to remove.

---

Tasks:

• Read repository guidelines (CLAUDE.md)
• Read the modified file
• Analyze error handling and retry logic changes
• Provide comprehensive review feedback

---

View job