fix(prestodb-driver): Send custom headers on nextUri poll requests

[ovr]

The upstream presto-client only applies custom headers to the initial POST /v1/statement request. The follow-up nextUri GET polls (and the cancel/kill/node requests) were issued with an empty header set, so custom headers required by a proxy/gateway in front of Presto/Trino (e.g. Proxy-Authorization) were dropped after the first request, causing the query to fail with "User authentication failed".

Wrap client.request so the configured custom headers are applied to every request, while preserving the upstream nextUri host-following behaviour.

Also split the driver tests into test/unit and test/integration folders and add a unit script, matching the trino-driver convention.

[claude]

Claude finished @ovr's task in 1m 8s —— View job

---

Code review

Verdict: Looks good overall — targeted fix, clear rationale, real regression coverage. A few notes below; nothing blocking.

Details — 0 high, 2 medium, 2 low

Checklist

• Read PR diff and changed files
• Inspect applyCustomHeadersToAllRequests monkey-patch
• Review new unit test
• Check package.json script changes

Medium

PrestoDriver.ts:187 — mutating caller's opts.headers

opts.headers = { ...customHeaders, ...(opts.headers || {}) };

This mutates the object passed in by the caller (the upstream presto-client or queryPromised). Since queryPromised passes headers: this.config.headers directly, the merge into opts.headers is fine on the first call but the object identity of this.config.headers is not touched (a new object is assigned to opts.headers). So this is safe in practice — but worth a one-line comment, or returning a new opts object, to avoid future regressions if a caller starts reusing opts across calls.

PrestoDriver.ts:174-175 — protocol/port handling for the view

view.port = href.port || (href.protocol === 'https:' ? '443' : '80');
view.protocol = href.protocol;

The upstream presto-client switches between follow-redirects/http and follow-redirects/https based on this.ssl (truthy → https), not this.protocol. If a nextUri points to a different scheme than the configured one (e.g. coordinator is HTTPS but a worker nextUri returns plain HTTP, or vice versa), setting view.protocol alone won't switch the transport — the request will be dispatched to the wrong library. Worth verifying against the actual client and either overriding view.ssl or documenting that scheme switching is not supported. The new test covers same-scheme cross-host only.

Low

PrestoDriver.ts:148 — monkey-patching via prototype capture

const originalRequest = client.request;
client.request = function patchedRequest(opts, callback) { ... }

Object.create(this) is a clever way to preserve host-following behaviour, but it ties this code to internal implementation details of presto-client (the existence of request on the prototype, the string-vs-object dispatch, the host/port/protocol property names). The linked upstream PR (tagomoris/presto-client-node#97) is the right long-term fix — consider adding a TODO referencing it so this monkey-patch can be removed once upstream lands.

test/unit/headers.test.ts:104 — checkInterval: 1 cast as any
The checkInterval field isn't in PrestoDriverConfiguration because it's an upstream presto-client option spread through ...config. Either add it to the type or move the cast comment inline so a future reader doesn't think it's a typo.

Nits / praise

• Splitting into test/unit and test/integration with the matching unit script is a clean alignment with trino-driver — good housekeeping.
• The header-merge precedence ({ ...customHeaders, ...(opts.headers || {}) }) lets the client's own per-call headers (e.g. X-Presto-Transaction-Id) win, which is the right call.
• The test using a mocked follow-redirects transport — rather than stubbing the client — exercises the real client + driver wiring and would catch a future upstream change that broke the patch surface.

· [`fix/trino-driver-custom-headers`](https://github.com/cube-js/cube/tree/fix/trino-driver-custom-headers)

[claude]

Claude finished @ovr's task in 31s —— View job

---

Code review (re-run on sync)

Verdict: No code changes since the previous review — the new commit is a yarn.lock sync only. Prior feedback still stands.

Details — 0 high, 2 medium, 2 low (unchanged from prior review)

Checklist

• Diff origin/master..HEAD — only yarn.lock changed since last review
• Re-confirm prior findings still apply
• Update tracking comment

Medium

PrestoDriver.ts:187 — mutating caller's opts.headers

opts.headers = { ...customHeaders, ...(opts.headers || {}) };

Assigns a new object to opts.headers (does not mutate the original headers object), so safe in practice given current callers — but a brief comment or returning a fresh opts would prevent regressions if a future caller reuses opts.

PrestoDriver.ts:174-175 — protocol/port handling for the view

view.port = href.port || (href.protocol === 'https:' ? '443' : '80');
view.protocol = href.protocol;

Upstream presto-client selects between follow-redirects/http and follow-redirects/https based on this.ssl, not this.protocol. If a nextUri ever crosses schemes, setting view.protocol alone won't switch the transport. Worth overriding view.ssl too, or documenting that scheme switching is unsupported. The new test only covers same-scheme cross-host.

Low

PrestoDriver.ts:148 — monkey-patch couples to upstream internals
Object.create(this) + reassigning client.request is clever but ties the driver to internal details of presto-client. Consider a TODO referencing the upstream PR (tagomoris/presto-client-node#97) so this can be removed once it lands.

test/unit/headers.test.ts:104 — checkInterval: 1 as any
checkInterval is an upstream presto-client option spread via ...config and isn't on PrestoDriverConfiguration. Either extend the type or add an inline comment so the cast doesn't read as a typo.

Nits / praise

• test/unit vs test/integration split + new unit script matches trino-driver — good housekeeping.
• Header-merge precedence lets the client's per-call headers (e.g. X-Presto-Transaction-Id) win — correct call.
• Mocking follow-redirects rather than stubbing the client exercises the real client+driver wiring and would catch upstream changes that break the patch surface.

· fix/trino-driver-custom-headers ·

[codecov]

Codecov Report

❌ Patch coverage is 82.35294% with 3 lines in your changes missing coverage. Please review.
✅ Project coverage is 58.51%. Comparing base (44bd455) to head (d5f854c).

Files with missing lines	Patch %	Lines
...ackages/cubejs-prestodb-driver/src/PrestoDriver.ts	82.35%	1 Missing and 2 partials ⚠️

❗ There is a different number of reports uploaded between BASE (44bd455) and HEAD (d5f854c). Click for more details.

HEAD has 1 upload less than BASE

Flag	BASE (44bd455)	HEAD (d5f854c)
cubesql	1	0

Additional details and impacted files

@@             Coverage Diff             @@
##           master   #11037       +/-   ##
===========================================
- Coverage   78.78%   58.51%   -20.28%     
===========================================
  Files         471      216      -255     
  Lines       94509    17265    -77244     
  Branches     3520     3524        +4     
===========================================
- Hits        74463    10102    -64361     
+ Misses      19538     6648    -12890     
- Partials      508      515        +7     

Flag	Coverage Δ
cube-backend	58.51% <82.35%> (+0.06%)	⬆️
cubesql	?

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.