Prevent Unhandled Promise Rejections and Improve Type Safety #7070

JFixby · 2025-11-18T12:24:39Z

This PR addresses critical stability issues in WalletConnect v2.0 that cause application crashes due to unhandled promise rejections, type safety errors, and race conditions. The changes add comprehensive error handling, type guards, and graceful degradation for edge cases that occur in production environments.

Problems Addressed

1. Unhandled Promise Rejections

Multiple locations in the codebase had unhandled promise rejections that could crash Node.js applications:

Promise.all() calls without error handlers
Async operations without catch blocks
Event handlers that could throw unhandled errors
Promise rejections in timeout handlers

2. Type Safety Errors

Runtime errors when non-string values are passed to string methods:

TypeError: chain.split is not a function
TypeError: chainId.startsWith is not a function
TypeError: hex.startsWith is not a function

3. Race Conditions

Missing proposals/sessions causing crashes when:

Proposals expire before responses arrive
Sessions are deleted before updates are processed
Cleanup operations race with pending requests

4. Error Logging Issues

Using error() for expected race conditions (expired proposals/sessions)
Logging full error objects with stack traces for expected scenarios
Missing error context in logs

Solution

1. Comprehensive Promise Error Handling

Added .catch() handlers to all Promise.all() calls and async operations to prevent unhandled rejections:

Request handling in engine.ts
Ping operations
Authentication flows
Cleanup operations
Deeplink handling
Session/proposal deletion

2. Type Safety Guards

Added explicit type checking and conversion before calling string methods:

Type guards that cannot be optimized away by bundlers
Explicit String() conversions for non-string values
Null/undefined checks before method calls
Fallback handling for malformed CAIP identifiers

3. Race Condition Handling

Graceful handling of missing proposals/sessions:

Try-catch blocks around proposal.get() and session.get() calls
Warning logs instead of errors for expected race conditions
Early returns when resources are missing (expired/deleted)
Validation error handling for missing sessions

4. Improved Logging

Use warn() instead of error() for expected race conditions
Log error messages instead of full error objects to reduce noise
Better error context in log messages

Detailed Changes

`packages/sign-client/src/controllers/engine.ts` (282 lines changed)

Error Handling Improvements:

Added try-catch around processPendingMessageEvents() initialization
Wrapped getTVFParams() in try-catch to handle synchronous errors (e.g., startsWith errors)
Added error handlers to all Promise.all() calls:
- Request handling (session requests)
- Ping operations
- Authentication flows
- Cleanup operations (deleteSession, deleteProposal, deletePendingSessionRequest, deletePendingAuthRequest)
Added promise rejection handlers for deeplink operations
Wrapped onProviderMessageEvent() in error handler

Race Condition Fixes:

onSessionProposeResponse(): Try-catch around proposal.get() with warning for missing proposals
onSessionSettleRequest(): Try-catch around proposal.get() with warning for missing proposals
onSessionUpdateRequest(): Graceful handling of missing sessions with warning
onSessionConnect(): Changed from throwing error to logging warning when no listeners

Error Logging:

onRelayMessage(): Log error message instead of full error object
Better error context in all error handlers

Promise Rejection Prevention:

All async operations have proper error handling

`packages/core/src/controllers/store.ts` (9 lines changed)

Logging Improvements:

Use warn() instead of error() for proposals and sessions
Missing proposals/sessions are expected in race conditions (expired, rejected, cleaned up)
Error logging remains for other store types

`packages/utils/src/misc.ts` (55 lines changed)

Promise Rejection Handling:

Enhanced createDelayedPromise() timeout handler with comprehensive error handling
Added try-catch in timeout handler to prevent unhandled rejections
Proper cleanup of timeout and reject handlers
Attached catch handler to returned promise to prevent unhandled rejections
Better error messages with fallback handling

`packages/utils/src/caip.ts` (4 lines changed)

Type Safety:

Added type check before calling split() on chain parameter
Ensures chain is converted to string before parsing

`providers/universal-provider/src/UniversalProvider.ts` (25 lines changed)

Type Safety:

Added type conversion for chainId before constructing CAIP chain identifier
Prevents errors when chainId is passed as a number

Error Handling:

Added catch handler for request errors to prevent unhandled promise rejections
Re-throws error so caller can handle it properly

Provider Safety:

Added checks for setDefaultChain() availability before calling
Handles cases where provider may be undefined during cleanup or race conditions
Warning logs instead of crashes when provider is not available

`providers/universal-provider/src/providers/eip155.ts` (23 lines changed)

Type Safety:

Enhanced getDefaultChain() with type safety checks
Handles cases where defaultChain or chainId may not be strings
Added fallback for chainId values without colon separator
Type check in handleSwitchChain() before calling startsWith()

`providers/universal-provider/src/utils/caip25.ts` (42 lines changed)

Type Safety:

Improved type guards in hexToDecimal() and decimalToHex() functions
Added explicit type checking that cannot be optimized away by bundlers
Better handling of null/undefined values
Type checks before calling hexToDecimal() and decimalToHex() in extractCapabilitiesFromSession()

Files Changed

packages/sign-client/src/controllers/engine.ts (282 lines changed - major refactoring)
packages/core/src/controllers/store.ts (9 lines changed)
packages/utils/src/misc.ts (55 lines changed)
packages/utils/src/caip.ts (4 lines changed)
providers/universal-provider/src/UniversalProvider.ts (25 lines changed)
providers/universal-provider/src/providers/eip155.ts (23 lines changed)
providers/universal-provider/src/utils/caip25.ts (42 lines changed)
package-lock.json (64 lines removed - dependency cleanup)

Total: 8 files changed, 346 insertions(+), 158 deletions(-)

Testing

These changes have been tested in a production environment with:

Various chainId formats (strings, numbers, hex values)
Edge cases with null/undefined values
Real-world wallet connection scenarios
Race conditions (expired proposals, deleted sessions)
Network failures and timeout scenarios
Concurrent request handling
Application shutdown and cleanup scenarios

Impact

Stability: Prevents application crashes from unhandled promise rejections
Reliability: Gracefully handles race conditions and edge cases
Backward Compatible: All changes maintain backward compatibility
No Breaking Changes: Existing code continues to work as before
Better Observability: Improved logging with appropriate log levels
Type Safety: Prevents runtime errors from type mismatches

Related Issues

This PR addresses:

Unhandled promise rejections causing Node.js application crashes
Type safety issues when chainId values come in different formats
Race conditions in session/proposal lifecycle management
Error logging noise from expected race conditions

Fix critical stability issues that cause application crashes: - Add comprehensive error handling for all Promise.all() calls - Prevent unhandled promise rejections in async operations - Add type safety guards for chainId and CAIP parsing - Handle race conditions gracefully (expired proposals/sessions) - Improve error logging (warn vs error for expected conditions) Changes: - engine.ts: Add error handlers to all Promise.all() calls and async ops - engine.ts: Handle missing proposals/sessions in race conditions - engine.ts: Fix onSessionConnect to warn instead of throw - engine.ts: Log error messages instead of full error objects - store.ts: Use warn() for proposals/sessions instead of error() - misc.ts: Enhance createDelayedPromise with proper error handling - caip.ts: Add type check before split() on chain parameter - UniversalProvider.ts: Add type conversion and error handling - eip155.ts: Add type safety checks in getDefaultChain() - caip25.ts: Improve type guards in hexToDecimal/decimalToHex Fixes: - TypeError: chainId.startsWith is not a function - TypeError: chain.split is not a function - Unhandled promise rejections causing crashes - Race conditions with expired proposals/sessions

github-actions · 2025-11-18T12:24:51Z

All contributors have signed the CTA ✍️ ✅
_{Posted by the CLA Assistant Lite bot.}

JFixby · 2025-11-18T12:25:14Z

I have read the CTA Document and I hereby sign the CTA

Fix unhandled promise rejections and crashes caused by race conditions in WalletConnect message processing. These changes prevent crashes by gracefully handling expected edge cases. Changes: 1. crypto.decode() - Handle missing symmetric keys gracefully - Wrap generateSharedKey() in try-catch (was outside, causing crashes) - Return undefined on decode failure instead of throwing - Change error logging to warn (expected race condition) 2. history.resolve() - Handle missing history records gracefully - Wrap getRecord() in try-catch to prevent crashes - Log warnings instead of errors for missing records - Continue processing other messages when record not found 3. engine.onRelayMessage() - Handle undefined payload from decode - Check for undefined payload after decode - Early return with warning if decode fails - Prevents downstream crashes from undefined values 4. engine.onRelayEventResponse() - Handle missing history records - Wrap history.get() in try-catch - Early return with warning if record not found - Prevents crashes when response arrives before request recorded These fixes address stack traces showing: - "No matching key. history: <id>" errors - "onRelayMessage() -> failed to process inbound message" errors - Unhandled promise rejections causing crashes All changes maintain backward compatibility and use appropriate log levels (warn for expected race conditions, error for unexpected).

…d errors Add comprehensive error handling to prevent crashes caused by unhandled promise rejections and missing error handlers in sendResult() and sendError() methods. These changes prevent crashes when history records are missing or when async operations fail. Changes: 1. sendError() - Add catch handler for relayer.publish() promise - Added .catch() handler to relayer.publish() call (line 1926) - Prevents unhandled promise rejection when publish fails - Logs error with proper error message formatting 2. sendError() - Add try-catch for history.resolve() - Wrapped history.resolve() in try-catch block (line 1923-1927) - Prevents crashes when history record is missing during resolve - Logs warning instead of error (expected race condition) 3. sendResult() - Add try-catch for history.resolve() - Wrapped history.resolve() in try-catch block (line 1884-1888) - Prevents crashes when history record is missing during resolve - Logs warning instead of error (expected race condition) 4. sendResult() - Handle missing history records gracefully - Changed history.get() error handling to set record = null instead of throwing (line 1854-1859) - Added null check when accessing record.request.method (line 1864) - Uses default method "wc_sessionRequest" if record is missing - Prevents crashes when history record is cleaned up before response is sent - Changed error logging to warn level (expected race condition) 5. sendError() - Handle missing history records gracefully - Changed history.get() error handling to set record = null instead of throwing (line 1910-1916) - Added null check when accessing record.request.method (line 1922) - Uses default method "wc_sessionRequest" if record is missing - Prevents crashes when history record is cleaned up before error is sent - Changed error logging to warn level (expected race condition) 6. sendResult() - Add null check for record when getting TVF params - Added if (record) check before accessing record.request (line 1846) - Prevents crashes when record is null after history.get() fails These fixes address crashes caused by: - Unhandled promise rejections from relayer.publish() failures - Missing history records when resolving responses/errors - Race conditions where history records are cleaned up before use - Null pointer exceptions when accessing record properties All changes maintain backward compatibility and use appropriate log levels (warn for expected race conditions, error for unexpected failures).

## Changes ### 1. Removed error logging before re-throw (15 locations) Removed `logger.error()` calls before `throw error` in all catch blocks where errors are re-thrown, since the caller will log them with full context. **Files changed:** - `packages/sign-client/src/controllers/engine.ts` (14 locations) - `providers/universal-provider/src/UniversalProvider.ts` (1 location) **Locations:** - `connect() -> pairing.get()` error - `pair()` error - `approve() -> proposal.get()` error - `approve() -> isValidApprove()` error - `approve()` publish error - `reject() -> isValidReject()` error - `reject() -> proposal.get()` error - `update() -> isValidUpdate()` error - `extend() -> isValidExtend()` error - `request() -> isValidRequest()` error - `request()` Promise.all error - `ping() -> isValidPing()` error - `sendRequest() -> core.crypto.encode()` error - `sendError() -> core.crypto.encode()` error - `universal-provider request()` error ### 2. Enhanced Promise.all error context Added detailed context to Promise.all errors to identify which promise failed and what operation was being performed. **Changes in `engine.ts:request()` method:** - Wrapped `sendRequest` errors with context: `SendRequest failed for method ${method}, chainId ${chainId}: ${errorMessage}` - Enhanced Promise.all catch to include: `Promise.all failed in request - method: ${method}, chainId: ${chainId}, error: ${errorMessage}` - Preserved original error stack trace **Example error messages:** - Before: Generic error from Promise.all - After: `Promise.all failed in request - method: eth_sendTransaction, chainId: eip155:137, error: SendRequest failed for method eth_sendTransaction, chainId eip155:137: Request expired` ## Rationale Errors that are re-thrown will be caught and logged by the caller with full context. Logging before re-throw creates duplicate logs without adding value. Removing intermediate logging eliminates duplication while preserving error propagation. Enhanced error context in Promise.all makes it immediately clear which operation failed (sendRequest vs timeout) and what was being attempted (method and chainId), significantly improving debugging experience. ## Impact - Eliminates duplicate error logs when errors are re-thrown - Improves error messages with method and chainId context - Makes debugging easier by clearly identifying failed operations - No functional changes - error handling behavior unchanged

… unhandled crashes - Add serializeError() method to properly serialize errors of various types (Error instances, objects, primitives) with comprehensive detail capture - Enhance error handling in sendRequest catch block to preserve full error context including type, keys, own properties, and stringified representation - Improve Promise.all error handling with detailed error information capture - Handle empty error objects from wallet responses by providing default error structure with meaningful message and error code - Preserve original error stack traces while adding contextual information This prevents unhandled promise rejections and improves debugging by capturing complete error information before it might be lost.

JFixby force-pushed the v2.0-fix-unhadled-crashes branch from c3afc8a to 84d3c7d Compare November 18, 2025 18:57

JFixby added 3 commits November 18, 2025 20:06

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Prevent Unhandled Promise Rejections and Improve Type Safety #7070

Prevent Unhandled Promise Rejections and Improve Type Safety #7070

Uh oh!

JFixby commented Nov 18, 2025

Uh oh!

github-actions bot commented Nov 18, 2025 •

edited

Loading

Uh oh!

JFixby commented Nov 18, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Prevent Unhandled Promise Rejections and Improve Type Safety #7070

Are you sure you want to change the base?

Prevent Unhandled Promise Rejections and Improve Type Safety #7070

Uh oh!

Conversation

JFixby commented Nov 18, 2025

Problems Addressed

1. Unhandled Promise Rejections

2. Type Safety Errors

3. Race Conditions

4. Error Logging Issues

Solution

1. Comprehensive Promise Error Handling

2. Type Safety Guards

3. Race Condition Handling

4. Improved Logging

Detailed Changes

packages/sign-client/src/controllers/engine.ts (282 lines changed)

packages/core/src/controllers/store.ts (9 lines changed)

packages/utils/src/misc.ts (55 lines changed)

packages/utils/src/caip.ts (4 lines changed)

providers/universal-provider/src/UniversalProvider.ts (25 lines changed)

providers/universal-provider/src/providers/eip155.ts (23 lines changed)

providers/universal-provider/src/utils/caip25.ts (42 lines changed)

Files Changed

Testing

Impact

Related Issues

Uh oh!

github-actions bot commented Nov 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JFixby commented Nov 18, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

`packages/sign-client/src/controllers/engine.ts` (282 lines changed)

`packages/core/src/controllers/store.ts` (9 lines changed)

`packages/utils/src/misc.ts` (55 lines changed)

`packages/utils/src/caip.ts` (4 lines changed)

`providers/universal-provider/src/UniversalProvider.ts` (25 lines changed)

`providers/universal-provider/src/providers/eip155.ts` (23 lines changed)

`providers/universal-provider/src/utils/caip25.ts` (42 lines changed)

github-actions bot commented Nov 18, 2025 •

edited

Loading