Return 503 on open-interest all-paths-failure instead of fabricated zeros - #2491
Return 503 on open-interest all-paths-failure instead of fabricated zeros#24910x-SquidSol wants to merge 1 commit into
Conversation
…ated zeros
/api/open-interest/[slab] returned HTTP 200 with { totalOi:"0", ... } when every
data path failed, so a transient RPC failure rendered as a genuine "$0 OI /
Balanced" market. OpenInterestCard already throws on !res.ok and falls back to
the real on-chain engine OI — but the 200 response defeated that and the false
zeros were trusted and displayed.
Return 503 with an `unavailable` marker so the client distinguishes "temporarily
unavailable" from "no open interest" and shows the real on-chain OI (or a neutral
error state) instead of a fabricated $0. The account-missing case is still 404'd
earlier; this only changes the all-paths-failed exception path.
- open-interest/[slab]: 503 + unavailable flag on the all-paths-failed fallback
- add a regression test modeling the consumer contract (200+zeros -> false $0;
503 -> real on-chain OI)
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
@0x-SquidSol is attempting to deploy a commit to the Khubair Nasir's projects Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughThe open-interest API now returns HTTP 503 with ChangesOpen-interest degraded response
Estimated code review effort: 2 (Simple) | ~10 minutes Suggested reviewers: 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches 💡 1🛠️ Fix failing CI checks 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@app/__tests__/api/open-interest-degraded-status.test.ts`:
- Around line 16-45: Replace the local consumerTotalOi model tests with
production contract coverage: mock failed chain and backend dependencies, invoke
the GET route, and assert HTTP 503, unavailable: true, and Cache-Control:
no-store. Test the actual OpenInterestCard response-mapping logic, or extract
that mapping into an importable helper and verify it falls back to on-chain OI
for non-OK responses and remains neutral when unavailable.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: f09bbef6-b45a-49cd-b4d7-17a743e27d88
📒 Files selected for processing (2)
app/__tests__/api/open-interest-degraded-status.test.tsapp/app/api/open-interest/[slab]/route.ts
| // Faithful model of OpenInterestCard's fetch handling (route.tsx:110-142). | ||
| function consumerTotalOi( | ||
| res: { ok: boolean; body: { totalOi?: string } }, | ||
| engineOi: { long: bigint; short: bigint } | null, | ||
| ): string | null { | ||
| if (!res.ok) { | ||
| // catch → on-chain fallback | ||
| if (engineOi) return (engineOi.long + engineOi.short).toString(); | ||
| return null; // neutral/error state (no fabricated number) | ||
| } | ||
| return res.body.totalOi ?? null; // trusts the API body | ||
| } | ||
|
|
||
| const engine = { long: 100n, short: 50n }; // real on-chain OI = 150 | ||
|
|
||
| describe("open-interest degraded response", () => { | ||
| it("200 + zeros makes the consumer show a false $0 (the bug)", () => { | ||
| const res = { ok: true, body: { totalOi: "0" } }; | ||
| expect(consumerTotalOi(res, engine)).toBe("0"); // wrong — market actually has OI | ||
| }); | ||
|
|
||
| it("503 makes the consumer fall back to real on-chain OI (the fix)", () => { | ||
| const res = { ok: false, body: {} }; | ||
| expect(consumerTotalOi(res, engine)).toBe("150"); // real OI shown instead | ||
| }); | ||
|
|
||
| it("503 with no engine yields a neutral/error state, not a fabricated $0", () => { | ||
| const res = { ok: false, body: {} }; | ||
| expect(consumerTotalOi(res, null)).toBeNull(); | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift
Test the production API-to-consumer contract.
This test only verifies a local consumerTotalOi model. It does not call GET or import the OpenInterestCard response logic. It will still pass if the route returns HTTP 200, omits unavailable: true, or the real consumer stops falling back on a non-OK response.
Mock the failed chain and backend paths. Call the route and assert HTTP 503, unavailable: true, and Cache-Control: no-store. Test the actual consumer logic, or extract and import its response-mapping helper.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@app/__tests__/api/open-interest-degraded-status.test.ts` around lines 16 -
45, Replace the local consumerTotalOi model tests with production contract
coverage: mock failed chain and backend dependencies, invoke the GET route, and
assert HTTP 503, unavailable: true, and Cache-Control: no-store. Test the actual
OpenInterestCard response-mapping logic, or extract that mapping into an
importable helper and verify it falls back to on-chain OI for non-OK responses
and remains neutral when unavailable.
dcccrypto
left a comment
dcccrypto
left a comment
There was a problem hiding this comment.
Reviewed on head 87cf55b0. Fix is correct and the reasoning checks out.
Suite clean: 2922 passed / 0 failed.
The load-bearing claim is true — I verified it rather than taking it
The PR rests on "OpenInterestCard already throws on !res.ok and falls back to
the real on-chain engine OI". If that were wrong, 200→503 would break the card
rather than fix it. It's right:
// components/market/OpenInterestCard.tsx:110-142
const res = await fetch(`/api/open-interest/${slabAddress}`);
if (!res.ok) throw new Error("Failed to fetch open interest data");
...
} catch (err) {
setError(...);
if (isV17 && oiLong != null && oiShort != null) { /* real v17 OI */ }
else if (engine) { /* real v12 OI */ }
}Better than the PR claims, actually: the card already has purpose-built UX for
this exact path — :349 renders {error} (on-chain fallback) as a small warning
line. So the user gets the true on-chain OI plus an honest note that the API
was unavailable, instead of a confident fabricated $0 / Balanced. Nothing to
add on the consumer side.
Also checked there is no second consumer that would regress: OpenInterestCard
is the only caller of this endpoint in the app.
Scope note — where the fallback is actually reachable
Worth stating in the PR, because it bounds the blast radius and I got it wrong
first myself. The new 503 is only reached in the indexer / self-contained
branch. When hasIndexerDb() is false the route proxies to Railway and returns
upstream's own response:
const upstream = await proxyToApi(req, `/open-interest/${validSlab}`);
if (!upstream.ok) return upstream; // already non-200 — never falls throughSo the Railway path was never fabricating zeros; the bug and the fix are both
specific to the self-contained path, which is the playground. That's the right
scope — just worth saying so nobody expects a behaviour change on the other one.
The test doesn't bind the fix — seventh in the run
open-interest-degraded-status.test.ts hand-models the consumer
(consumerTotalOi) and never imports the route. I deleted status: 503 —
GH#2489 fully restored — and it stayed 3/3 green.
Drop-in below drives the real GET with every path failing. 4/4 with the fix,
2/4 fail under that same deletion. The hasIndexerDb: () => true mock is the
non-obvious part, per the scope note above.
app/__tests__/api/open-interest-degraded-route-enforcement.test.ts
import { afterAll, beforeEach, describe, expect, it, vi } from "vitest";
import { NextRequest } from "next/server";
const prevNetwork = process.env.NEXT_PUBLIC_DEFAULT_NETWORK;
process.env.NEXT_PUBLIC_DEFAULT_NETWORK = "devnet";
afterAll(() => { process.env.NEXT_PUBLIC_DEFAULT_NETWORK = prevNetwork; });
// Chain path fails: the RPC read throws.
vi.mock("@solana/web3.js", async (importOriginal) => {
const actual = await importOriginal<typeof import("@solana/web3.js")>();
return { ...actual, Connection: class { getAccountInfo() { throw new Error("rpc down"); } } };
});
// The 503 fallback is reachable only in the self-contained/indexer branch: with
// hasIndexerDb() false the route proxies to Railway and returns the upstream's
// own status via `if (!upstream.ok) return upstream`, never falling through.
vi.mock("@/lib/indexer-db", () => ({ hasIndexerDb: () => true }));
vi.mock("@/lib/api-proxy", () => ({
proxyToApi: vi.fn(async () => { throw new Error("proxyToApi must not be reached in the indexer branch"); }),
}));
vi.mock("@sentry/nextjs", () => ({ captureException: vi.fn(), captureMessage: vi.fn() }));
const { GET } = await import("@/app/api/open-interest/[slab]/route");
const SLAB = "DJ54k4wH92NTtNP8RuHAwG8si1bevXEknzctDdqYN8eC";
const get = (slab: string) =>
GET(new NextRequest(`http://localhost/api/open-interest/${slab}`), {
params: Promise.resolve({ slab }),
});
describe("open-interest route signals a degraded state when every path fails", () => {
beforeEach(() => {
vi.stubGlobal("fetch", vi.fn(async () => new Response("nope", { status: 502 })));
});
it("does NOT return 200 (a 200 is what made a transient outage look like $0)", async () => {
const res = await get(SLAB);
expect(res.status).not.toBe(200);
expect(res.status).toBeGreaterThanOrEqual(500);
});
it("returns 503 with an unavailable marker", async () => {
const res = await get(SLAB);
expect(res.status).toBe(503);
const body = (await res.json()) as { unavailable?: boolean; error?: string };
expect(body.unavailable).toBe(true);
expect(typeof body.error).toBe("string");
});
it("keeps the zero-valued body shape so non-checking consumers don't crash", async () => {
const body = (await (await get(SLAB)).json()) as Record<string, unknown>;
for (const k of ["totalOi", "longOi", "shortOi", "netLpPosition"]) {
expect(body[k], k).toBe("0");
}
expect(Array.isArray(body.historicalOi)).toBe(true);
});
it("is not cached — a degraded response must not stick in a CDN", async () => {
const res = await get(SLAB);
expect(res.headers.get("Cache-Control")).toContain("no-store");
});
});Keeping Cache-Control: no-store on the degraded response is the right call and
easy to lose in a later refactor, so it's asserted too — a cached 503 would
outlive the outage.
2 participants
What
Return
503(not200with zeros) when/api/open-interest/[slab]exhausts everydata path, so a transient failure isn't rendered as a real "$0 OI" market.
Closes #2489.
Why
The all-paths-failed fallback returned
{ totalOi:"0", ... }with HTTP 200, so atransient RPC/backend failure looked like a genuine zero-OI market.
OpenInterestCardis already built to recover —
if (!res.ok) throw→ catch → fall back to the realon-chain engine OI — but the 200 response prevented that fallback from firing, so
the fabricated zeros were trusted and displayed.
Changes
503with anunavailable: truemarker (zero fields kept so the body shape stays valid). Theaccount-missing
404path and the success path are unchanged.OpenInterestCard's handling:200+zeros yields afalse
$0, while503triggers the real on-chain OI fallback (and a neutralerror state when no engine data is available).
Testing
npx tsc --noEmit— clean.phantom-oi-unknown-accounts+ theOpenInterestCardcomponent test —3 files, 22 tests, all pass.
Notes
a
5xxcorrectly, so this makes its existing on-chain fallback actually engage.Summary by CodeRabbit