Skip to content

fix(orb): fail safe on a stream read error in readOrbRelayRegisterBody - #3456

Merged
loopover-orb[bot] merged 1 commit into
mainfrom
fix/orb-relay-body-read-stream-error
Jul 5, 2026
Merged

fix(orb): fail safe on a stream read error in readOrbRelayRegisterBody#3456
loopover-orb[bot] merged 1 commit into
mainfrom
fix/orb-relay-body-read-stream-error

Conversation

@JSONbored

@JSONbored JSONbored commented Jul 5, 2026

Copy link
Copy Markdown
Owner

Closes #3460

Summary

  • Found via Sentry (issue GITTENSORY-J, 79 occurrences, regressed): orb_broker_unavailable: Orb broker token exchange failed (500). Seer's suggested framing ("wrap the entire handler in a top-level try-catch") turned out to be imprecise — the /v1/orb/token route (src/api/routes.ts:3175) already wraps its actual token-minting call (brokerOrbToken) in a try/catch that correctly returns a clean 503. The real gap: readOrbRelayRegisterBody (src/orb/relay.ts), which reads the raw request body via a ReadableStream reader, is called before that try/catch at all three of its call sites (POST /v1/orb/token, /v1/orb/relay/register, /v1/orb/relay pull). A dropped connection or network reset mid-read throws uncaught out of this function, escaping as a bare framework 500 instead of any route's own clean 4xx/503 JSON response — exactly matching the observed "(500)" in the error text.
  • Fix: wrap the read loop in readOrbRelayRegisterBody itself and return null on any stream error — the same sentinel already used uniformly by all three callers for "payload too large." This is a single-point fix (the function, not each call site) since all three routes already handle the null case identically.
  • Closes fix(orb): readOrbRelayRegisterBody throws uncaught on a stream read error #3460.

Scope

  • The PR title follows type(scope): short summary Conventional Commit format, for example fix(api): restore profile access checks.
  • This PR is focused and does not mix unrelated backend, UI, MCP, docs, dependency, and deploy changes.
  • This follows CONTRIBUTING.md and does not reintroduce GitHub Pages, VitePress, site/, or CNAME.
  • I linked an issue, or this is small enough that the summary explains why an issue is not needed.

Validation

  • git diff --check
  • npm run typecheck (clean)
  • npx vitest run test/integration/orb-relay.test.ts — 77/77 passing
  • Scoped coverage check (vitest --coverage --coverage.include=src/orb/relay.ts): 100% line coverage on the changed function; the one pre-existing uncovered branch (in an unrelated function, shouldPersistRelayFailure) is untouched by this change.
  • npm run test:workers / npm run build:mcp / npm run test:mcp-pack / npm run ui:openapi:check / npm run ui:build — not run individually this PR; no worker/MCP/OpenAPI/UI surface touched.
  • New or changed behavior has unit/integration tests for new branches, fallback paths, and sanitizer boundaries — added a regression test constructing a ReadableStream that enqueues a partial chunk then errors (simulating a realistic mid-read network drop), asserting readOrbRelayRegisterBody resolves to null instead of rejecting.

Safety

  • No secrets, wallet details, hotkeys, coldkeys, user PATs, private keys, raw trust scores, private rankings, or private maintainer evidence are exposed.
  • Public GitHub text stays sanitized, low-noise, and does not imply compensation guarantees or optimization tactics.
  • Auth, cookie, CORS, GitHub App, Cloudflare, or session changes include negative-path tests. — N/A; this is a body-parsing robustness fix, not an auth-boundary change (auth on these routes is unaffected and unconditioned on this function's return value beyond the existing null-check).
  • API/OpenAPI/MCP behavior is updated and tested where needed. — N/A, no request/response contract change (the 413 response for null already existed at every call site; this only prevents an additional failure mode from bypassing it as an uncaught exception).
  • UI changes use live API data or real empty/error/loading states, not production mock/demo fallbacks. — N/A, no UI change.
  • Visible UI changes include a UI Evidence section below. — N/A, no visible UI change.
  • Public docs/changelogs are updated where needed. — N/A, internal reliability fix, no documented/user-facing behavior change.

Notes

Found via Sentry (GITTENSORY-J): a dropped connection or network
reset mid-read threw uncaught out of readOrbRelayRegisterBody, which
all three callers (POST /v1/orb/token, /v1/orb/relay/register,
/v1/orb/relay pull) invoke before their own try/catch -- the
exception escaped as a bare framework 500 instead of each route's
own clean 4xx/503 JSON response. Wrap the read loop and return null
(the same sentinel already used for an oversized payload) on any
stream error, fixing all three call sites at the source.
@superagent-security

Copy link
Copy Markdown
Contributor

Superagent didn't find any vulnerabilities or security issues in this PR.

@loopover-orb loopover-orb Bot added the gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. label Jul 5, 2026
@loopover-orb

loopover-orb Bot commented Jul 5, 2026

Copy link
Copy Markdown
Contributor

Tip

🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩🟩

✅ Gittensory review result - approve/merge recommended

Review updated: 2026-07-05 07:52:01 UTC

2 files · 1 AI reviewer · no blockers · readiness 100/100 · CI green · clean

✅ Suggested Action - Approve/Merge

  • safe to merge

Review summary
The change puts the stream-read failure handling at the right layer: `readOrbRelayRegisterBody` is the shared raw-body reader, and converting `reader.read()` failures to the existing `null` sentinel matches the callers' established rejection path. The regression test exercises a realistic partial-read-then-error shape rather than only an immediate synthetic failure, so the covered path maps to the bug described. I do not see a reachable correctness blocker in the diff.

Nits — 5 non-blocking
  • nit: `src/orb/relay.ts:166` turns a small helper comment into incident narrative; keep the docstring to the behavioral contract and move route/Sentry history to the PR or test name.
  • nit: `test/integration/orb-relay.test.ts:144` repeats several implementation details from the production comment, which makes future route refactors more likely to leave stale test prose behind.
  • In `src/orb/relay.ts:166`, shorten the comment to something like `Returns null when the body is too large or the request stream errors while being read.`
  • In `test/integration/orb-relay.test.ts:144`, keep the test focused on the contract: partial body chunk followed by stream error resolves to `null`.
  • PR author also opened the linked issue — Link an issue that was opened by a different contributor, or provide a rationale for why this self-authored issue represents genuine discovery work.
Signal Result Evidence
Code review ✅ No blockers 1 reviewer
Linked issue ✅ Linked #3460
Related work ✅ No active overlap found No same-issue or scoped active PR overlap found.
Change scope ✅ 20/20 Low review scope from cached public metadata (1 linked issue).
Validation posture ✅ 25/25 PR body includes validation/test evidence.
Contributor workload ✅ 10/10 Author activity: 56 registered-repo PR(s), 46 merged, 416 issue(s).
Contributor context ✅ Confirmed Gittensor contributor JSONbored; Gittensor profile; 56 PR(s), 416 issue(s).
Gate result ✅ Passing No configured blocker found.
Review context
  • Author: JSONbored
  • Role context: owner (maintainer lane)
  • Public audience mode: oss maintainer
  • Lane context: Repository registration is not available in the local Gittensory cache.
  • Public profile languages: not available
  • Official Gittensor activity: 56 PR(s), 416 issue(s).
  • PR-specific overlap: none found.
Contributor next steps
  • Treat this as maintainer-lane context rather than normal contributor-lane activity.
  • No action.
Signal definitions
  • Related work = same linked issue, overlapping active PRs, or title/path similarity.
  • Change scope = cached public metadata such as size labels, draft state, and review-burden hints.
  • Validation posture = whether the PR provides enough public validation/test evidence for maintainer review.
  • Contributor workload = public contributor activity and cleanup pressure, not a repo-wide quality failure.
  • Contributor context = public GitHub/Gittensor identity context; non-Gittensor status is not a blocker.

🟩 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 Gittensory, a quiet PR intelligence layer for OSS maintainers.

  • Re-run Gittensory review

@codecov

codecov Bot commented Jul 5, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 92.99%. Comparing base (be744ba) to head (b74fc7d).
⚠️ Report is 2 commits behind head on main.
✅ All tests successful. No failed tests found.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #3456   +/-   ##
=======================================
  Coverage   92.99%   92.99%           
=======================================
  Files         296      296           
  Lines       30972    30974    +2     
  Branches    11294    11294           
=======================================
+ Hits        28803    28805    +2     
  Misses       1514     1514           
  Partials      655      655           
Files with missing lines Coverage Δ
src/orb/relay.ts 100.00% <100.00%> (ø)
🚀 New features to boost your workflow:
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@loopover-orb loopover-orb Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gittensory approves — the gate is satisfied and CI is green.

@loopover-orb
loopover-orb Bot merged commit 65de78e into main Jul 5, 2026
10 checks passed
@loopover-orb
loopover-orb Bot deleted the fix/orb-relay-body-read-stream-error branch July 5, 2026 07:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

gittensor:bug Gittensor-scored bug fix — scores a 0.05x multiplier. manual-review Gittensor contributor context

Development

Successfully merging this pull request may close these issues.

fix(orb): readOrbRelayRegisterBody throws uncaught on a stream read error

1 participant