From 62ff1f55a8ee21101dc7e0ac557585d69d772312 Mon Sep 17 00:00:00 2001 From: Teescom Date: Tue, 25 Aug 2026 10:19:49 +0000 Subject: [PATCH] fix: harden RPC error classification, nonce validation, and builder type honesty MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Resolve four Stellar Wave issues: - #456: RateLimitError.fromRpcError() no longer conflates HTTP 503 with 429. A 503 is now reported as a distinct exported RpcServiceUnavailableError, and the internal RPC retry wrapper only backoff-retries genuine RateLimitErrors so callers can fail over to another RPC URL instead of retrying a dead endpoint. - #457: catchNetworkError() only reclassifies errors that are provably transport failures (canonical fetch/axios messages or a network errno code on the error or its nested cause) instead of substring-matching the whole error text, so unrelated TypeErrors are no longer masked as network outages. - #458: NonceManager.toSafeBigInt() throws a descriptive error for unparseable nonce strings instead of silently coercing them to 0n. - #459: StreamBuilder.build() stringifies a numeric ratePerSecond so the runtime value matches the declared `ratePerSecond?: string` return type. Adds regression tests for all four fixes and updates docs + CHANGELOG. πŸ€– Generated with Codebuff Co-Authored-By: Codebuff --- CHANGELOG.md | 4 ++ docs/api.md | 4 +- src/builder.ts | 9 ++- src/errors.ts | 60 ++++++++++++++--- src/index.ts | 1 + src/nonce/NonceManager.ts | 17 +++-- src/soroban.ts | 78 +++++++++++++++++---- src/tests/builder.test.ts | 9 ++- src/tests/nonce-concurrent.test.ts | 25 +++++++ src/tests/rate-limit-error.test.ts | 31 ++++++++- src/tests/soroban-network-error.test.ts | 90 +++++++++++++++++++++++++ src/tests/soroban-rate-limit.test.ts | 16 ++++- 12 files changed, 309 insertions(+), 35 deletions(-) create mode 100644 src/tests/soroban-network-error.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index db31af3..ef05be3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -18,6 +18,10 @@ All notable changes are documented here. Format based on [Keep a Changelog](http - Documented `ConduitClient`'s `pauseStream()`, `unpauseStream()`, and `setWallet()` convenience methods in `docs/api.md`, and fixed `setWallet()`'s JSDoc block, which had been orphaned above `pauseStream()`/`unpauseStream()` and left `setWallet()` itself undocumented. ### Fixed +- **Breaking:** `RateLimitError.fromRpcError()` no longer conflates HTTP 503 (Service Unavailable) with 429 (Too Many Requests). A 503 is now reported as a distinct exported `RpcServiceUnavailableError`, and the internal RPC retry wrapper only backoff-retries genuine `RateLimitError`s β€” a 503 fails fast so consumers can fail over to a different RPC URL instead of retrying a dead endpoint (#456). +- `catchNetworkError()` no longer reclassifies *any* `TypeError` whose text happens to contain `fetch`/`connect`/`network`/etc. It now only reclassifies errors that are provably transport failures: the canonical fetch/axios network messages (`fetch failed`, `Failed to fetch`, `Network Error`, `Load failed`) or an error (or its nested `cause`) carrying a network errno code such as `ECONNREFUSED`/`ENOTFOUND`/`ERR_NETWORK`. A programming `TypeError` (e.g. `Cannot read properties of undefined (reading 'connect')`) is re-thrown as-is instead of being masked as a network outage (#457). +- `NonceManager` now throws a descriptive error for an unparseable nonce string (e.g. `startNonce: 'not-a-number'`) instead of silently coercing it to `0n`, which masked caller bugs as an explicit zero (#458). +- `StreamBuilder.build()` now stringifies a numeric `ratePerSecond` so the runtime value matches the declared `ratePerSecond?: string` return type; previously a `number` input passed through unchanged, so callers trusting the type (`.trim()`, string concatenation) hit runtime errors (#459). - **Critical:** `FeeEstimator.estimateFee()` now uses `bigint` stroops instead of floating-point for fee representation, eliminating IEEE-754 precision loss. All monetary amounts in the SDK now consistently use bigint to avoid rounding errors. - **Critical:** `WalletConnectAdapter.signTransaction()` now requires `networkPassphrase` to be explicitly provided, preventing silently reconstructed Transaction objects with empty passphrases. Throws clear error if passphrase is missing. - **Critical:** `StreamBuilder.submit()` now properly removes failed payloads from `pendingQueue` in a finally block, preventing queue overflow from accumulated failed submissions under sustained network failures. diff --git a/docs/api.md b/docs/api.md index 9b65ffa..a558a15 100644 --- a/docs/api.md +++ b/docs/api.md @@ -357,7 +357,9 @@ clearServerCache(); > **Internal usage:** All SDK functions that interact with the Soroban RPC (`buildContractCallTx`, > `simulateReadOnly`, `invokeContract`, `StreamsModule`, `subscribeToStream`, etc.) build their > server through an internal wrapper that calls `getServer` for the cached instance and adds -> automatic retry-with-backoff on rate-limit errors (HTTP 429/503). Calling `getServer` yourself +> automatic retry-with-backoff on rate-limit errors (HTTP 429). HTTP 503 (Service Unavailable) is +> **not** retried β€” it is surfaced as a `RpcServiceUnavailableError` so callers can fail over to a +> different RPC URL instead of retrying a node that is down. Calling `getServer` yourself > gives you the cached-but-unwrapped instance β€” no automatic retry β€” so you do not need to call > it yourself unless you are using the low-level Soroban helpers directly and want to manage > retries on your own. diff --git a/src/builder.ts b/src/builder.ts index 30b93bb..5bda943 100644 --- a/src/builder.ts +++ b/src/builder.ts @@ -156,7 +156,14 @@ export class StreamBuilder { amount: this._amount, }; if (this._ratePerSecond !== undefined && this._ratePerSecond !== null) { - config.ratePerSecond = this._ratePerSecond; + // build()'s return type promises `ratePerSecond?: string`, but + // bigintSafeStringify() only stringifies `bigint` values β€” a `number` + // input would otherwise pass through unchanged and lie about its type + // at runtime. Coerce numeric inputs here so the declared type is + // honest (see #459). + config.ratePerSecond = typeof this._ratePerSecond === 'number' + ? String(this._ratePerSecond) + : this._ratePerSecond; } return bigintSafeStringify(config) as { diff --git a/src/errors.ts b/src/errors.ts index e0a7cab..dfea5b9 100644 --- a/src/errors.ts +++ b/src/errors.ts @@ -287,6 +287,11 @@ function fromStroopsInternal(stroops: bigint, decimals: number): string { * an equivalent JSON-RPC rate-limit error code, instead of the previous * generic/unclassified error that made rate limiting indistinguishable * from any other network failure. See #120. + * + * A 429 means the caller is being throttled: back off and retry against + * the *same* endpoint. HTTP 503 (Service Unavailable) is a different + * failure mode and is reported as {@link RpcServiceUnavailableError} so + * consumers can distinguish "throttled" from "endpoint down". */ export class RateLimitError extends Error { /** Milliseconds to wait before retrying, parsed from a Retry-After header if present. */ @@ -319,27 +324,41 @@ export class RateLimitError extends Error { } /** - * Detects a rate-limit condition from a raw error thrown by the RPC - * client and converts it into a typed RateLimitError. Handles two shapes: + * Detects a rate-limit or service-unavailable condition from a raw error + * thrown by the RPC client and converts it into a typed error. Handles + * two shapes: * - * 1. An axios-style error (network-level 429), shaped like - * `{ response: { status: 429, headers } }`. + * 1. An axios-style error (network-level HTTP status), shaped like + * `{ response: { status, headers } }`. * 2. A raw JSON-RPC error object (rpc/jsonrpc.js does `throw response.data.error` * directly, so it is a plain object, not an Error instance), shaped * like `{ code: -32029 }`. * - * Returns null if `raw` is not a rate-limit error, so callers can fall - * back to their existing error handling. + * HTTP 429 and the equivalent JSON-RPC codes map to a {@link RateLimitError} + * (throttled β€” back off and retry the same endpoint), while HTTP 503 maps + * to a {@link RpcServiceUnavailableError} (endpoint down β€” consider failing + * over to a different RPC URL). See #456. + * + * Returns null if `raw` is neither a rate-limit nor a service-unavailable + * error, so callers can fall back to their existing error handling. */ - static fromRpcError(raw: unknown): RateLimitError | null { + static fromRpcError(raw: unknown): RateLimitError | RpcServiceUnavailableError | null { if (!raw || typeof raw !== 'object') return null; const response = (raw as { response?: { status?: number; headers?: Record } }).response; - if (response?.status === 429 || response?.status === 503) { + if (response?.status === 429) { const retryAfterHeader = response.headers?.['retry-after']; const retryAfterMs = RateLimitError.parseRetryAfterMs(retryAfterHeader); return new RateLimitError( - `RPC node rate limit or service unavailable (${response.status}). Back off and retry.`, + `RPC node rate limit exceeded (429). Back off and retry against the same endpoint.`, + retryAfterMs, + ); + } + if (response?.status === 503) { + const retryAfterHeader = response.headers?.['retry-after']; + const retryAfterMs = RateLimitError.parseRetryAfterMs(retryAfterHeader); + return new RpcServiceUnavailableError( + `RPC node service unavailable (503). The endpoint may be down; consider failing over to a different RPC URL.`, retryAfterMs, ); } @@ -355,3 +374,26 @@ export class RateLimitError extends Error { return null; } } + +/** + * Thrown when an RPC node responds with HTTP 503 (Service Unavailable). + * + * Distinct from {@link RateLimitError}: a 429 means the caller is being + * throttled and should back off and retry the *same* endpoint, while a 503 + * usually means the node/endpoint itself is down β€” the appropriate + * remediation is to fail over to a different RPC URL rather than retrying + * the same one. Consumers catching `RateLimitError` for backoff-and-retry + * therefore never loop forever against a genuinely unavailable service. + * See #456. + */ +export class RpcServiceUnavailableError extends Error { + /** Milliseconds to wait before retrying, parsed from a Retry-After header if present. */ + readonly retryAfterMs: number | undefined; + + constructor(message: string, retryAfterMs?: number) { + super(message); + this.name = 'RpcServiceUnavailableError'; + this.retryAfterMs = retryAfterMs; + Object.setPrototypeOf(this, new.target.prototype); + } +} diff --git a/src/index.ts b/src/index.ts index ba74de3..453a22a 100644 --- a/src/index.ts +++ b/src/index.ts @@ -27,6 +27,7 @@ export { StreamFiNetworkError, InsufficientBalanceError, RateLimitError, + RpcServiceUnavailableError, SUPPORTED_NETWORKS, UNKNOWN_CONTRACT_ERROR_CODE, } from './errors.js'; diff --git a/src/nonce/NonceManager.ts b/src/nonce/NonceManager.ts index 54e7ce9..cb9ee49 100644 --- a/src/nonce/NonceManager.ts +++ b/src/nonce/NonceManager.ts @@ -42,14 +42,17 @@ export class NonceManager { } private toSafeBigInt(value: bigint | number | string): bigint { - if (typeof value === 'string') { - try { - return BigInt(value); - } catch { - return 0n; - } + if (typeof value === 'string' && value.trim() === '') { + // BigInt('') would coerce to 0n, silently masking a caller bug. + throw new Error('NonceManager: nonce value cannot be an empty string'); + } + try { + return BigInt(value); + } catch { + throw new Error( + `NonceManager: invalid nonce value "${String(value)}" β€” expected a non-negative integer (bigint, number, or numeric string)`, + ); } - return BigInt(value); } private nonceKey(nonce: bigint): string { diff --git a/src/soroban.ts b/src/soroban.ts index c2bd8d7..6380596 100644 --- a/src/soroban.ts +++ b/src/soroban.ts @@ -89,7 +89,10 @@ function normalizePollingOptions(options: ConfirmationPollingOptions = {}): Requ /** * Creates a SorobanRpc.Server instance wrapped with an exponential backoff retry mechanism. - * Retries on HTTP 429 and 503 rate limits. + * Retries on HTTP 429 rate limits (throttled β€” back off and retry the same + * endpoint), but fails fast on HTTP 503, which is surfaced as a + * {@link RpcServiceUnavailableError} so callers can fail over to a + * different RPC URL instead of retrying a node that is down. See #456. */ export function createRpcServer(rpcUrl: string): SorobanRpc.Server { const cached = _proxiedServerCache.get(rpcUrl); @@ -119,12 +122,15 @@ export function createRpcServer(rpcUrl: string): SorobanRpc.Server { try { return await (origMethod as (...a: unknown[]) => Promise).apply(target, args); } catch (err) { - const rateLimitErr = RateLimitError.fromRpcError(err); + const classified = RateLimitError.fromRpcError(err); - if (!rateLimitErr || attempt === MAX_RETRIES) { - throw rateLimitErr ?? err; + // Only a genuine RateLimitError (HTTP 429 / JSON-RPC 429) is + // retried with backoff. A 503 is classified as + // RpcServiceUnavailableError and thrown immediately. + if (!(classified instanceof RateLimitError) || attempt === MAX_RETRIES) { + throw classified ?? err; } - const waitTime = rateLimitErr.retryAfterMs ?? delay; + const waitTime = classified.retryAfterMs ?? delay; await sleep(waitTime); delay *= 2; // Backoff factor: 2x } @@ -346,25 +352,71 @@ function sleep(ms: number): Promise { // ── Network error helpers ───────────────────────────────────────────────────── +/** + * Error codes that identify a transport-level failure: connection + * refused/reset, DNS resolution failure, timeout, unreachable host. These + * come from Node's net/dns layer (`ECONNREFUSED`, `ENOTFOUND`, ...), + * undici (`UND_ERR_*`), axios (`ERR_NETWORK`), or the browser fetch stack + * (`ERR_CONN_*`). + */ +const NETWORK_ERROR_CODE_PATTERN = + /^(?:ECONNREFUSED|ECONNRESET|ENOTFOUND|ETIMEDOUT|ENETUNREACH|EHOSTUNREACH|EAI_AGAIN|EPIPE|ERR_NETWORK|ERR_CONN|ERR_SOCKET|UND_ERR|ERR_HTTP2_CONNECT_ERROR|ERR_TLS)/i; + +/** + * Canonical network-layer failure messages produced by the fetch/axios HTTP + * stacks. These are matched exactly (never substring-matched) so an unrelated + * programming TypeError such as `Cannot read properties of undefined (reading + * 'connect')` is not misclassified as a network outage. + */ +const NETWORK_TYPE_ERROR_MESSAGES = new Set([ + 'fetch failed', // Node.js undici + 'Failed to fetch', // Chromium fetch + 'Network Error', // axios (browser) + 'Load failed', // Safari fetch +]); + +/** + * Walks an error and its nested `cause` chain looking for a transport-level + * error code. Node's `fetch` rejects with `TypeError: fetch failed` whose + * `.cause` carries the real errno code (e.g. `ECONNREFUSED`), so checking the + * nested cause is more reliable than substring-matching the whole error text. + */ +function hasNetworkErrorCode(cause: unknown): boolean { + let current: unknown = cause; + for (let depth = 0; depth < 5 && current !== null && typeof current === 'object'; depth++) { + if ('code' in current) { + const code = String((current as { code: unknown }).code); + if (NETWORK_ERROR_CODE_PATTERN.test(code)) return true; + } + current = (current as { cause?: unknown }).cause; + } + return false; +} + /** * Catches an RPC-level error (e.g. `TypeError: fetch failed`) and re-throws it * as a `StreamFiNetworkError` so callers can distinguish network outages from * contract-logic failures. + * + * Only errors that are *provably* transport failures are reclassified: the + * canonical fetch/axios network messages, or an error (or its nested `cause`) + * carrying a network errno code. A `TypeError` from a programming mistake in + * the simulate/assemble/sign pipeline is re-thrown as-is so it isn't masked + * as a network outage. See #457. */ export function catchNetworkError(label: string, promise: Promise): Promise { return promise.catch((cause: unknown) => { if (cause instanceof StreamFiNetworkError || cause instanceof InsufficientBalanceError) { throw cause; } - if (cause instanceof TypeError && /fetch|network|connect|refused|dns|econnrefused|enotfound|etimedout/i.test(String(cause))) { - throw new StreamFiNetworkError(`Network error during ${label}: ${(cause as Error).message}`, cause); - } - if (cause && typeof cause === 'object' && 'code' in cause) { - const code = String((cause as { code: unknown }).code); - if (/ECONNREFUSED|ENOTFOUND|ETIMEDOUT|ECONNRESET|ERR_CONN/i.test(code)) { - const message = cause instanceof Error ? cause.message : String(cause); - throw new StreamFiNetworkError(`Network error during ${label}: ${message}`, cause); + if (cause instanceof TypeError) { + const message = (cause as Error).message ?? String(cause); + if (NETWORK_TYPE_ERROR_MESSAGES.has(message) || hasNetworkErrorCode(cause)) { + throw new StreamFiNetworkError(`Network error during ${label}: ${(cause as Error).message}`, cause); } + } else if (hasNetworkErrorCode(cause)) { + const message = cause instanceof Error ? cause.message : String(cause); + throw new StreamFiNetworkError(`Network error during ${label}: ${message}`, cause); } // Re-throw non-network errors as-is throw cause; diff --git a/src/tests/builder.test.ts b/src/tests/builder.test.ts index beee510..d893680 100644 --- a/src/tests/builder.test.ts +++ b/src/tests/builder.test.ts @@ -88,7 +88,7 @@ describe('StreamBuilder', () => { }); }); - it('includes ratePerSecond as a number when set with a number', () => { + it('serialises a numeric ratePerSecond to a string to match the declared type', () => { const stream = new StreamBuilder() .token('CAAQCAIBAEAQCAIBAEAQCAIBAEAQCAIBAEAQCAIBAEAQCAIBAEAQC526') .sender('GAAQCAIBAEAQCAIBAEAQCAIBAEAQCAIBAEAQCAIBAEAQCAIBAEAQDZ7H') @@ -97,7 +97,12 @@ describe('StreamBuilder', () => { .ratePerSecond(500) .build(); - expect(stream.ratePerSecond).toBe(500); + // build()'s return type promises `ratePerSecond?: string` β€” the runtime + // value must match the declared type (see #459). + expect(stream.ratePerSecond).toBe('500'); + expect(typeof stream.ratePerSecond).toBe('string'); + const json = JSON.parse(JSON.stringify(stream)); + expect(json.ratePerSecond).toBe('500'); }); it('serialises bigint ratePerSecond to string', () => { diff --git a/src/tests/nonce-concurrent.test.ts b/src/tests/nonce-concurrent.test.ts index cc0ebd3..58439c4 100644 --- a/src/tests/nonce-concurrent.test.ts +++ b/src/tests/nonce-concurrent.test.ts @@ -148,6 +148,31 @@ describe('NonceManager β€” Concurrent Nonce Integration Tests', () => { m.destroy(); }); + it('throws a descriptive error for an unparseable string startNonce', () => { + // Regression test for #458: an unparseable nonce must not silently + // coerce to 0n, which would mask a caller bug (e.g. a stringified + // undefined or a malformed network response) as an explicit 0. + expect(() => new NonceManager({ startNonce: 'not-a-number' })).toThrow( + /invalid nonce value "not-a-number"/, + ); + }); + + it('throws for an empty-string startNonce instead of silently using 0n', () => { + expect(() => new NonceManager({ startNonce: '', maxNonce: '100' })).toThrow( + /empty string/, + ); + }); + + it('accepts numeric and bigint startNonce values', () => { + const fromNumber = new NonceManager({ startNonce: 42, maxNonce: 100 }); + expect(fromNumber.current).toBe(42n); + fromNumber.destroy(); + + const fromBigInt = new NonceManager({ startNonce: 7n, maxNonce: 100n }); + expect(fromBigInt.current).toBe(7n); + fromBigInt.destroy(); + }); + it('reset clears state and allows reacquisition', async () => { const lock1 = await manager.acquire(); lock1.release(); diff --git a/src/tests/rate-limit-error.test.ts b/src/tests/rate-limit-error.test.ts index 35590ff..65cb936 100644 --- a/src/tests/rate-limit-error.test.ts +++ b/src/tests/rate-limit-error.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect, vi } from 'vitest'; -import { RateLimitError } from '../errors.js'; +import { RateLimitError, RpcServiceUnavailableError } from '../errors.js'; describe('RateLimitError.fromRpcError', () => { it('detects an axios-style 429 response and returns a RateLimitError', () => { @@ -60,6 +60,35 @@ describe('RateLimitError.fromRpcError', () => { vi.useRealTimers(); }); + it('classifies a 503 response as a distinct RpcServiceUnavailableError', () => { + const serviceUnavailable = { + message: 'Request failed with status code 503', + response: { + status: 503, + headers: { 'retry-after': '10' }, + data: {}, + }, + }; + + const result = RateLimitError.fromRpcError(serviceUnavailable); + expect(result).toBeInstanceOf(RpcServiceUnavailableError); + expect(result).toBeInstanceOf(Error); + expect(result?.name).toBe('RpcServiceUnavailableError'); + expect((result as RpcServiceUnavailableError).retryAfterMs).toBe(10_000); + }); + + it('keeps 503 distinguishable from 429 for retry/failover decisions', () => { + const r429 = RateLimitError.fromRpcError({ response: { status: 429 } }); + const r503 = RateLimitError.fromRpcError({ response: { status: 503 } }); + + // A 429 is a RateLimitError (retry the same endpoint)… + expect(r429).toBeInstanceOf(RateLimitError); + // …while a 503 must NOT be, so backoff-and-retry loops keyed on + // `instanceof RateLimitError` never retry a dead endpoint forever. + expect(r503).not.toBeInstanceOf(RateLimitError); + expect(r503).toBeInstanceOf(RpcServiceUnavailableError); + }); + it('detects a raw JSON-RPC rate-limit error object (not an Error instance)', () => { const rawJsonRpcError = { code: -32029, message: 'Too many requests' }; diff --git a/src/tests/soroban-network-error.test.ts b/src/tests/soroban-network-error.test.ts new file mode 100644 index 0000000..53daa0e --- /dev/null +++ b/src/tests/soroban-network-error.test.ts @@ -0,0 +1,90 @@ +import { describe, it, expect } from 'vitest'; +import { catchNetworkError } from '../soroban.js'; +import { StreamFiNetworkError, InsufficientBalanceError } from '../errors.js'; + +describe('catchNetworkError', () => { + it('reclassifies the canonical undici `TypeError: fetch failed` as a StreamFiNetworkError', async () => { + const promise = Promise.reject(new TypeError('fetch failed')); + + await expect(catchNetworkError('simulateTransaction', promise)).rejects.toBeInstanceOf( + StreamFiNetworkError, + ); + await expect(catchNetworkError('simulateTransaction', promise)).rejects.toThrow( + /Network error during simulateTransaction/, + ); + }); + + it('reclassifies a TypeError whose nested cause carries a network errno code', async () => { + // Node's fetch rejects with `TypeError: fetch failed` and puts the real + // errno (ECONNREFUSED etc.) on the nested `cause`. + const inner = Object.assign(new Error('connect ECONNREFUSED 127.0.0.1:8000'), { code: 'ECONNREFUSED' }); + const cause = Object.assign(new TypeError('fetch failed'), { cause: inner }); + + await expect(catchNetworkError('sendTransaction', Promise.reject(cause))).rejects.toBeInstanceOf( + StreamFiNetworkError, + ); + }); + + it('reclassifies browser fetch failures (`Failed to fetch`)', async () => { + await expect( + catchNetworkError('getAccount', Promise.reject(new TypeError('Failed to fetch'))), + ).rejects.toBeInstanceOf(StreamFiNetworkError); + }); + + it('reclassifies a non-TypeError error carrying a network errno code', async () => { + const cause = Object.assign(new Error('getaddrinfo ENOTFOUND soroban-testnet.stellar.org'), { + code: 'ENOTFOUND', + }); + + await expect(catchNetworkError('getAccount', Promise.reject(cause))).rejects.toBeInstanceOf( + StreamFiNetworkError, + ); + }); + + it('reclassifies an axios-style error object with ERR_NETWORK code', async () => { + await expect( + catchNetworkError('simulateTransaction', Promise.reject({ code: 'ERR_NETWORK', message: 'Network Error' })), + ).rejects.toBeInstanceOf(StreamFiNetworkError); + }); + + it('does NOT misclassify an unrelated TypeError mentioning "connect" (regression for #457)', async () => { + // A programming bug in the simulate/assemble/sign pipeline must be + // reported as the real TypeError, not masked as a network outage. + const bug = new TypeError("Cannot read properties of undefined (reading 'connect')"); + + await expect(catchNetworkError('simulateTransaction', Promise.reject(bug))).rejects.toBe(bug); + await expect(catchNetworkError('simulateTransaction', Promise.reject(bug))).rejects.toBeInstanceOf(TypeError); + await expect(catchNetworkError('simulateTransaction', Promise.reject(bug))).rejects.not.toBeInstanceOf( + StreamFiNetworkError, + ); + }); + + it('does NOT misclassify an unrelated TypeError mentioning "fetch"', async () => { + const bug = new TypeError("Cannot read properties of undefined (reading 'fetch')"); + + await expect(catchNetworkError('getAccount', Promise.reject(bug))).rejects.toBe(bug); + await expect(catchNetworkError('getAccount', Promise.reject(bug))).rejects.not.toBeInstanceOf( + StreamFiNetworkError, + ); + }); + + it('re-throws non-network errors as-is', async () => { + const contractError = new Error('Simulation failed: contract error #7'); + + await expect( + catchNetworkError('simulateTransaction', Promise.reject(contractError)), + ).rejects.toBe(contractError); + }); + + it('passes an already-classified StreamFiNetworkError through unchanged', async () => { + const networkError = new StreamFiNetworkError('Network error during getAccount: fetch failed'); + + await expect(catchNetworkError('getAccount', Promise.reject(networkError))).rejects.toBe(networkError); + }); + + it('passes an InsufficientBalanceError through unchanged', async () => { + const insufficient = new InsufficientBalanceError(10_000_000n, 50_000_000n); + + await expect(catchNetworkError('invoke', Promise.reject(insufficient))).rejects.toBe(insufficient); + }); +}); diff --git a/src/tests/soroban-rate-limit.test.ts b/src/tests/soroban-rate-limit.test.ts index ffb9eb6..062cf59 100644 --- a/src/tests/soroban-rate-limit.test.ts +++ b/src/tests/soroban-rate-limit.test.ts @@ -1,5 +1,5 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; -import { RateLimitError } from '../errors.js'; +import { RateLimitError, RpcServiceUnavailableError } from '../errors.js'; const { mockSimulateTransaction, @@ -70,6 +70,20 @@ describe('soroban.ts rate limit handling', () => { await expect(promise).rejects.toBeInstanceOf(RateLimitError); }); + it('surfaces a 503 as RpcServiceUnavailableError without retrying the same endpoint', async () => { + // Regression test for #456: a 503 means the node is down, so the retry + // proxy must not backoff-and-retry it like a 429 β€” it fails fast with a + // distinguishable error so callers can fail over to another RPC URL. + mockSimulateTransaction.mockRejectedValue({ + response: { status: 503, headers: {} }, + }); + + await expect( + simulateReadOnly('http://localhost:8000', 'passphrase', {} as any) + ).rejects.toBeInstanceOf(RpcServiceUnavailableError); + expect(mockSimulateTransaction).toHaveBeenCalledTimes(1); + }); + it('still throws the original error for non-rate-limit failures', async () => { mockSimulateTransaction.mockRejectedValueOnce(new Error('ECONNREFUSED'));