fix(orb): catch stream-read errors in readOrbIngestBody (#8330)#8432
Conversation
A dropped connection or network reset mid-upload rejected reader.read() uncaught, so /v1/orb/ingest and /v1/ams/ingest surfaced a bare framework 500 instead of the clean 413/400 JSON error every other rejected-body case already returns. Wraps the read loop in the same try/catch pattern readOrbRelayRegisterBody already uses for the identical failure mode, returning null (every caller's existing "reject this request" contract) instead of letting the rejection propagate. Adds regression tests mirroring orb-relay.test.ts's dropped-connection case at both the function and route level.
|
Superagent didn't find any vulnerabilities or security issues in this PR. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #8432 +/- ##
==========================================
- Coverage 92.42% 89.61% -2.81%
==========================================
Files 791 98 -693
Lines 79290 22773 -56517
Branches 23950 3896 -20054
==========================================
- Hits 73283 20408 -52875
+ Misses 4868 2187 -2681
+ Partials 1139 178 -961
Flags with carried forward coverage won't be shown. Click here to find out more.
|
|
Tip ✅ LoopOver review result - approve/merge recommendedReview updated: 2026-07-24 13:12:37 UTC
Review summary Nits — 3 non-blocking
Decision drivers
Context & advisory signals — never blocks the verdict
Linked issue satisfactionAddressed Review context
Contributor next steps
Signal definitions
🧪 Chat with LoopOverAsk LoopOver a question about this PR directly in a comment — grounded only in the same cached, public-safe facts shown above, never a new claim.
Full command reference: https://loopover.ai/docs/loopover-commands 🧪 Experimental — new and may change. 🟩 Safe / merged · 🟦 Advisory · 🟨 Held for review · 🟥 Blocked / closed 💰 Earn for open-source contributions like this. Gittensor lets GitHub contributors earn for the work they already do — register to start earning →. Checked by LoopOver, a quiet PR intelligence layer for OSS maintainers.
|
Summary
readOrbRelayRegisterBody(src/orb/relay.ts) wraps its stream-read loop in try/catch specifically becausereader.read()can reject on a dropped connection / network reset mid-read, and every caller already treats anullreturn identically to "reject this request" — a documented, deliberate fix for a real production 500 (GITTENSORY-J).readOrbIngestBody(src/orb/ingest.ts) is the same read-loop pattern, reused as-is by both/v1/orb/ingestand/v1/ams/ingest, but had no try/catch: a dropped connection mid-upload threw an uncaught rejection out of the route handler instead of the clean413/400JSON response the route otherwise returns for an oversized or malformed body.readOrbIngestBody's read loop in the identical try/catch →return nullpattern, with no change to the byte-cap or declared-length-check behavior.orb-relay.test.ts's dropped-connection case at both levels: the function returnsnulldirectly, and the/v1/orb/ingestroute returns its normal clean413 payload_too_largeresponse rather than a framework 500.Closes #8330
Scope
type(scope): short summaryConventional Commit format, for examplefix(api): restore profile access checks.CONTRIBUTING.mdand does not reintroduce GitHub Pages, VitePress,site/, orCNAME.Validation
git diff --checknpm run actionlintnpm run typechecknpm run test:coveragelocally;codecov/patchrequires ≥99% coverage of the lines AND branches you changed (aim for 100% on your diff so CI variance does not fail near the threshold). Global coverage is a non-blocking trend with a loose 90% backstop, not the gate.npm run test:workersnpm run build:mcpnpm run test:mcp-packnpm run ui:openapi:checknpm run ui:lintnpm run ui:typechecknpm run ui:buildnpm audit --audit-level=moderateIf any required check was skipped, explain why:
src/orb/ingest.ts(pure backend logic, no UI/MCP/workers/OpenAPI surface) plus its test file. Verified vianpx vitest run test/integration/orb-ingest.test.ts test/integration/orb-relay.test.ts: 127/128 tests pass, including both new regression tests (function-level and route-level). The single failure (handleOrbIngest() > skips events with bad repo_hash / pr_hash / outcome) is an unrelated, pre-existing local-environment timeout — confirmed by reproducing it identically on a clean stash of this branch's base commit before any of this PR's changes existed; it exerciseshandleOrbIngest, a function this PR does not touch. Coverage on the changed lines (verified directly againstcoverage/lcov.info): every branch insidereadOrbIngestBody, including the newtry/catch, shows both arms hit (e.g.BRDA:45,6,0,7/BRDA:45,6,1,9for the loop'sdonecheck; the catch body'sreturn nullat line 55 shows 2 hits, one per new regression test).Safety
UI Evidencesection below with JPG/JPEG or PNG screenshots. (N/A — no visible UI changes.)Notes