Skip to content

test(account-abstraction): Add comprehensive error handling tests#160

Open
martinvibes wants to merge 1 commit intoancore-org:mainfrom
martinvibes:typed_errors
Open

test(account-abstraction): Add comprehensive error handling tests#160
martinvibes wants to merge 1 commit intoancore-org:mainfrom
martinvibes:typed_errors

Conversation

@martinvibes
Copy link
Copy Markdown

@martinvibes martinvibes commented Mar 26, 2026

  • Add unit tests for all typed error classes (AccountContractError, AlreadyInitializedError, NotInitializedError, InvalidNonceError, UnauthorizedError, SessionKeyNotFoundError, SessionKeyExpiredError, InsufficientPermissionError, ContractInvocationError)
  • Add tests for CONTRACT_ERROR_MESSAGES constant validation
  • Add tests for CONTRACT_ERROR_CODES mapping and error instantiation
  • Add tests for mapContractError function with various contract error scenarios
  • Export error classes and utilities from main index file for public API access
  • Ensure proper error inheritance chain and code assignment across all error types

closes #114

Summary by CodeRabbit

Release Notes

  • New Features

    • Added SessionKeyExpiredError exception to handle session key expiration scenarios.
    • Added InsufficientPermissionError exception to handle permission denial scenarios.
    • Enhanced error detection with improved support for numeric and text-based contract error identification.
    • Exported CONTRACT_ERROR_CODES for direct programmatic contract error code mapping and resolution.
  • Tests

    • Added comprehensive test suite for error classes and contract error mapping behavior.

- Add unit tests for all typed error classes (AccountContractError, AlreadyInitializedError, NotInitializedError, InvalidNonceError, UnauthorizedError, SessionKeyNotFoundError, SessionKeyExpiredError, InsufficientPermissionError, ContractInvocationError)
- Add tests for CONTRACT_ERROR_MESSAGES constant validation
- Add tests for CONTRACT_ERROR_CODES mapping and error instantiation
- Add tests for mapContractError function with various contract error scenarios
- Export error classes and utilities from main index file for public API access
- Ensure proper error inheritance chain and code assignment across all error types
@drips-wave
Copy link
Copy Markdown

drips-wave bot commented Mar 26, 2026

@martinvibes Great news! 🎉 Based on an automated assessment of this PR, the linked Wave issue(s) no longer count against your application limits.

You can now already apply to more issues while waiting for a review of this PR. Keep up the great work! 🚀

Learn more about application limits

@coderabbitai
Copy link
Copy Markdown

coderabbitai bot commented Mar 26, 2026

📝 Walkthrough

Walkthrough

Two new error classes (SessionKeyExpiredError and InsufficientPermissionError) are added alongside a numeric error-code mapping structure. The mapContractError function is enhanced to detect and map both numeric Soroban contract codes and string-based error patterns to corresponding typed errors. Comprehensive test coverage validates error class behavior and mapping logic.

Changes

Cohort / File(s) Summary
Error Class Additions
packages/account-abstraction/src/errors.ts
Added SessionKeyExpiredError and InsufficientPermissionError subclasses; introduced CONTRACT_ERROR_CODES numeric mapping record; extended CONTRACT_ERROR_MESSAGES; enhanced mapContractError to detect numeric codes via regex and apply string-pattern matching.
Error Type Exports
packages/account-abstraction/src/index.ts
Re-exported new error types and added CONTRACT_ERROR_CODES to public API alongside existing CONTRACT_ERROR_MESSAGES.
Error Mapping Test Suite
packages/account-abstraction/src/__tests__/errors.test.ts
New comprehensive Jest test suite validating error class constructors, message/code defaults, inheritance chain, exported constants, numeric code detection with regex variations, string-pattern matching with variants, and precedence behavior.

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Possibly related PRs

Poem

🐰 Contract errors hop in from afar,
Typed classes map them, near and far—
Session keys expire, permissions gate,
Now tests ensure they're handled straight!

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The PR title accurately describes the primary change: adding comprehensive error handling tests for the account-abstraction package.
Linked Issues check ✅ Passed The PR fulfills all coding requirements from issue #114: typed error classes are implemented, error mapping is implemented, and comprehensive unit tests are added covering all error types and mapping scenarios.
Out of Scope Changes check ✅ Passed All changes are directly aligned with issue #114 requirements. The PR adds typed error classes, implements error mapping logic, exports public APIs, and provides comprehensive test coverage—no out-of-scope modifications detected.
Docstring Coverage ✅ Passed Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Comment @coderabbitai help to get the list of available commands and usage tips.

Copy link
Copy Markdown

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (1)
packages/account-abstraction/src/errors.ts (1)

119-125: ⚠️ Potential issue | 🟠 Major

Add string-path mapping for SessionKeyNotFoundError to keep typed mapping complete.

SessionKeyNotFoundError exists, but mapContractError does not map "Session key not found" messages when no numeric code is present (Line 155 onward). That path currently degrades to ContractInvocationError.

🔧 Proposed fix
 export const CONTRACT_ERROR_MESSAGES = {
   ALREADY_INITIALIZED: 'Already initialized',
   NOT_INITIALIZED: 'Not initialized',
   INVALID_NONCE: 'Invalid nonce',
+  SESSION_KEY_NOT_FOUND: 'Session key not found',
   SESSION_KEY_EXPIRED: 'Session key expired',
   INSUFFICIENT_PERMISSION: 'Insufficient permission',
 } as const;
@@
 export function mapContractError(message: string, raw?: unknown): AccountContractError {
@@
+  if (
+    message.toLowerCase().includes('session key not found') ||
+    message.toLowerCase().includes('sessionkeynotfound')
+  ) {
+    return new SessionKeyNotFoundError();
+  }
   if (
     message.toLowerCase().includes('session key expired') ||
     message.toLowerCase().includes('sessionkeyexpired')
   ) {

Also applies to: 155-176

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@packages/account-abstraction/src/errors.ts` around lines 119 - 125, The
mapping for the string error path "Session key not found" is missing, so add a
CONTRACT_ERROR_MESSAGES entry (e.g. SESSION_KEY_NOT_FOUND: 'Session key not
found') and update mapContractError to handle that string path by
returning/throwing the SessionKeyNotFoundError instead of falling back to
ContractInvocationError; locate CONTRACT_ERROR_MESSAGES and the mapContractError
function to make these coordinated changes so the typed mapping includes the
session-key-not-found branch.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Outside diff comments:
In `@packages/account-abstraction/src/errors.ts`:
- Around line 119-125: The mapping for the string error path "Session key not
found" is missing, so add a CONTRACT_ERROR_MESSAGES entry (e.g.
SESSION_KEY_NOT_FOUND: 'Session key not found') and update mapContractError to
handle that string path by returning/throwing the SessionKeyNotFoundError
instead of falling back to ContractInvocationError; locate
CONTRACT_ERROR_MESSAGES and the mapContractError function to make these
coordinated changes so the typed mapping includes the session-key-not-found
branch.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: 7853ad57-b58f-4d42-8535-8a2e93774d88

📥 Commits

Reviewing files that changed from the base of the PR and between 6bf7d94 and ffc657c.

📒 Files selected for processing (3)
  • packages/account-abstraction/src/__tests__/errors.test.ts
  • packages/account-abstraction/src/errors.ts
  • packages/account-abstraction/src/index.ts

@wheval
Copy link
Copy Markdown
Contributor

wheval commented Mar 31, 2026

@martinvibes please resolve conflicts

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Account Abstraction Errors Mapping (typed errors)

2 participants