fix: harden RPC error classification, nonce validation, and builder type honesty

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.

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.

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 {

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<string, unknown> } }).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);
+  }
+}

src/index.ts

@@ -27,6 +27,7 @@ export {
   StreamFiNetworkError,
   InsufficientBalanceError,
   RateLimitError,
+  RpcServiceUnavailableError,
   SUPPORTED_NETWORKS,
   UNKNOWN_CONTRACT_ERROR_CODE,
 } from './errors.js';

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 {

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<unknown>).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<void> {
 
 // ── 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<T>(label: string, promise: Promise<T>): Promise<T> {
   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;

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', () => {

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();

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' };