diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index e0de8924..fd1b3271 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -79,6 +79,13 @@ jobs: - 'src/**' - 'schemas/**' - 'tests/e2e/**' + # #630 Phase 1: tests/e2e/clickhouse-http-transport.spec.js now + # imports this ONE shared spike fixture server directly — a + # real cross-tree dependency the PR path filter didn't know + # about before. Deliberately narrow (not `tests/spike/**`): + # the rest of that historical spike suite is not a dependency + # of the root e2e suite and stays out of ordinary PR CI. + - 'tests/spike/clickhouse-client/fault-server.mjs' - 'playwright.config.js' - 'build/**' - 'package.json' diff --git a/CHANGELOG.md b/CHANGELOG.md index a8f6a087..77383d10 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,29 @@ auto-generated per-PR notes; this file is the curated, human-readable history. ## [Unreleased] +### Added +- **#630 Phase 1: characterize native Fetch/Response/cancellation semantics + ahead of the `@altinity/clickhouse-http` extraction.** No production + behavior changed — `src/net/clickhouse-http-transport.ts`, + `clickhouse-transport.types.ts`, and `ch-client.ts` are untouched. Strengthened + `tests/unit/clickhouse-transport-contract.ts` and + `tests/unit/clickhouse-http-transport.test.ts` to prove strict native + `Response` identity (2xx and non-2xx), no hidden body consumption, exactly + one injected Fetch call (including pre-aborted input), exact SQL/Authorization + fidelity, raw invalid-UTF-8 byte safety, live `origin()`/`fetch()` accessors, + and current URL-serialization literals (zero/empty/reserved values). Added a + real-browser Chromium/WebKit proof + (`tests/e2e/clickhouse-http-transport.{html,spec.js}`) covering pre-abort, + abort-while-awaiting-headers, native post-header body cancellation, + no-callbacks-after-cancellation, concurrent-request isolation, and + abort-after-completion — driven through the actual production transport, not + a synthetic stream. `tests/spike/clickhouse-client/fault-server.mjs` gained + byte-safe request-body capture, an opt-in CORS mode, and a deterministic + post-header-hold fixture to support this; `.github/workflows/ci.yml`'s e2e + path filter now includes that one shared fixture file. This is a + characterization-only unit (issue #630, phase 1 of 8); the package + extraction itself begins in phase 2. + ### Changed - **ADR-0005 reverted to Rejected (`@clickhouse/client-web` not adopted); briefly Accepted for part of one day.** A 2026-08-07 decision-methodology diff --git a/tests/e2e/clickhouse-http-transport.html b/tests/e2e/clickhouse-http-transport.html new file mode 100644 index 00000000..26f61cc9 --- /dev/null +++ b/tests/e2e/clickhouse-http-transport.html @@ -0,0 +1,305 @@ + + + + + #630 Phase 1 — native Fetch/Response/cancellation transport harness + + + + + + diff --git a/tests/e2e/clickhouse-http-transport.spec.js b/tests/e2e/clickhouse-http-transport.spec.js new file mode 100644 index 00000000..c6502c14 --- /dev/null +++ b/tests/e2e/clickhouse-http-transport.spec.js @@ -0,0 +1,197 @@ +import { randomUUID } from 'node:crypto'; +import { test, expect } from '@playwright/test'; +import { startFaultServer, POST_HEADER_ABORT_HOLD_MS } from '../spike/clickhouse-client/fault-server.mjs'; + +// #630 Phase 1 — freezes native Fetch/Response/cancellation semantics for the +// CURRENT `createHttpTransport` implementation, in real Chromium/WebKit, +// against a real cross-origin HTTP server (the shared spike fault server, +// started here in explicit browser/CORS mode). This spec owns the fault +// server's Node-side lifecycle: the root Playwright config only starts +// build/e2e-serve.mjs (the static/raw-ESM host on :5599) — it knows nothing +// about this ephemeral fixture server. Firefox cannot launch locally +// (repo-wide constraint); Chromium and WebKit are this phase's real +// acceptance signal, exactly as the plan requires. + +test.describe('#630 Phase 1 — native Fetch/Response/cancellation characterization', () => { + test.skip( + ({ browserName }) => browserName === 'firefox', + '#630 Phase 1 acceptance is explicitly Chromium/WebKit', + ); + + /** @type {Awaited>} */ + let fault; + + test.beforeAll(async () => { + fault = await startFaultServer({ cors: true }); + }); + + test.afterAll(async () => { + // Playwright still runs a describe's afterAll even when every test in it + // was test.skip()-ed (Firefox here) — but in that case beforeAll never + // ran, so `fault` is still undefined. Guard rather than let a skipped + // Firefox run fail on an unrelated hook error. + await fault?.close(); + }); + + test.beforeEach(async ({ page }) => { + await page.goto('/tests/e2e/clickhouse-http-transport.html'); + await page.waitForFunction(() => window.__ready === true); + }); + + // Unique per test/project so log-filtering assertions never depend on + // global log emptiness or ordering relative to any other test. + function qid(fixture, projectName) { + return `${fixture}__${projectName}-${randomUUID()}`; + } + + async function waitForServerRequest(predicate, timeoutMs = 5000) { + const start = Date.now(); + for (;;) { + if (fault.requestsLog.some(predicate)) return; + if (Date.now() - start > timeoutMs) throw new Error('timed out waiting for the expected fault-server request'); + await new Promise((r) => setTimeout(r, 20)); + } + } + + test('Scenario 1 — request and Response fidelity', async ({ page }, testInfo) => { + const queryId = qid('ordinary-query', testInfo.project.name); + const result = await page.evaluate( + ({ baseUrl, queryId }) => window.__scenario1(baseUrl, queryId), + { baseUrl: fault.baseUrl, queryId }, + ); + expect(result.identity).toBe(true); + expect(result.count).toBe(1); + expect(result.sqlMatches).toBe(true); + expect(result.authorization).toBe('Bearer test-token-abc123'); + expect(result.bodyUsedBeforeConsume).toBe(false); + expect(result.status).toBe(200); + expect(result.url).toContain('default_format=JSONCompact'); + + // A cross-origin POST with an Authorization header triggers a CORS + // preflight OPTIONS to the SAME URL first (logged too, body '') — filter + // to the actual POST so this corroborates the real request body/params. + const serverEntry = fault.requestsLog.find((e) => e.method === 'POST' && e.params && e.params.query_id === queryId); + expect(serverEntry).toBeTruthy(); + // Server-observed corroboration is independent end-to-end evidence, not + // the sole SQL proof — the exact transport boundary is asserted above + // via `result.sqlMatches` (the wrapper's captured `init.body`). + expect(serverEntry.body).toBe(' -- leading comment\n\tSELECT \'héllo\', 1 -- trailing comment\nFORMAT CSV; \n'); + expect(serverEntry.params.wait_end_of_query).toBe('0'); + expect(serverEntry.params.empty_setting).toBe(''); + expect(serverEntry.params.space_val).toBe('a b'); + expect(serverEntry.params.reserved_val).toBe('a&b=c?d#e'); + expect(serverEntry.params.empty_param).toBe(''); + }); + + test('Scenario 2 — non-2xx resolves untouched', async ({ page }, testInfo) => { + const queryId = qid('ch-non-2xx-shaped', testInfo.project.name); + const result = await page.evaluate( + ({ baseUrl, queryId }) => window.__scenario2(baseUrl, queryId), + { baseUrl: fault.baseUrl, queryId }, + ); + expect(result.identity).toBe(true); + expect(result.count).toBe(1); + expect(result.status).toBe(500); + expect(result.bodyUsedBeforeConsume).toBe(false); + expect(result.text).toBe('Code: 999. DB::Exception: synthetic non-2xx failure. (SYNTHETIC)'); + }); + + test('Scenario 3 — pre-aborted request invokes Fetch once but produces no server traffic', async ({ page }, testInfo) => { + const queryId = qid('ordinary-query', testInfo.project.name); + const result = await page.evaluate( + ({ baseUrl, queryId }) => window.__scenario3(baseUrl, queryId), + { baseUrl: fault.baseUrl, queryId }, + ); + expect(result.count).toBe(1); + expect(result.rejectedName).toBe('AbortError'); + const matching = fault.requestsLog.filter((e) => e.params && e.params.query_id === queryId); + expect(matching.length).toBe(0); // no OPTIONS, no POST — the browser never dispatched real traffic + }); + + test('Scenario 4 — abort while awaiting headers rejects AbortError', async ({ page }, testInfo) => { + const queryId = qid('slow-headers', testInfo.project.name); + await page.evaluate( + ({ baseUrl, queryId }) => window.__scenario4Start(baseUrl, queryId), + { baseUrl: fault.baseUrl, queryId }, + ); + // Real request dispatch happened (server logged the POST) — this is what + // distinguishes "cancelled before dispatch" (Scenario 3) from "cancelled + // while genuinely awaiting headers". + await waitForServerRequest((e) => e.method === 'POST' && e.params && e.params.query_id === queryId); + const result = await page.evaluate(() => window.__scenario4AbortAndAwait()); + expect(result.count).toBe(1); + expect(result.rejectedName).toBe('AbortError'); + }); + + test('Scenario 5 — native post-header body lifetime: pending read rejects AbortError, prior resolution stands', async ({ page }, testInfo) => { + const queryId = qid('post-header-abort-hold', testInfo.project.name); + const start = await page.evaluate( + ({ baseUrl, queryId }) => window.__scenario5Start(baseUrl, queryId), + { baseUrl: fault.baseUrl, queryId }, + ); + expect(start.identity).toBe(true); + expect(start.bodyUsedBeforeRead).toBe(false); + expect(start.firstDone).toBe(false); + expect(start.firstText).toContain('first'); + + const after = await page.evaluate(() => window.__scenario5AbortAndReadNext()); + expect(after.rejectedName).toBe('AbortError'); + // The already-settled send() Response is untouched by the later abort — + // no already-errored synthetic stream, no status/ok mutation. + expect(after.sendResponseStatus).toBe(200); + expect(after.sendResponseOk).toBe(true); + }); + + test('Scenario 6 — production streamLines() emits no callbacks after observable cancellation', async ({ page }, testInfo) => { + test.setTimeout(30_000); + const queryId = qid('post-header-abort-hold', testInfo.project.name); + const result = await page.evaluate( + ({ baseUrl, queryId, holdMs }) => window.__scenario6(baseUrl, queryId, holdMs), + { baseUrl: fault.baseUrl, queryId, holdMs: POST_HEADER_ABORT_HOLD_MS }, + ); + expect(result.rejectedName).toBe('AbortError'); + expect(result.chunksAtRejection).toBeGreaterThanOrEqual(1); + // Nothing changed across the wait spanning the fixture's held second + // write — the abort truly stopped the loop, not merely delayed it. + expect(result.linesAfterWait).toBe(result.linesAtRejection); + expect(result.chunksAfterWait).toBe(result.chunksAtRejection); + }); + + test('Scenario 7 — cancellation of one concurrent request cannot affect another sharing the same transport', async ({ page }, testInfo) => { + test.setTimeout(30_000); + const queryIdA = qid('post-header-abort-hold', testInfo.project.name); + const queryIdB = qid('post-header-abort-hold', testInfo.project.name); + const result = await page.evaluate( + ({ baseUrl, queryIdA, queryIdB, holdMs }) => window.__scenario7(baseUrl, queryIdA, queryIdB, holdMs), + { baseUrl: fault.baseUrl, queryIdA, queryIdB, holdMs: POST_HEADER_ABORT_HOLD_MS }, + ); + expect(result.aRejectedName).toBe('AbortError'); + expect(result.bFirstHeldDone).toBe(false); + expect(result.bFirstHeldText).toContain('after-hold'); + expect(result.bCompletedCleanly).toBe(true); + }); + + test('Scenario 8 — abort after full body completion has no effect', async ({ page }, testInfo) => { + const queryId = qid('ordinary-query', testInfo.project.name); + const result = await page.evaluate( + ({ baseUrl, queryId }) => window.__scenario8(baseUrl, queryId), + { baseUrl: fault.baseUrl, queryId }, + ); + expect(result.linesBefore).toBeGreaterThan(0); + expect(result.abortThrew).toBe(false); + expect(result.linesAfter).toBe(result.linesBefore); + expect(result.chunksAfter).toBe(result.chunksBefore); + }); + + test('Extra — invalid UTF-8 raw bytes remain byte-identical at the native boundary', async ({ page }, testInfo) => { + const queryId = qid('invalid-utf8-raw', testInfo.project.name); + const result = await page.evaluate( + ({ baseUrl, queryId }) => window.__scenarioInvalidUtf8(baseUrl, queryId), + { baseUrl: fault.baseUrl, queryId }, + ); + expect(result.identity).toBe(true); + expect(result.bodyUsedBeforeConsume).toBe(false); + expect(result.status).toBe(200); + expect(result.bytes).toEqual([0x61, 0x62, 0xff, 0xfe, 0x63, 0x00, 0x0a, 0x64]); + }); +}); diff --git a/tests/spike/clickhouse-client/fault-server.mjs b/tests/spike/clickhouse-client/fault-server.mjs index f25a669e..0c710e31 100644 --- a/tests/spike/clickhouse-client/fault-server.mjs +++ b/tests/spike/clickhouse-client/fault-server.mjs @@ -17,6 +17,11 @@ import { createServer } from 'node:http'; const EXCEPTION_MARKER = '__exception__'; +// #630 Phase 1 — see the 'post-header-abort-hold' fixture below. Exported so +// the e2e spec's own post-hold "assert no later callbacks" wait can size +// itself relative to this value instead of an independent magic number. +export const POST_HEADER_ABORT_HOLD_MS = 3000; + function sleep(ms) { return new Promise((resolve) => setTimeout(resolve, ms)); } @@ -40,18 +45,82 @@ function ndjson(obj) { // Attempt counters for the 401-then-success fixture, keyed by query_id. const attemptCounts = new Map(); -/** Start the fault server on an ephemeral loopback port. Returns +/** Start the fault server on an ephemeral loopback port. `opts.cors` (#630 + * Phase 1, default off — every pre-existing caller keeps today's behavior) + * additionally: answers CORS preflight for POST + Authorization, logs the + * OPTIONS request too, and stamps `Access-Control-Allow-Origin` on every + * actual response — required because the new root e2e transport spec loads + * this server cross-origin from the Playwright-served page. Returns * `{ server, port, baseUrl, requestsLog, close() }`. `requestsLog` accumulates * `{ method, url, headers }` for every request the request-inspection * scenarios assert against — `authorization` is recorded only as a redacted * boolean-shape summary (scheme + presence), never the raw header value * (plan §12 "avoid logging authorization values" / CLAUDE.md's credential * hygiene rule). */ -export function startFaultServer() { +export function startFaultServer(opts = {}) { + const { cors = false } = opts; const requestsLog = []; + // #630 Phase 1 review fix — test-only observability for the + // `res.on('error', ...)` suppression scoping below: records whether the + // most recently dispatched response registered an 'error' listener, so a + // spike `.ts` test can assert the handler is scoped to `cors: true` + // without importing `node:http` itself (plan §8 keeps Node-typed imports + // out of spike `.ts` files). Read via `getLastErrorListenerCount()`. + let lastErrorListenerCount = 0; const server = createServer(async (req, res) => { + if (cors) { + // #630 Phase 1: a client that aborts a cross-origin request (the + // native cancellation scenarios) tears down the underlying TCP + // connection while a fixture below is still `await sleep(...)`-ing + // between writes. The next `res.write()`/`res.end()` on that + // torn-down socket would otherwise surface as an uncaught 'error' + // event and crash this whole shared test server — swallow it. Scoped + // to the opt-in CORS path only: `cors` defaults off (see docstring + // above) and every pre-existing no-option caller + // (`parity.test.ts`/`run-matrix.mjs`) must keep today's behavior of + // NOT suppressing `ServerResponse` errors, so a real fixture/server + // failure there still surfaces instead of being silently hidden. + res.on('error', () => {}); + // Node's ServerResponse#writeHead always returns `this`; no call site + // below chains off its return value, so wrapping it to inject the + // header is transparent to every existing fixture branch. Node's real + // signature is `writeHead(status[, statusMessage][, headers])` — every + // call site in this file today uses the 2-arg `(status, headers)` + // form, but handle the 3-arg form too so a future fixture that passes + // a `statusMessage` can't have it silently mistaken for `headers`. + const nativeWriteHead = res.writeHead.bind(res); + res.writeHead = (status, ...rest) => { + if (rest.length >= 2) { + const [statusMessage, headers] = rest; + return nativeWriteHead(status, statusMessage, { 'access-control-allow-origin': '*', ...(headers || {}) }); + } + const [headers] = rest; + return nativeWriteHead(status, { 'access-control-allow-origin': '*', ...(headers || {}) }); + }; + } + lastErrorListenerCount = res.listenerCount('error'); const url = new URL(req.url, 'http://localhost'); + // A cross-origin POST carrying an Authorization header is never a + // CORS-simple request, so the browser sends a preflight OPTIONS first — + // to the exact same URL (including query string), so `query_id` is still + // present on it. Answer it before any fixture dispatch: preflight has no + // fixture behavior of its own. + if (cors && req.method === 'OPTIONS') { + requestsLog.push({ + method: 'OPTIONS', + pathname: url.pathname, + params: Object.fromEntries(url.searchParams.entries()), + headers: {}, + body: '', + }); + res.writeHead(204, { + 'access-control-allow-methods': 'POST, OPTIONS', + 'access-control-allow-headers': 'Authorization, Content-Type', + }); + res.end(); + return; + } // Both adapters always POST to the connection URL's root path (verified // empirically: the official client folds any URL path segment into a // ClickHouse `database` query param instead of preserving it as the HTTP @@ -65,8 +134,15 @@ export function startFaultServer() { // protocol-interpreted value. const queryId = url.searchParams.get('query_id') || ''; const fixture = queryId.split('__')[0] || ''; - let body = ''; - for await (const chunk of req) body += chunk; + // #630 Phase 1: byte-safe capture. `body += chunk` (the prior version) + // performs an implicit per-Buffer UTF-8 decode, so a multi-byte character + // split across two TCP chunks corrupts independently of anything the + // transport under test does — too weak for an exact-SQL server-observed + // proof. Collect raw Buffers and decode once, after every byte has + // arrived. + const bodyChunks = []; + for await (const chunk of req) bodyChunks.push(chunk); + const body = Buffer.concat(bodyChunks).toString('utf8'); const auth = req.headers.authorization; requestsLog.push({ @@ -269,6 +345,23 @@ export function startFaultServer() { res.end(); return; } + case 'post-header-abort-hold': { + // #630 Phase 1's native post-header cancellation-lifetime fixture + // (plan "Detailed browser scenarios" 5-7): headers plus one complete + // NDJSON row arrive in the same immediate write, so a real + // `reader.read()` for it resolves right away — then the NEXT chunk is + // held for POST_HEADER_ABORT_HOLD_MS, comfortably longer than + // Chromium/WebKit scheduling jitter, so the test can guarantee a real + // native second `read()` is genuinely pending when the original + // signal is aborted. The existing ~120ms gap in + // 'delayed-headers-scheduled-rows' is unnecessarily tight for that. + res.writeHead(200, { 'content-type': 'application/json' }); + res.write(ndjson({ row: { n: 'first' } })); + await sleep(POST_HEADER_ABORT_HOLD_MS); + res.write(ndjson({ row: { n: 'after-hold' } })); + res.end(); + return; + } case 'slow-headers': { // Headers themselves are delayed (plan §18 "cancel awaiting headers"; // §21 "timeout") — unlike 'delayed-headers-scheduled-rows', where @@ -367,6 +460,7 @@ export function startFaultServer() { baseUrl: `http://127.0.0.1:${port}`, requestsLog, resetAttemptCounts: () => attemptCounts.clear(), + getLastErrorListenerCount: () => lastErrorListenerCount, close: () => new Promise((res2) => server.close(() => res2())), }); }); diff --git a/tests/spike/clickhouse-client/parity.test.ts b/tests/spike/clickhouse-client/parity.test.ts index f473244b..93e9bc6e 100644 --- a/tests/spike/clickhouse-client/parity.test.ts +++ b/tests/spike/clickhouse-client/parity.test.ts @@ -1077,6 +1077,35 @@ describe('§16 runtime-surface experiment — literal cast-forced in isolation, }); }); +// PR review fix (#630 Phase 1): `fault-server.mjs`'s own docstring says +// `opts.cors` defaults off and every pre-existing no-option caller (this +// file, `run-matrix.mjs`) keeps today's behavior — but the response +// error-suppression handler had been wired unconditionally, ahead of the +// `if (cors)` branch, silently changing that behavior for every caller here. +// Assert the scoping directly via the fault server's own +// `getLastErrorListenerCount()` introspection (kept inside `fault-server.mjs` +// so this `.ts` file needs no `node:http` import, per plan §8). +describe('#630 Phase 1 review fix — ServerResponse error-suppression is scoped to cors:true', () => { + it('registers no ServerResponse error listener for a legacy no-option (cors:false, default) request', async () => { + const req = baseReq('ordinary-query'); + const current = await runCurrent(req, fault.baseUrl, fetch); + expect(current.outcome.error).toBeNull(); + expect(fault.getLastErrorListenerCount()).toBe(0); + }); + + it('registers exactly one ServerResponse error listener for an opt-in cors:true request', async () => { + const corsFault = await startFaultServer({ cors: true }); + try { + const req = baseReq('ordinary-query'); + const current = await runCurrent(req, corsFault.baseUrl, fetch); + expect(current.outcome.error).toBeNull(); + expect(corsFault.getLastErrorListenerCount()).toBe(1); + } finally { + await corsFault.close(); + } + }); +}); + function createOfficialConnectionWithFetch(baseUrl: string, fetchImpl: typeof fetch) { return createOfficialConnection(baseUrl, fetchImpl); } diff --git a/tests/unit/clickhouse-http-transport.test.ts b/tests/unit/clickhouse-http-transport.test.ts index 5fba628d..da9a724c 100644 --- a/tests/unit/clickhouse-http-transport.test.ts +++ b/tests/unit/clickhouse-http-transport.test.ts @@ -46,6 +46,48 @@ describe('chUrl', () => { expect(url).toContain('wait_end_of_query=1'); expect(url).toContain('x=a%20b'); }); + + // #630 Phase 1 — exact-literal zero/empty/reserved-value matrix (none of + // these are derived through chUrl() itself; each expected string is an + // independently authored literal, per the plan's failure/gap policy). + + it('serializes a numeric zero setting/param literally as 0, never omitted', () => { + expect(chUrl('https://o', { extra: { max_result_rows: 0 } })) + .toBe('https://o?default_format=JSONStringsEachRowWithProgress&enable_http_compression=1&max_result_rows=0'); + expect(chUrl('https://o', { params: { query_id: 0 } })) + .toBe('https://o?default_format=JSONStringsEachRowWithProgress&enable_http_compression=1&query_id=0'); + }); + + it('serializes an empty-string setting/param as a bare trailing "="', () => { + expect(chUrl('https://o', { extra: { session_id: '' } })) + .toBe('https://o?default_format=JSONStringsEachRowWithProgress&enable_http_compression=1&session_id='); + expect(chUrl('https://o', { params: { query_id: '' } })) + .toBe('https://o?default_format=JSONStringsEachRowWithProgress&enable_http_compression=1&query_id='); + }); + + it('percent-encodes spaces and reserved URL characters (& = ? # / %) in a setting/param value', () => { + const url = chUrl('https://o', { extra: { a: 'x y' }, params: { b: 'a&b=c?d#e/f%g' } }); + expect(url).toBe( + 'https://o?default_format=JSONStringsEachRowWithProgress&enable_http_compression=1' + + '&a=x%20y&b=a%26b%3Dc%3Fd%23e%2Ff%25g', + ); + }); + + it('serializes extra (settings) before params, each in its own object\'s key insertion order', () => { + const url = chUrl('https://o', { + extra: { z_setting: 1, a_setting: 2 }, + params: { z_param: 3, a_param: 4 }, + }); + expect(url).toBe( + 'https://o?default_format=JSONStringsEachRowWithProgress&enable_http_compression=1' + + '&z_setting=1&a_setting=2&z_param=3&a_param=4', + ); + }); + + it('always orders default_format before enable_http_compression, even with an explicit format override', () => { + expect(chUrl('https://o', { format: 'TabSeparated' })) + .toBe('https://o?default_format=TabSeparated&enable_http_compression=1'); + }); }); describe('createHttpTransport().send — exact request shape', () => { diff --git a/tests/unit/clickhouse-transport-contract.ts b/tests/unit/clickhouse-transport-contract.ts index cf6e95f5..bf6d12ee 100644 --- a/tests/unit/clickhouse-transport-contract.ts +++ b/tests/unit/clickhouse-transport-contract.ts @@ -79,14 +79,6 @@ export function runTransportContractSuite(name: string, makeTransport: MakeTrans expect(url).toBe('https://ch.example?default_format=JSON&enable_http_compression=1'); }); - it('carries each send\'s own authorization value with no state cached between sends', async () => { - const { transport, fetchMock } = harness(() => new Response('ok')); - await transport.send(baseRequest({ authorization: 'Bearer first' })); - await transport.send(baseRequest({ authorization: 'Bearer second' })); - expect((fetchMock.mock.calls[0][1] as RequestInit).headers as unknown as HeadersRecord).toEqual({ Authorization: 'Bearer first' }); - expect((fetchMock.mock.calls[1][1] as RequestInit).headers as unknown as HeadersRecord).toEqual({ Authorization: 'Bearer second' }); - }); - it('invokes the stub fetch exactly once per send — no internal retry or header caching — on a 2xx response', async () => { const { transport, fetchMock } = harness(() => new Response('ok', { status: 200 })); await transport.send(baseRequest()); @@ -99,17 +91,19 @@ export function runTransportContractSuite(name: string, makeTransport: MakeTrans expect(fetchMock).toHaveBeenCalledTimes(1); }); - it('resolves (never throws) on a non-2xx response, with the body reaching the caller byte-identical', async () => { - const { transport } = harness(() => new Response('{"exception":"Code: 60. DB::Exception: table not found"}', { status: 500 })); + it('resolves (never throws) on a non-2xx response, leaving bodyUsed === false until the caller consumes it, with the body then byte-identical', async () => { + const fetchResponse = new Response('{"exception":"Code: 60. DB::Exception: table not found"}', { status: 500 }); + const { transport } = harness(() => fetchResponse); const resp = await transport.send(baseRequest()); expect(resp.status).toBe(500); + expect(resp.bodyUsed).toBe(false); expect(await resp.text()).toBe('{"exception":"Code: 60. DB::Exception: table not found"}'); }); - it('rejects with the network/abort failure rather than resolving, for an aborted signal', async () => { + it('rejects with the network/abort failure rather than resolving, for an aborted signal, having still invoked the injected fetch exactly once', async () => { const controller = new AbortController(); controller.abort(); - const { transport } = harness((_url, init) => { + const { transport, fetchMock } = harness((_url, init) => { if ((init.signal as AbortSignal | undefined)?.aborted) { const err = Object.assign(new Error('aborted'), { name: 'AbortError' }); return Promise.reject(err); @@ -117,6 +111,80 @@ export function runTransportContractSuite(name: string, makeTransport: MakeTrans return Promise.resolve(new Response('ok')); }); await expect(transport.send(baseRequest({ signal: controller.signal }))).rejects.toMatchObject({ name: 'AbortError' }); + expect(fetchMock).toHaveBeenCalledTimes(1); + }); + + it('returns the exact native Response object from a 2xx Fetch — never a clone or wrapper', async () => { + const fetchResponse = new Response('ok', { status: 200 }); + const { transport } = harness(() => fetchResponse); + const resp = await transport.send(baseRequest()); + expect(resp).toBe(fetchResponse); + }); + + it('returns the exact native Response object from a non-2xx Fetch — never a clone or wrapper', async () => { + const fetchResponse = new Response('denied', { status: 403 }); + const { transport } = harness(() => fetchResponse); + const resp = await transport.send(baseRequest()); + expect(resp).toBe(fetchResponse); + }); + + it('sends an independently authored pathological SQL literal — leading/trailing whitespace, comments, tab/newline, an authored FORMAT clause, a trailing semicolon, and a non-ASCII codepoint — byte-identical as the POST body', async () => { + const { transport, fetchMock } = harness(() => new Response('ok')); + // Deliberately NOT derived from chUrl/the transport under test — an + // independently authored literal, per the plan's "do not derive + // expected SQL/URL/Auth values through the production helper under + // test" failure/gap policy. + const sql = ' -- leading comment\n\tSELECT \'héllo\', 1 -- trailing comment\nFORMAT CSV; \n'; + await transport.send(baseRequest({ sql, defaultFormat: 'JSON' })); + expect((fetchMock.mock.calls[0][1] as RequestInit).body).toBe(sql); + }); + + it('carries opaque Bearer, Basic, Digest, and a genuinely nonstandard scheme Authorization value verbatim across sequential sends — no scheme normalization, no cross-send caching', async () => { + const { transport, fetchMock } = harness(() => new Response('ok')); + const values = [ + 'Bearer tok-1', + 'Basic dXNlcjpwYXNz', + 'Digest realm="ch", nonce="abc", response="def"', + // A scheme name that is not in the IANA HTTP Authentication Scheme + // Registry (unlike Bearer/Basic/Digest above) — e.g. a custom SSO + // gateway's own token scheme. Catches an implementation that + // special-cases a closed allowlist of known schemes while mangling + // (rejecting, dropping, or rewriting) anything outside it: such an + // implementation would still pass a matrix built only from + // registered schemes. + 'XAuth opaque-value-1', + 'Bearer tok-2', // back to Bearer with a DIFFERENT value — proves no retained prior-header state + ]; + for (const authorization of values) { + await transport.send(baseRequest({ authorization })); + } + values.forEach((expected, i) => { + expect((fetchMock.mock.calls[i][1] as RequestInit).headers as unknown as HeadersRecord).toEqual({ Authorization: expected }); + }); + }); + + it('returns invalid-UTF-8 raw bytes byte-identical via arrayBuffer(), proving send() never decodes the body itself', async () => { + const bytes = new Uint8Array([0x61, 0x62, 0xff, 0xfe, 0x63, 0x00, 0x0a, 0x64]); + const { transport } = harness(() => new Response(bytes, { status: 200 })); + const resp = await transport.send(baseRequest()); + expect(resp.bodyUsed).toBe(false); + const buf = await resp.arrayBuffer(); + expect(new Uint8Array(buf)).toEqual(bytes); + }); + + it('reads deps.fetch() live per request instead of snapshotting it at construction time', async () => { + const fetchMockA = vi.fn(() => Promise.resolve(new Response('a'))); + const fetchMockB = vi.fn(() => Promise.resolve(new Response('b'))); + let current: FetchImpl = fetchMockA as unknown as FetchImpl; + const transport = makeTransport({ + fetch: () => current as unknown as typeof fetch, + origin: () => 'https://ch.example', + }); + await transport.send(baseRequest()); + current = fetchMockB as unknown as FetchImpl; + await transport.send(baseRequest()); + expect(fetchMockA).toHaveBeenCalledTimes(1); + expect(fetchMockB).toHaveBeenCalledTimes(1); }); it('surfaces a mid-stream abort from streamLines rather than swallowing it', async () => {