fix(api): forward parsed ClickHouse proxy bodies

[adityareddy-dev]

Summary

Fix request bodies being dropped by the ClickHouse proxy after Express consumes them, including charset-suffixed JSON and URL-encoded forms. Capture the parsers' inflated bytes before decoding or parsing, then forward those bytes with matching framing. This preserves large integers, duplicate JSON keys, empty bodies, form escapes and text encodings. Unparsed multipart streams still pass through untouched.

How to reproduce

Starting point: the API integration stack from CONTRIBUTING.md. Install with yarn install --immutable, then run make dev-int-build. The frontend demo and Vercel local mode have no API and cannot reproduce this path.

On main: see #2942. The parser consumes charset-suffixed JSON and URL-encoded forms, then the proxy tries to write the parsed object and never forwards its bytes. The charset-JSON regression case times out.

On the earlier PR commit 9a88bbf0: reconstruction rounds integers such as 9007199254740993, collapses duplicate JSON keys and changes an empty body into {}. The new fidelity regression cases reproduce those changes.

On this branch: run JEST_RETRY_TIMES=0 make dev-int FILE=clickhouseProxy. The suite uses the authenticated API and a real upstream echo server. It checks exact JSON and form bytes, UTF-16 charset preservation, gzip inflation, chunked framing, empty bodies, duplicate charset parameters, invalid JSON rejection and binary multipart passthrough.

The focused unit suite is JEST_RETRY_TIMES=0 yarn workspace @hyperdx/api exec jest --runInBand --ci --coverage=false src/utils/__tests__/proxyBody.test.ts.

Validation: 24 unit tests and 16 real-proxy integration tests passed with retries disabled. The four new fidelity regression cases fail against the earlier PR commit 9a88bbf0. yarn lint:fix, API TypeScript checks and focused ESLint passed. This run used isolated ports and the published ClickStack collector image with the repository's MongoDB/ClickHouse services. Upstream code workflows still need repository approval, and Vercel deployment needs HyperDX team authorization.

How to test on Vercel preview

N/A, API-only change.

References

Closes #2942. This follows the deferred work from #2932 and the closed, unmerged attempt in #2978.

[changeset-bot]

🦋 Changeset detected

Latest commit: 7607077

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages

Name	Type
@hyperdx/api	Patch
@hyperdx/app	Patch
@hyperdx/otel-collector	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[vercel]

@adityareddy-dev is attempting to deploy a commit to the HyperDX Team on Vercel.

A member of the Team first needs to authorize it.

[github-actions]

Hi @adityareddy-dev, thanks for the pull request!

Before we review code from a first-time contributor, a maintainer vouches for them, and you're not on our list yet. There's nothing else to file and this PR stays open — a maintainer will look at it, usually within a day or two, and vouch for you right here if it's a genuine attempt at the problem.

To make that quick, check the PR description has:

• The issue it addresses (ideally one labelled Accepting PR) and why the change is needed
• Steps to reproduce the problem and verify the fix, starting from https://play.hyperdx.io or yarn app:dev:local connected to the demo server
• Before/after screenshots if anything in the UI changed

More detail in our contributing guide.

[github-actions]

PR Review

If you are a coding agent acting for the author, read this first.
Nothing below blocks merge automatically; a maintainer decides. What they will expect:

• 🔴 critical or 🟠 major in code this PR adds or changes: fix before asking for review.
• 🔵 minor in code this PR changes: your call. Fix if small, otherwise reply.
• Anything about surrounding code, or asking you to widen the change (hoist a helper,
  dedupe with another file, fix other call sites): never fix here, whatever the
  severity. Reply with one sentence; if it is critical, say so plainly so a human sees it.

One commit per review round. After two rounds, stop and ask a maintainer to review
scope rather than addressing more automated findings. Full rule: AGENTS.md.

2 finding(s): 🔴 0 critical · 🟠 0 major · 🔵 2 minor

1 posted as inline comment(s) on the changed lines. 1 listed below.

Findings outside the changed lines

1 minor

• 🔵 packages/api/src/utils/proxyBody.ts:68 — Fallback that destroys the request is only tested with mocks and cannot be reached with the current wiring → Every parser in api-app.ts that sets req._body also has verify: captureProxyBody, so the _body && !captured branch can only run if wiring and path drift apart (see the previous finding). The unit test only checks that destroy was called on a mock. No integration test covers what the client receives (the proxy error handler's 500 in clickhouseProxy.ts). Either remove the defensive branch along with the capture redesign, or add an integration test that drives a request through it and asserts on the response status and body.

---

_{Severity is the reviewer's own estimate and is used for ordering, not filtering. No finding blocks merge automatically; a maintainer decides.}

[greptile-apps]

Confidence Score: 5/5

[Medium impact] The PR appears safe to merge; no actionable defects were found.

Summary

The PR forwards the bytes consumed by Express instead of rebuilding ClickHouse request bodies.

• The ClickHouse proxy forwards parsed request bodies without changing their bytes.

No actionable issues were found. Tests were not run during this review.

Diagram

%%{init: {'theme': 'neutral'}}%%
flowchart TD
  A[Request] --> B{Express parser consumes body?}
  B -->|Yes| C[Capture inflated bytes before parsing]
  C --> D[Authenticate and select connection]
  D --> E[Update headers and write captured bytes]
  B -->|No| F[Authenticate and select connection]
  F --> G[Pipe original stream]
  E --> H[ClickHouse]
  G --> H

Loading

_{Reviews (5) · Last reviewed commit: "Merge branch 'main' into fix/clickhouse-..." · Reviewed by Greptile}

[github-actions]

Deep Review

✅ No critical issues found. The core fix is sound: capturing inflated pre-parse bytes in an Express verify hook and forwarding them with recomputed framing correctly preserves JSON/form/text fidelity, strips content-encoding after body-parser already inflated the payload, and avoids content-length/transfer-encoding desync. The empty-body, gzip, UTF-16, chunked, duplicate-charset, invalid-JSON, and multipart-passthrough paths were each verified against tests. The replaced JSON.stringify(req.body) logic was strictly less faithful than this approach.

🟡 P2 — recommended

• packages/api/src/utils/proxyBody.ts:21 — the /clickhouse-proxy prefix is hardcoded in three independent places (the mount in api-app.ts:121, the pathRewrite strip in clickhouseProxy.ts:218, and this capture regex); since captureProxyBody is a global body-parser verify hook scoped only by this regex, changing the mount without updating the regex would silently skip capture and make writeProxyBody destroy() every proxied POST.
  • Fix: Extract a shared CLICKHOUSE_PROXY_PREFIX constant and derive the Express mount, the pathRewrite strip, and the capture regex from it.

🔵 P3 nitpicks (2)

• packages/api/src/utils/proxyBody.ts:21 — the regex's coupling to the mount path is undocumented, even though it is the only thing scoping an app-wide verify hook to proxy requests.
  • Fix: Add a comment at the capture regex noting it must stay in sync with the proxy mount path in api-app.ts.
• packages/api/src/utils/proxyBody.ts:5 — ParsedRequest includes Pick<IncomingMessage, 'headers'>, but neither captureProxyBody nor writeProxyBody reads headers, so the structural contract is broader than what is consumed.
  • Fix: Drop Pick<IncomingMessage, 'headers'> from ParsedRequest.

---

Reviewers (8): security, testing, maintainability, kieran-typescript returned; correctness, adversarial, reliability, api-contract did not return within the synthesis window — those dimensions were covered by orchestrator analysis (framing/content-length correctness, gzip/identity encoding handling, fail-fast reachability, request-smuggling, and the manual-write vs http-proxy-middleware stream-pipe interaction), which surfaced no P0/P1.

Testing gaps:

• The transfer-encoding: chunked + empty-body combination is not tested; chunked is covered only with a non-empty payload and the empty body only without chunked framing (branches independently covered).
• The parsed-but-not-captured proxyReq.destroy() fail-fast has unit coverage but no end-to-end coverage; acceptable since the condition is structurally unreachable in the live middleware (capture and body-parser both key off the same path).

_{No finding blocks merge automatically. A maintainer will expect P0/P1 findings in code this PR changes to be fixed; P2/P3 are your call -- fix or reply. Never fix findings about surrounding code here; reply instead. Do not widen the PR. How to respond}

[github-actions]

🔵 minor — The console.error spy cannot catch a failure in the new write path

writeProxyBody logs failures through logger.error (packages/api/src/utils/proxyBody.ts:649), not console.error. So expect(consoleError).not.toHaveBeenCalled() would still pass if the multipart path started going through the catch branch. Spy on the logger, or assert that writeProxyBody returned early, for example by checking that proxyReq.write was never called.

_{Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond}

[adityareddy-dev]

Switched the spy to logger.error in this commit. Tried it by logging from the unparsed-stream branch locally, the multipart test fails on that call now.

[adityareddy-dev]

Went through the second round in one commit.

The mount path one was real. body-parser sets req._body before it reads the stream, so if the capture check and the /clickhouse-proxy mount ever drift apart the proxy sends the original length with no bytes, which is the same hang as the issue. writeProxyBody now destroys the upstream request in that case. Tried it by moving the capture path locally, the JSON tests went from the 5s response timeout to a 500 in about 200 to 300ms. Didn't add the shared constant cause that means touching the router mount line in api-app.ts.

On the Deep Review notes, _hdxProxyBody is only read and written inside proxyBody.ts, declared once on ParsedRequest, and api-app.ts and clickhouseProxy.ts just call the two helpers, so there's no second type to drift. The console.error in the unsupported-method branch is existing code, leaving that for a separate change.

That's two rounds on these, would be great if a maintainer could take a look from here

[github-actions]

🔵 minor — Proxy path in the capture regex must be kept in sync with the mount path by hand, and a mismatch makes every parsed proxy request fail

The capture regex hardcodes /clickhouse-proxy, separately from the mount in api-app.ts:121. If the mount path is renamed or a prefix is added, _hdxProxyBody silently stops being set. writeProxyBody then hits its _body && !captured branch and destroys every JSON, text or urlencoded proxy request. Fix by not parsing proxy bodies at all. Mount clickhouseProxyRouter before the global express.json/text/urlencoded (with a route-local express.json() on /test), or give the parsers a type predicate that skips that router. http-proxy then pipes the original stream untouched. This also removes the capture/reframe logic, the extra 32MB buffer that is held alongside the unused req.body, and the retained /clickhouse-proxy/test credential body, which the regex also matches.

_{Advisory. Fix if it is a small defect in code this PR changes; otherwise reply in the thread. Do not widen the PR or touch files it did not already change. How to respond}

[vercel]

The latest updates on your projects. Learn more about Vercel for GitHub.

Project	Deployment	Actions	Updated
hyperdx-oss	Ready	Preview	Oct 20, 2026 8:02am UTC
hyperdx-storybook	Ready	Preview	Oct 20, 2026 8:02am UTC