Skip to content

feat: implement neverthrow wrapper for orchestrator client#333

Draft
zeroknots wants to merge 1 commit intomainfrom
feat/error-handling
Draft

feat: implement neverthrow wrapper for orchestrator client#333
zeroknots wants to merge 1 commit intomainfrom
feat/error-handling

Conversation

@zeroknots
Copy link
Member

Result-based Error Handling for Orchestrator Client

Motivation

The current Orchestrator client uses traditional throw-based error handling. While familiar, this pattern has drawbacks:

  1. Errors are invisible in function signatures - Callers don't know what errors a function can throw without reading implementation details
  2. Easy to forget error handling - Nothing forces developers to handle errors at compile time
  3. Try/catch blocks are verbose - Error handling code gets scattered and nested
  4. No composability - Can't easily chain operations that might fail

This PR introduces a Rust-inspired Result pattern using neverthrow, which makes error handling:

  • Explicit - Errors are part of the return type
  • Exhaustive - TypeScript ensures all error cases are handled
  • Composable - Chain operations with .map(), .mapErr(), .andThen()

Changes

New Dependencies

  • neverthrow - Lightweight Result type library

New Exports

Export Type Description
SafeOrchestrator Class Result-returning wrapper around Orchestrator
getSafeOrchestrator() Function Factory function (mirrors getOrchestrator())
OrchestratorResult<T> Type ResultAsync<T, OrchestratorError>
ErrorCategory Const Category enum: Auth, Balance, Validation, Server, RateLimit, Unknown
CategorizedError Type Discriminated union for exhaustive matching
categorizeError() Function Converts error to CategorizedError

Backwards Compatibility

The existing Orchestrator class is unchanged. All current code continues to work.

Usage

Basic Usage

import {
  getSafeOrchestrator,
  type OrchestratorResult
} from '@rhinestone/sdk/dist/src/orchestrator'

const client = getSafeOrchestrator(apiKey)

const result = await client.getPortfolio(address)

if (result.isErr()) {
  console.error(result.error.message)
  return
}

// TypeScript knows result.value is Portfolio
console.log(result.value)

Pattern Matching with .match()

const message = await client.getPortfolio(address).match(
  (portfolio) => `Found ${portfolio.length} tokens`,
  (error) => `Error: ${error.message}`
)

Chaining Operations

const ethBalance = await client
  .getPortfolio(address)
  .map((portfolio) => portfolio.find((t) => t.symbol === 'ETH'))
  .map((eth) => eth?.balances.unlocked ?? 0n)
  .mapErr((error) => {
    logger.error('Portfolio fetch failed', { error, traceId: error.traceId })
    return error
  })

Exhaustive Error Handling

import {
  getSafeOrchestrator,
  categorizeError,
  ErrorCategory
} from '@rhinestone/sdk/dist/src/orchestrator'

const client = getSafeOrchestrator(apiKey)
const result = await client.getPortfolio(address)

if (result.isErr()) {
  const categorized = categorizeError(result.error)

  switch (categorized.category) {
    case ErrorCategory.Auth:
      // Handle auth errors (invalid API key, unauthorized, forbidden)
      showLoginPrompt()
      break

    case ErrorCategory.Balance:
      // Handle balance errors (insufficient balance, insufficient liquidity)
      if (categorized.error instanceof InsufficientLiquidityError) {
        showPartialFillOptions(categorized.error.availableIntents)
      } else {
        showInsufficientFundsMessage()
      }
      break

    case ErrorCategory.Validation:
      // Handle validation errors (bad request, unsupported chain/token, no path)
      showValidationError(categorized.error.message)
      break

    case ErrorCategory.Server:
      // Handle server errors (500, 503)
      showRetryMessage()
      break

    case ErrorCategory.RateLimit:
      // Handle rate limiting with retry-after
      const retryAfter = categorized.retryAfter ?? '60'
      scheduleRetry(parseInt(retryAfter) * 1000)
      break

    case ErrorCategory.Unknown:
      // Handle unexpected errors
      reportToSentry(categorized.error)
      break

    default:
      // TypeScript error if a case is missed
      assertNever(categorized)
  }
}

function assertNever(x: never): never {
  throw new Error(`Unhandled case: ${x}`)
}

Available Methods

All Orchestrator methods have SafeOrchestrator equivalents:

// Throws
const orchestrator = getOrchestrator(apiKey)
const portfolio = await orchestrator.getPortfolio(address)        // throws on error

// Returns Result
const safeOrchestrator = getSafeOrchestrator(apiKey)
const result = await safeOrchestrator.getPortfolio(address)       // returns Result

// Methods available:
safeOrchestrator.getPortfolio(address, filter?)      // OrchestratorResult<Portfolio>
safeOrchestrator.getIntentRoute(input)               // OrchestratorResult<IntentRoute>
safeOrchestrator.splitIntents(input)                 // OrchestratorResult<SplitIntentsResult>
safeOrchestrator.submitIntent(signedOp, dryRun)      // OrchestratorResult<IntentResult>
safeOrchestrator.getIntentOpStatus(intentId)         // OrchestratorResult<IntentOpStatus>

Error Categories

Category Errors When
Auth AuthenticationRequiredError, InvalidApiKeyError, ForbiddenError Missing/invalid API key, insufficient permissions
Balance InsufficientBalanceError, InsufficientLiquidityError User has no funds or not enough liquidity
Validation BadRequestError, UnsupportedChainError, UnsupportedTokenError, NoPathFoundError, etc. Invalid input, unsupported chains/tokens
Server InternalServerError, ServiceUnavailableError 500/503 errors, retry recommended
RateLimit RateLimitedError 429 error, includes retryAfter hint
Unknown OrchestratorError Catch-all for unrecognized errors

Testing

bun run test -- src/orchestrator/safeClient.test.ts

@zeroknots
Copy link
Member Author

@AmanRaj1608 @Destiner the categorization needs work. this should be a good first step tho

*/
export type CategorizedError =
| { category: typeof ErrorCategory.Auth; error: AuthenticationRequiredError | InvalidApiKeyError | ForbiddenError }
| { category: typeof ErrorCategory.Balance; error: InsufficientBalanceError | InsufficientLiquidityError }
Copy link
Member Author

Choose a reason for hiding this comment

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

on 2nd thought. I would probably move InsufficientLiquidityError into a different category

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.

1 participant