Skip to content

[absorbed into #630 Phase 1] freeze native Fetch, Response, and cancellation semantics #631

Description

@BorisTyshkevich

Part of #630.

Goal

Turn the existing Phase-1 transport contract from #585 into a complete behavioral specification for the first-party package, especially the browser cancellation behavior that the official-client spike failed to preserve.

This issue changes tests and test infrastructure only, except for the smallest production-only test seam needed to observe existing behavior without changing it. No transport refactor starts here.

Context

The existing tests/unit/clickhouse-transport-contract.ts already proves one Fetch invocation, native Response, non-2xx resolution, exact SQL body, Authorization passthrough, and no body consumption. Its current "mid-stream abort" case only injects a ReadableStream that is already errored; it does not prove that aborting the request's caller-owned signal after headers have arrived aborts the real browser response body.

That missing proof is the cancellation incompatibility that invalidated #585's official-client Phase 2.

Required contract

Characterize and lock these final semantics:

  1. send() / future package request() invokes the injected fetch exactly once for every call, including an already-aborted signal.
  2. The exact caller AbortSignal is passed to the actual Fetch request; no derived controller owns the network request.
  3. An already-aborted signal causes native Fetch rejection and no real server-side request, even though the injected fetch function is still invoked once.
  4. Abort while waiting for headers rejects with AbortError.
  5. Once headers resolve, the request promise remains resolved even if the signal aborts later.
  6. Abort after headers but before/during body consumption makes the real body read reject with AbortError.
  7. No row/chunk callback occurs after cancellation becomes observable to the stream consumer.
  8. Abort after body completion has no effect.
  9. Aborting request A cannot affect concurrent request B.
  10. The exact native Fetch Response object is returned by reference, not a clone or structural replacement.
  11. Non-2xx responses return the same native Response and leave bodyUsed === false.
  12. Raw bytes survive unchanged, including byte sequences that are not valid UTF-8.
  13. SQL body is exact: preserve leading/trailing whitespace, comments, trailing semicolons, authored FORMAT, and embedded newlines.
  14. Authorization is opaque: verify Bearer, Basic, and an arbitrary custom scheme/value.
  15. Current URL behavior remains characterized for default_format, enable_http_compression=1, settings, param_*, query_id, session_id, role, zero, empty string where currently representable, Unicode, spaces, &, =, %, and other encoded values.
  16. deps.origin() and deps.fetch() remain live per request rather than construction-time snapshots.

Deliverables

Extend the reusable transport contract suite

Strengthen tests/unit/clickhouse-transport-contract.ts without binding it to a future package implementation name.

Add strict identity assertions such as:

const expected = new Response(...);
const actual = await transport.send(...);
expect(actual).toBe(expected);

Add raw-byte coverage using bytes that would be lossy or replacement-decoded by Response.text().

Add real-browser Fetch cancellation coverage

Use the existing Phase-0 browser/fault-server infrastructure where possible rather than inventing a second server harness.

The server needs deterministic endpoints/scenarios for:

  • accept request and stall before headers;
  • send headers + first body chunk, then stall;
  • two concurrent streaming requests where one can be aborted independently;
  • record whether a pre-aborted request ever reached the server.

Run the browser cases in Chromium and WebKit locally through the repository's Playwright setup.

The critical post-header test must follow this order:

  1. start request with a caller-owned AbortController.signal;
  2. server sends headers and one chunk, then stalls;
  3. await send() resolves;
  4. begin reading response.body;
  5. abort the original controller;
  6. assert body consumption rejects with AbortError;
  7. assert no later chunks/rows are delivered;
  8. assert a concurrent request continues normally.

Do not replace this with an artificial already-errored ReadableStream; the proof must exercise browser Fetch end-to-end.

Preserve existing behavior

Do not add a manual pre-abort short-circuit merely to make the network test pass. The current contract calls the injected fetch once and relies on native Fetch to suppress the real network side effect for an already-aborted signal.

Likely files

  • tests/unit/clickhouse-transport-contract.ts
  • tests/unit/clickhouse-http-transport.test.ts
  • tests/spike/clickhouse-client/fault-server.mjs
  • tests/spike/clickhouse-client/browser-harness.ts
  • tests/spike/clickhouse-client/scenarios.ts
  • existing Playwright spec/config files under tests/spike/clickhouse-client/

Use the existing harness unless a focused helper extraction is required for deterministic tests.

Tests

Required new tests map directly to the 16 contract bullets above.

In addition to focused unit tests, run:

npm run test:client-spike:browser

or the repository's current equivalent that executes Chromium and WebKit for this harness.

Also run the full repository gate:

npm run check:types
npm run check:arch
npm run check:schemas
npm run check:examples
npm test
npm run build

Acceptance criteria

  • The reusable contract asserts strict native Response identity.
  • The contract proves low-level non-2xx responses remain unconsumed.
  • Exact SQL and opaque Authorization cases include edge values, not only the happy path.
  • Raw non-UTF-8 bytes are preserved without text conversion.
  • A real-browser test proves abort while awaiting headers rejects with AbortError.
  • A real-browser test proves abort after headers aborts an in-progress response body through the original caller signal.
  • A real-browser test proves concurrent-request cancellation isolation.
  • A real-browser test distinguishes "injected fetch invoked" from "network request reached server" for a pre-aborted signal.
  • Browser tests run in Chromium and WebKit.
  • No production behavior is intentionally changed.
  • Full repository gate and relevant browser tests pass.

Non-goals

  • Creating @altinity/clickhouse-http yet.
  • Refactoring chUrl, createHttpTransport, streamLines, ch-client.ts, or application services.
  • Changing pre-aborted semantics from the current exactly-one-injected-fetch contract.
  • Adding timeout behavior.
  • Changing retry, auth, epoch, session, or error policy.
  • Re-testing @clickhouse/client-web; ADR-0005: adopt @clickhouse/client-web behind the SQL Browser transport adapter #585 already records that rejected path.

Agent execution notes

Before planning, read #630, #585's current status/ADR addendum, src/net/clickhouse-transport.types.ts, src/net/clickhouse-http-transport.ts, the current contract suite, and the existing Phase-0 fault/browser harness. Reuse existing infrastructure and keep this PR characterization-only.

Metadata

Metadata

Assignees

No one assigned

    Labels

    refactorRestructuring without user-facing behavior changetech-debt

    Type

    No type

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions