Skip to content

v6: fix transport error metadata, subscription teardown, and the spec accept header - #4429

Draft
trevor-scheer wants to merge 7 commits into
graphiql-6from
trevor/transport-correctness
Draft

v6: fix transport error metadata, subscription teardown, and the spec accept header#4429
trevor-scheer wants to merge 7 commits into
graphiql-6from
trevor/transport-correctness

Conversation

@trevor-scheer

@trevor-scheer trevor-scheer commented Jul 15, 2026

Copy link
Copy Markdown
Contributor

Summary

Behavioral fixes to the transport layer, plus two additive request fields, before Transport freezes at 6.0.0.

  • Error responses were discarding wire metadata. create-fetcher/lib.ts called response.json() unconditionally. A non-JSON error body (an HTML page from a proxy, a plain-text 401, an empty 204) made .json() throw, and the rejection collapsed into a generic error string exactly when the caller most needed status/statusText/headers. The body is now read as text first; if it parses as JSON it's used as-is, otherwise the raw text becomes the error message, but either way a real TransportResponse carrying the actual HTTP status comes back instead of a rejection. Reported response sizes use the raw wire text rather than a re-stringification of the parsed body.
  • Stopping a subscription didn't tear anything down. The wrapped transport (transport-hooks.ts#wrap) hands back an object whose [Symbol.asyncIterator]() mints a fresh iterator on every call. execution.ts's unsubscribe logic called that method a second time to get something to .return(), which built a throwaway iterator and closed it while the real one, still driving the loop, kept running untouched. Clicking Stop looked like it worked but the underlying socket or SSE connection just kept going. Both the transport and fetcher paths now capture the iterator once and drive/dispose that same object, feeding it to for await through a one-shot iterable so the loop's automatic .return() on abrupt completion (say, a throwing response handler) still disposes the right iterator. wrap() itself handles the disposal races: return() during an awaited onBeforeSend hook means the underlying send() is never invoked, and return() while a request is in flight drops the late value instead of delivering it.
  • Stopping a query or mutation didn't cancel anything either. run() creates an AbortController per request and passes its signal through TransportRequest.signal to the underlying fetch; stop() aborts it, and the aborted run stays silent instead of painting an abort error into the response pane. stop() also invalidates the run's query id, so even a custom transport that ignores the signal can't paint a late response into a pane the user already cleared, and starting a new run aborts whatever a previous run left in flight so a superseded request can neither paint results nor surface its errors over the newer run's. For streams (subscriptions, incremental delivery) the controller stays armed for the whole lifetime as an HTTP-level backstop — primary teardown is iterator disposal, but an aborted signal guarantees the connection dies even if some link in the disposal chain fails to forward .return().
  • The default transport wasn't asking for the spec media type. Incremental delivery is on by default, which routes through multipartHttpTransport, whose accept header was application/json, multipart/mixed with no application/graphql-response+json. Out of the box this asked spec-compliant servers to fall back to legacy response semantics. It now sends multipart/mixed, application/graphql-response+json;q=0.9, application/json;q=0.8 — explicit q-values so a server tie-break can't route an incremental response away from multipart, with the spec media type preferred among the JSON forms (matching the simple transport's application/graphql-response+json, application/json;q=0.9).
  • Added extensions and signal to TransportRequest. extensions rides along GraphQL-over-HTTP requests for things like persisted queries, and was already claimed (incorrectly) by a doc comment that never encoded it. It's JSON-stringified into the URL for GET and sent in the JSON body for POST and QUERY. signal is a plain AbortSignal, wired into run()/stop() as above. (Subscription Stop is the teardown fix above, not the abort signal — sockets need real teardown, not just an aborted fetch.)

A few judgment calls, made rather than left as an inconsistent matrix:

  • ok is now response.ok && !hasGraphQLErrors instead of only looking at the GraphQL body. A 401 or 500 was previously ok: true if the body happened to parse as JSON with no errors array, which collides with what Response.ok means everywhere else. Subscriptions have no HTTP status to consult, so their ok stays purely GraphQL-error-based.
  • Added an onError hook to the plugin transport context, alongside the existing onBeforeSend/onResponse. Without it a plugin had no way to observe a request that failed outright (network error, thrown onBeforeSend hook); for a frozen plugin API that asymmetry seemed like the likeliest regret to leave unaddressed. onError receives the request as transformed by whichever onBeforeSend hooks had run before the failure — the closest thing to what went on the wire — and does not fire for a request the user already stopped (an aborted fetch isn't a failure anyone is listening to). onResponse and onError are observe-only in the strong sense: a callback that throws is logged to the console and skipped, so one buggy plugin observer can't kill the stream or masquerade as a transport failure.
  • Dropped the reserved-but-unused ResolverTrace type and timing.resolverTraces field. Adding fields later is non-breaking, so reserving them now bought nothing and just shipped a field that always reads undefined.
  • Left wrap()'s hand-rolled async iterator alone rather than rewriting it as an async function*. The iterator-capture fix plus the disposal-race checks are the actual bugs; a generator rewrite is a real simplification but a separate, riskier change.

This PR also updates create-transport/README.md: documents the new extensions/signal fields and fixes a stale legacyClient reference (should be legacyWsClient).

Test plan

  • Point a transport at an endpoint that returns a non-JSON body with a 500 (or intercept /graphql in devtools and force an HTML/plain-text error response) and confirm the response pane shows a real 500 status badge, not a generic error message.
  • Start a subscription, click Stop, and check the Network tab: the WebSocket/EventSource connection actually closes rather than staying open in the background.
  • Run a query against a slow endpoint, click Stop while it's still in flight, and confirm the request is cancelled in the Network tab and the response pane stays empty (no fabricated abort error gets written into it).
  • Run a slow query, then immediately Run again: only the newer run's result ever lands in the response pane, and the first request shows as cancelled in the Network tab.
  • With the default transport config (incremental delivery on, nothing overridden), inspect the outgoing request headers in devtools and confirm Accept lists multipart/mixed first and includes application/graphql-response+json.
  • Send a request with extensions set on a Transport and confirm it shows up in the request (query string for GET, JSON body for POST/QUERY).
  • yarn workspace @graphiql/toolkit test && yarn workspace @graphiql/react test passes.

Refs: #4219

@changeset-bot

changeset-bot Bot commented Jul 15, 2026

Copy link
Copy Markdown

⚠️ No Changeset found

Latest commit: aded491

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes changesets to release 8 packages
Name Type
@graphiql/toolkit Minor
@graphiql/react Minor
graphiql Minor
@graphiql/plugin-history Major
@graphiql/plugin-code-exporter Major
@graphiql/plugin-collections Major
@graphiql/plugin-doc-explorer Major
@graphiql/plugin-query-builder Major

Click here to learn what changesets are, and how to add one.

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

@trevor-scheer
trevor-scheer force-pushed the trevor/transport-correctness branch from 0b32914 to aded491 Compare August 11, 2026 01:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant