Implement graphql-over-http spec

.changeset/spec-compliant-fetch.md

@@ -0,0 +1,6 @@
+---
+'houdini-core': patch
+'houdini': patch
+---
+
+The built-in fetch plugin now follows the GraphQL-over-HTTP spec.

docs/shared/01-core/04-client.mdx

@@ -98,7 +98,7 @@ export default new HoudiniClient({
 
 Houdini's default pipeline is built from plugins exported from `$houdini/plugins`. You can import any of these to use in a custom `pipeline`:
 
-- `fetch`: resolves the pipeline by sending a standard HTTP request. The default terminating plugin. Accepts an optional handler function for fully custom request logic.
+- `fetch`: resolves the pipeline by sending a standard HTTP request following the [GraphQL-over-HTTP spec](https://graphql.github.io/graphql-over-http/draft/). The default terminating plugin. Accepts an optional handler function for fully custom request logic.
 - `query`: core behavior for queries, establishing cache subscriptions and accumulating variables.
 - `mutation`: core behavior for mutations, including optimistic responses.
 - `subscription`: core behavior for subscriptions. Accepts a `SubscriptionHandler` to wire up a WebSocket client such as `graphql-ws`.

packages/houdini-core/runtime/plugins/cache.test.ts

@@ -1,9 +1,10 @@
 import { beforeEach, expect, test, vi } from 'vitest'
 
-import { testConfigFile } from '../../test'
-import { Cache } from '../cache/cache'
-import { CachePolicy, PendingValue } from '../lib'
-import { setMockConfig } from '../lib/config'
+import { Cache } from 'houdini/runtime/cache'
+import { CachePolicy, PendingValue } from 'houdini/runtime/types'
+import { testConfigFile } from 'houdini/test'
+
+import { setMockConfig } from '../config'
 import { cachePolicy } from './cache.js'
 import { createStore, fakeFetch } from './test.js'
 
@@ -424,8 +425,28 @@ test('NoCache', async () => {
 })
 
 test('loading states when fetching is true', async () => {
-	// create the store
-	const store = createStore()
+	// create the store with the same setFetching wiring that client.observe sets up:
+	// when the cache plugin flags a network request, the store's state gets the
+	// generated loading data
+	let storeRef: ReturnType<typeof createStore> | null = null
+	const store = createStore({
+		pipeline: [
+			cachePolicy({
+				serverSideFallback: false,
+				enabled: true,
+				cache: new Cache({ ...config, disabled: false }),
+				setFetching: (fetching: boolean, data?: any) => {
+					storeRef?.update((state) => ({
+						...state,
+						fetching,
+						...(fetching && data ? { data } : {}),
+					}))
+				},
+			}),
+			fakeFetch({}),
+		],
+	})
+	storeRef = store
 
 	// listen for changes in the store state
 	const fn = vi.fn()

packages/houdini-core/runtime/plugins/fetch.test.ts

@@ -0,0 +1,174 @@
+import { testConfigFile } from 'houdini/test'
+import { beforeEach, expect, test, vi } from 'vitest'
+
+import { setMockConfig } from '../config'
+import { fetch as fetchPlugin } from './fetch.js'
+import { createStore } from './test.js'
+
+beforeEach(async () => {
+	setMockConfig(testConfigFile())
+})
+
+function fakeResponse({
+	body,
+	status = 200,
+	contentType = 'application/graphql-response+json',
+	statusText = '',
+}: {
+	body: any
+	status?: number
+	contentType?: string | null
+	statusText?: string
+}) {
+	return vi.fn(async () => {
+		return new Response(typeof body === 'string' ? body : JSON.stringify(body), {
+			status,
+			statusText,
+			headers: contentType ? { 'Content-Type': contentType } : {},
+		})
+	})
+}
+
+test('sends a spec-compliant GraphQL-over-HTTP request', async () => {
+	const fetchMock = fakeResponse({ body: { data: { viewer: null } } })
+	const store = createStore({ pipeline: [fetchPlugin()] })
+
+	await store.send({ fetch: fetchMock, variables: { id: '1' } })
+
+	expect(fetchMock).toHaveBeenCalledOnce()
+	const [url, args] = fetchMock.mock.calls[0] as unknown as [string, RequestInit]
+	// the client resolves the default local endpoint from config
+	expect(String(url).endsWith('/_api')).toBe(true)
+	expect(args.method).toEqual('POST')
+	expect(args.headers).toMatchObject({
+		// the spec requires clients to include application/graphql-response+json
+		Accept: 'application/graphql-response+json, application/json;q=0.9',
+		'Content-Type': 'application/json',
+	})
+	expect(JSON.parse(args.body as string)).toEqual({
+		operationName: 'TestArtifact',
+		query: 'RAW_TEXT',
+		variables: { id: '1' },
+	})
+})
+
+test('surfaces request errors sent as 4xx with the graphql-response media type', async () => {
+	// a spec-compliant server responds to a validation failure with a 422 and the
+	// details in the errors list
+	const store = createStore({ pipeline: [fetchPlugin()] })
+	const result = await store.send({
+		fetch: fakeResponse({
+			body: { errors: [{ message: 'Cannot query field "foo"' }] },
+			status: 422,
+			contentType: 'application/graphql-response+json; charset=utf-8',
+		}),
+	})
+
+	expect(result.errors).toEqual([{ message: 'Cannot query field "foo"' }])
+	expect(result.data).toBeUndefined()
+})
+
+test('parses partial success responses independent of status code', async () => {
+	// the spec recommends the custom 294 status when data and errors are both present
+	const store = createStore({ pipeline: [fetchPlugin()] })
+	const result = await store.send({
+		fetch: fakeResponse({
+			body: { data: { viewer: null }, errors: [{ message: 'field error' }] },
+			status: 294,
+		}),
+	})
+
+	expect(result.data).toEqual({ viewer: null })
+	expect(result.errors).toEqual([{ message: 'field error' }])
+})
+
+test('parses application/json responses from legacy servers', async () => {
+	const store = createStore({ pipeline: [fetchPlugin()] })
+	const result = await store.send({
+		fetch: fakeResponse({
+			body: { data: { viewer: { id: '1' } } },
+			contentType: 'application/json',
+		}),
+	})
+
+	expect(result.data).toEqual({ viewer: { id: '1' } })
+})
+
+test('parses responses using the withdrawn pre-spec media type', async () => {
+	const store = createStore({ pipeline: [fetchPlugin()] })
+	const result = await store.send({
+		fetch: fakeResponse({
+			body: { errors: [{ message: 'bad request' }] },
+			status: 400,
+			contentType: 'application/graphql+json',
+		}),
+	})
+
+	expect(result.errors).toEqual([{ message: 'bad request' }])
+})
+
+test('does not read response headers on successful responses', async () => {
+	// SvelteKit's SSR fetch throws when a response header outside
+	// filterSerializedResponseHeaders is read, so the happy path must never touch them
+	const store = createStore({ pipeline: [fetchPlugin()] })
+	const result = await store.send({
+		fetch: vi.fn(async () => {
+			const response = new Response(JSON.stringify({ data: { viewer: null } }))
+			return new Proxy(response, {
+				get(target, prop) {
+					if (prop === 'headers') {
+						throw new Error(
+							'Failed to get response header — it must be included by the `filterSerializedResponseHeaders` option'
+						)
+					}
+					const value = Reflect.get(target, prop)
+					return typeof value === 'function' ? value.bind(target) : value
+				},
+			})
+		}),
+	})
+
+	expect(result.data).toEqual({ viewer: null })
+})
+
+test('throws on JSON error responses that are not GraphQL responses', async () => {
+	// an intermediary (a rate limiter, a gateway) can send a JSON body that parses fine
+	// but has no data or errors entry to surface
+	const store = createStore({ pipeline: [fetchPlugin()] })
+	await expect(
+		store.send({
+			fetch: fakeResponse({
+				body: { message: 'rate limited' },
+				status: 429,
+				statusText: 'Too Many Requests',
+				contentType: 'application/json',
+			}),
+		})
+	).rejects.toThrow('Failed to fetch: server returned invalid response with error 429')
+})
+
+test('throws a useful error when the response body is not valid JSON', async () => {
+	const store = createStore({ pipeline: [fetchPlugin()] })
+	await expect(
+		store.send({
+			fetch: fakeResponse({
+				body: '<html>not json</html>',
+				contentType: 'application/json',
+			}),
+		})
+	).rejects.toThrow('Failed to fetch: server returned a malformed response with status 200')
+})
+
+test('throws on error responses that are not GraphQL media types', async () => {
+	const store = createStore({ pipeline: [fetchPlugin()] })
+	await expect(
+		store.send({
+			fetch: fakeResponse({
+				body: '<html>Service Unavailable</html>',
+				status: 503,
+				statusText: 'Service Unavailable',
+				contentType: 'text/html',
+			}),
+		})
+	).rejects.toThrow('Failed to fetch: server returned invalid response with error 503')
+})

packages/houdini-core/runtime/plugins/fetch.ts

@@ -116,7 +116,10 @@ const defaultFetch = (
 			body: JSON.stringify({ operationName: name, query: text, variables }),
 			...params,
 			headers: {
-				Accept: 'application/graphql+json, application/json',
+				// the GraphQL-over-HTTP spec requires clients to include
+				// application/graphql-response+json in the Accept header. application/json is
+				// included at a lower priority for legacy servers
+				Accept: 'application/graphql-response+json, application/json;q=0.9',
 				'Content-Type': 'application/json',
 				// a header a cross-origin <form>/simple request cannot set. The server
 				// requires it for CORS-simple POSTs to the graphql endpoint (uploads use
@@ -127,18 +130,50 @@ const defaultFetch = (
 			},
 		})
 
-		// Avoid parsing the response if it's not JSON, as that will throw a SyntaxError
-		if (
-			!result.ok &&
-			!result.headers.get('content-type')?.startsWith('application/json') &&
-			!result.headers.get('content-type')?.startsWith('application/graphql+json')
-		) {
+		// a response served with a GraphQL media type is a well-formed GraphQL response
+		// regardless of status code (the spec sends request errors like validation
+		// failures as 4xx with the details in the errors list) so it always gets parsed.
+		// anything else that isn't a 2xx is a transport-level failure we can't interpret.
+		// only look at the content-type on failures: SvelteKit's SSR fetch throws when a
+		// header outside filterSerializedResponseHeaders is read, so the happy path must
+		// not touch headers
+		if (!result.ok) {
+			const contentType = result.headers.get('content-type') ?? ''
+			const isGraphQLResponse =
+				contentType.startsWith('application/graphql-response+json') ||
+				contentType.startsWith('application/json') ||
+				// some servers still use the withdrawn pre-spec media type
+				contentType.startsWith('application/graphql+json')
+			if (!isGraphQLResponse) {
+				throw new Error(
+					`Failed to fetch: server returned invalid response with error ${result.status}: ${result.statusText}`
+				)
+			}
+		}
+
+		let payload
+		try {
+			payload = await result.json()
+		} catch {
+			throw new Error(
+				`Failed to fetch: server returned a malformed response with status ${result.status}: ${result.statusText}`
+			)
+		}
+
+		// a JSON error response from an intermediary (a rate limiter, a gateway) parses fine
+		// but isn't a GraphQL response. If an error response has neither a data nor an errors
+		// entry there is nothing for the pipeline to surface, so treat it as a transport failure
+		const isGraphQLPayload =
+			payload !== null &&
+			typeof payload === 'object' &&
+			('data' in payload || 'errors' in payload)
+		if (!result.ok && !isGraphQLPayload) {
 			throw new Error(
 				`Failed to fetch: server returned invalid response with error ${result.status}: ${result.statusText}`
 			)
 		}
 
-		return await result.json()
+		return payload
 	}
 }
 

packages/houdini-core/runtime/plugins/optimisticKeys.test.ts

@@ -4,7 +4,7 @@ import { ArtifactKind, type QueryResult, type GraphQLObject } from 'houdini/runt
 import { testConfigFile } from 'houdini/test'
 import { beforeEach, expect, test } from 'vitest'
 
-import { setMockConfig } from '../lib/config'
+import { setMockConfig } from '../config'
 import { mutation } from './mutation.js'
 import { optimisticKeys } from './optimisticKeys.js'
 import { createStore, fakeFetch } from './test.js'

packages/houdini-core/runtime/plugins/query.test.ts

@@ -2,8 +2,9 @@ import { Cache } from 'houdini/runtime/cache'
 import { CachePolicy } from 'houdini/runtime/types'
 import { beforeEach, expect, test, vi } from 'vitest'
 
-import { testConfigFile } from '../../test'
-import { setMockConfig } from '../lib/config'
+import { testConfigFile } from 'houdini/test'
+
+import { setMockConfig } from '../config'
 import { query } from './query.js'
 import { createStore, fakeFetch } from './test.js'
 
@@ -32,7 +33,7 @@ test('refetch triggered by cache.refresh uses the most recent session, not the s
 		},
 	}
 
-	cache._internal_unstable.write({
+	cache.write({
 		selection,
 		data: { viewer: { id: '1', firstName: 'bob' } },
 	})
@@ -65,7 +66,7 @@ test('refetch triggered by cache.refresh uses the most recent session, not the s
 	fetchSpy.mockClear()
 
 	// trigger a refetch via the cache
-	cache._internal_unstable.refresh('User:1')
+	cache.refresh('User:1')
 
 	// give the async send a tick to run
 	await new Promise((r) => setTimeout(r, 0))

packages/houdini-core/runtime/plugins/test.ts

@@ -1,9 +1,13 @@
 import type { ClientPlugin, ClientPluginContext } from 'houdini/runtime/documentStore'
 import { DocumentStore } from 'houdini/runtime/documentStore'
 import type { DocumentArtifact, GraphQLObject, QueryResult } from 'houdini/runtime/types'
+import { ArtifactKind, DataSource } from 'houdini/runtime/types'
 import { vi } from 'vitest'
 
-import { createPluginHooks, HoudiniClient, type HoudiniClientConstructorArgs } from '..'
+import { createPluginHooks } from 'houdini/runtime/client'
+
+import { HoudiniClient, type HoudiniClientConstructorArgs } from '..'
+import { getCurrentConfig } from '../config'
 
 /**
  * Utilities for testing the cache plugin
@@ -17,15 +21,12 @@ export function createStore(
 	}
 
 	// instantiate the client
-	const client = new HoudiniClient({
-		url: 'URL',
-		...args,
-	})
+	const client = new HoudiniClient(args)
 
 	return new DocumentStore({
-		plugins: args.plugins ? createPluginHooks(client.plugins) : undefined,
-		pipeline: args.pipeline ? createPluginHooks(client.plugins) : undefined,
+		plugins: createPluginHooks(client.plugins),
 		client,
+		config: getCurrentConfig(),
 		artifact: args.artifact ?? {
 			stripVariables: [],
 			kind: ArtifactKind.Query,

vite.config.ts

@@ -19,7 +19,7 @@ export default defineConfig({
 			'./packages/houdini-react/runtime/**/*.test.{ts,js}',
 			'./packages/houdini-react/package/**/*.test.{ts,js}',
 			'./packages/houdini-svelte/package/**/*.test.{ts,js}',
-			'./packages/houdini-core/runtime/public/**/*.test.{ts,js}',
+			'./packages/houdini-core/runtime/**/*.test.{ts,js}',
 			'./site/**/*.test.{ts,js}',
 		],
 		projects: ['.', 'packages/sv-addon'],