Circuit breaker false-trips: CircuitBreaker.call() conflates 'source doesn't support this asset' with 'source is broken', tripping the whole source offline

[prodbycorne]

Overview

utils/circuitBreaker.js's CircuitBreaker.call() treats any null/undefined return from the wrapped function as a failure — but every price source's fetchPrice() legitimately returns null for the extremely common, permanent, non-error condition of "this source doesn't support this specific asset," not just for actual transient failures. Because each source's CircuitBreaker instance is shared across all assets (one breaker per source, not per source-per-asset), a source being asked about even one asset it doesn't support will eventually trip its circuit open — taking that source offline for every other asset it does support, for the full cooldown window, purely because of a normal "unsupported asset" response getting misclassified as a source failure.

// utils/circuitBreaker.js
async call(fn) {
  ...
  try {
    const result = await fn();
    if (result === null || result === undefined) {
      this.recordFailure();
    } else {
      this.recordSuccess();
    }
    return result ?? null;
  } catch (err) {
    this.recordFailure();
    throw err;
  } finally { ... }
}

recordFailure() increments failureCount and trips the breaker OPEN once failureThreshold (default 3) is reached. Now look at what each source's fetchPrice() actually returns for an unsupported asset — not an error, a plain, expected null:

// coingecko.js
async function fetchPrice(assetCode) {
  const coinId = STELLAR_COINGECKO_MAP[assetCode];   // { XLM: 'stellar' } — ONLY XLM is mapped
  if (!coinId) {
    logger.debug('Asset not supported by CoinGecko', { assetCode });
    return null;                                       // <-- counted as a CircuitBreaker "failure"
  }
  ...
}

// coinmarketcap.js — resolveMarket() returns null for any asset/issuer not in config.coinmarketcap.assetIssuerMap
// stellarDex.js — fetchPrice() returns null when a non-XLM asset is queried without an issuer

STELLAR_COINGECKO_MAP maps only XLM. Any other watched asset — and config.coinmarketcap.assetIssuerMap in src/config.js explicitly configures USDC as a first-class watched asset ([USDC:${usdcIssuer}]: { id: 3408 }), confirming multi-asset deployments are the expected norm, not an edge case — causes coingecko.fetchPrice('USDC', ...) to return null on every single call, forever, simply because CoinGecko was never mapped for that asset. Since priceOracle.js's SOURCES gives CoinGecko exactly one shared CircuitBreaker('coingecko', breakerOptions) instance covering every asset, three consecutive price-fetch cycles that happen to check USDC (or any other CoinGecko-unmapped asset) — which, given refreshAllCachedPrices() iterates every cached asset key every cycle, will happen constantly in any multi-asset deployment — will trip the CoinGecko breaker OPEN for timeoutMs (default 30000ms), during which every other asset CoinGecko does support (including XLM) also gets skipped, per fetchFromAllSources()'s source.breaker.call(...) short-circuiting to null while state === OPEN. The breaker then half-opens, probes (quite possibly against the same unsupported asset again, since nothing routes the probe toward a supported asset specifically), fails again, and reopens — a source that is perfectly healthy can end up permanently flapping open/half-open/closed, never contributing to price aggregation with the reliability its actual health would justify, purely as an artifact of also being asked about assets it was never going to support.

This directly degrades price-aggregation quality and redundancy (fewer of the 3 configured sources actually contributing to the median for supported assets, more often, than the underlying source health would suggest) and is a purely emergent, cross-file interaction — no single function is "wrong" in isolation; CircuitBreaker.call()'s null-means-failure convention is reasonable in general, and each source's return null for an unsupported asset is also individually reasonable — but combined, with a single breaker instance spanning all assets, they produce a false-positive trip mechanism that would be very easy to miss without deliberately tracing the full call chain from priceOracle.js → CircuitBreaker.call → each source's fetchPrice.

Requirements

• Distinguish "this source cannot serve this asset" (a permanent, per-asset, non-error condition) from "this source failed to serve a request it should be able to answer" (a transient, source-wide health signal) at the point where CircuitBreaker.call() decides whether to recordFailure(). The cleanest fix is for each source's fetchPrice() to signal "not supported" distinctly from "failed" — e.g. throwing/returning a distinguishable sentinel for "unsupported," or having callers check asset-support before ever invoking the breaker-wrapped call for that asset/source pair, so an unsupported lookup never reaches CircuitBreaker.call() at all.
• Ensure the fix doesn't lose the legitimate original signal CircuitBreaker.call() was trying to capture — a source that returns null for an asset it's supposed to support (e.g. CoinGecko returning no price data for XLM due to a transient issue) should still count toward the failure threshold as it does today.
• Add a test that specifically exercises a multi-asset scenario (one supported, one unsupported, by the same source) and asserts the unsupported asset's null returns do not degrade the breaker's state for the supported asset.

Acceptance Criteria

• Repeatedly requesting a price for an asset a given source doesn't support does not trip that source's circuit breaker OPEN.
• A source's circuit breaker still trips OPEN after failureThreshold genuine failures (network errors, unexpected null for an asset it should support, etc.) — the fix narrows what counts as a failure, it doesn't remove failure detection.
• A test asserts that fetching prices for a CoinGecko-unsupported asset (e.g. an issued asset not in STELLAR_COINGECKO_MAP) many times in a row does not affect CoinGecko's breaker state, and that XLM (CoinGecko-supported) fetches continue succeeding via CoinGecko unaffected throughout.
• The same fix (or an equivalent, source-appropriate one) is applied consistently across coingecko.js, coinmarketcap.js, and stellarDex.js, since all three have the identical "return null for an unsupported/misconfigured asset" pattern feeding into the same shared-breaker mechanism.

Additional Notes

More precise references

• src/utils/circuitBreaker.js:36-76 (CircuitBreaker.call): confirmed if (result === null || result === undefined) { this.recordFailure(); } at lines 62-63 — the exact null-means-failure logic.
• src/services/priceOracle.js:12-27 (SOURCES, real entries only — see the companion duplicate-SOURCES-entries issue in this batch for the separate bug affecting the dead duplicate entries): confirmed one CircuitBreaker instance is constructed per source name, shared across every fetch(assetCode, issuer) call for that source regardless of which asset is being queried.
• src/services/sources/coingecko.js:6-8,33-38: confirmed STELLAR_COINGECKO_MAP = { XLM: 'stellar' } and confirmed if (!coinId) { ...; return null; } for any asset not in that map.
• src/services/sources/coinmarketcap.js:28-52 (resolveMarket) and :60-63: confirmed if (!market) { return null; } for any asset/issuer combination not present in config.coinmarketcap.assetIssuerMap.
• src/services/sources/stellarDex.js:50-53: confirmed if (!issuer && normalizedCode !== 'XLM') { ...; return null; } for a non-XLM asset queried without an issuer.
• src/config.js coinmarketcap.assetIssuerMap: confirmed { XLM: { symbol: 'XLM' }, [USDC:${usdcIssuer}]: { id: 3408 } } — confirming USDC is a real, explicitly-configured, first-class watched asset in this deployment's own default config, which per STELLAR_COINGECKO_MAP above is not supported by CoinGecko at all, making this issue's scenario the expected default state, not a contrived edge case.
• src/services/priceOracle.js:106-121 (fetchFromAllSources): confirmed await source.breaker.call(() => source.fetch(assetCode, issuer)) is called uniformly for every configured asset against every source, with no asset-support pre-check anywhere in this function — every call, supported or not, goes through the shared breaker.

Additional edge cases

• This interacts with the half-open probe mechanism specifically: once OPEN, the breaker transitions to HALF_OPEN after timeoutMs and allows exactly one probe call through (halfOpenInFlight guarding against concurrent probes). If that probe happens to be for the same unsupported asset that caused the trip in the first place (quite likely, since fetchFromAllSources is called per-asset and a refresh cycle iterates all cached assets in sequence), the probe itself returns null again, and per recordFailure()'s HALF_OPEN branch, immediately re-trips to OPEN (if (this.state === STATES.HALF_OPEN) { this._transitionTo(STATES.OPEN, ...); return; }) — meaning a source can get stuck in a near-permanent open/half-open-fail/open cycle driven entirely by a single unsupported asset happening to be checked around the same time the cooldown expires, never getting a chance to actually prove itself healthy against an asset it supports.
• Worth considering whether per-source-per-asset breakers (rather than per-source-only) would be the more robust long-term fix versus filtering out "unsupported" at the call site — either is a legitimate fix; the acceptance criteria above are written to accommodate either approach as long as the false-positive-trip behavior is eliminated.

Test/reproduction plan

// Simulate 3+ consecutive lookups for a CoinGecko-unsupported asset.
for (let i = 0; i < 5; i++) {
  await priceOracle.fetchFreshPrice('SOME_UNSUPPORTED_ASSET', null);
}
// Then check whether CoinGecko's breaker is OPEN (it currently would be) and whether an XLM lookup
// right after still gets a CoinGecko contribution to its median (it currently would not, while OPEN).
const xlmResult = await priceOracle.fetchFreshPrice('XLM', null);
expect(xlmResult.sources_attempted).toContain('coingecko'); // currently fails after the unsupported-asset trips

Cross-references

• Companion issue in this batch — "priceOracle SOURCES array has broken duplicate entries" — a separate, independently-confirmed bug in the same SOURCES/circuit-breaker neighborhood; both are worth fixing together since a reviewer will likely touch nearby code for each.
• Scheduled price-refresh cron has no reentrancy guard — slow cycles overlap and race on the cache #71 (open), Price cache-miss stampede: concurrent requests for the same asset all hit CoinGecko/CoinMarketCap simultaneously #69 (open), Price anomaly detection only logs — anomalous or single-source prices are still cached and served #70 (open), CoinMarketCap's nonRetryable auth-failure flag is set but never consulted — invalid keys are retried every 30s forever #95 (closed) — all concern price-oracle reliability/circuit-breaker behavior; this issue's false-trip mechanism is a distinct, previously-undocumented root cause of reduced source redundancy, worth cross-referencing for anyone investigating "why does price aggregation seem to use fewer sources than expected" reports.

[davidishere1]

I’m interested in this issue because it highlights an important reliability problem in the circuit-breaker logic: a legitimate null response is being treated as a source failure, allowing an unsupported asset to affect availability for assets that the source actually supports.

I have 5+ years of programming experience, with a focus on building reliable and maintainable applications. I’m comfortable working with asynchronous services, error handling, circuit breakers, and debugging edge cases across shared service infrastructure. I’d address this by distinguishing expected “unsupported asset” responses from genuine failures, while preserving the circuit breaker’s behavior for actual transient errors. I’d also add regression tests covering unsupported assets, supported assets, and circuit state transitions to ensure the fix is robust.

[grantfox-oss]

🦊 GrantFox — @davidishere1 has been assigned to this issue as part of the Third Campaign campaign!

Next steps:

1. Open a Pull Request referencing this issue (e.g., Closes #130)
2. Your PR will be reviewed by the SmartDropLabs maintainers

Good luck! Track your progress on GrantFox.