From 443cb5b89854c3f7b9db900ea07888f9b0f5de7a Mon Sep 17 00:00:00 2001 From: Arda TANRIKULU Date: Fri, 29 Jul 2022 16:29:45 +0300 Subject: [PATCH 1/4] Respect Accept headers --- packages/graphql-yoga/__tests__/node.spec.ts | 62 +++++++++++++++++++ .../src/plugins/resultProcessor/multipart.ts | 28 ++++++--- .../src/plugins/resultProcessor/push.ts | 28 ++++++--- .../src/plugins/resultProcessor/regular.ts | 16 +++-- packages/graphql-yoga/src/plugins/types.ts | 18 +++--- .../src/plugins/useResultProcessor.ts | 14 ++--- packages/graphql-yoga/src/processRequest.ts | 6 +- 7 files changed, 127 insertions(+), 45 deletions(-) diff --git a/packages/graphql-yoga/__tests__/node.spec.ts b/packages/graphql-yoga/__tests__/node.spec.ts index 2526a12adb..e3eaecbebb 100644 --- a/packages/graphql-yoga/__tests__/node.spec.ts +++ b/packages/graphql-yoga/__tests__/node.spec.ts @@ -1481,3 +1481,65 @@ describe('404 Handling', () => { expect(body).toEqual('Do you really like em?') }) }) + +describe('Respect Accept headers', () => { + const yoga = createYoga({ + schema, + }) + const server = createServer(yoga) + let port: number + let url: string + beforeAll((done) => { + port = Math.floor(Math.random() * 100) + 4000 + url = `http://localhost:${port}/graphql` + server.listen(port, done) + }) + afterAll(() => { + server.close() + }) + it('should force the server return event stream even if the result is not', async () => { + const response = await fetch(`${url}?query=query{ping}`, { + headers: { + Accept: 'text/event-stream', + }, + }) + expect(response.headers.get('content-type')).toEqual('text/event-stream') + const iterator = response.body![Symbol.asyncIterator]() + const { value } = await iterator.next() + const valueStr = Buffer.from(value).toString('utf-8') + expect(valueStr).toMatchInlineSnapshot(` + "data: {\\"data\\":{\\"ping\\":\\"pong\\"}} + + " + `) + }) + it('should force the server return multipart even if the result is not', async () => { + const response = await fetch(`${url}?query=query{ping}`, { + headers: { + Accept: 'multipart/mixed', + }, + }) + expect(response.headers.get('content-type')).toEqual( + 'multipart/mixed; boundary="-"', + ) + const iterator = response.body![Symbol.asyncIterator]() + const { value } = await iterator.next() + const valueStr = Buffer.from(value).toString('utf-8') + expect(valueStr).toMatchInlineSnapshot(` + "--- + Content-Type: application/json; charset=utf-8 + Content-Length: 24 + + {\\"data\\":{\\"ping\\":\\"pong\\"}} + ---" + `) + }) + it('should not allow to return if the result is an async iterable and accept is just json', async () => { + const response = await fetch(`${url}?query=subscription{counter}`, { + headers: { + Accept: 'application/json', + }, + }) + expect(response.status).toEqual(406) + }) +}) diff --git a/packages/graphql-yoga/src/plugins/resultProcessor/multipart.ts b/packages/graphql-yoga/src/plugins/resultProcessor/multipart.ts index c22f72d1ed..6a45c3b573 100644 --- a/packages/graphql-yoga/src/plugins/resultProcessor/multipart.ts +++ b/packages/graphql-yoga/src/plugins/resultProcessor/multipart.ts @@ -3,18 +3,13 @@ import { ExecutionResult } from 'graphql' import { ExecutionPatchResult, FetchAPI } from '../../types.js' import { ResultProcessorInput } from '../types.js' -export function isMultipartResult( - request: Request, - result: ResultProcessorInput, -): result is AsyncIterable { - return ( - isAsyncIterable(result) && - !!request.headers.get('accept')?.includes('multipart/mixed') - ) +export function isMultipartResult(request: Request): boolean { + // There should be an explicit accept header for this result type + return !!request.headers.get('accept')?.includes('multipart/mixed') } export function processMultipartResult( - executionPatchResultIterable: AsyncIterable, + result: ResultProcessorInput, fetchAPI: FetchAPI, ): Response { const headersInit: HeadersInit = { @@ -33,7 +28,20 @@ export function processMultipartResult( const readableStream = new fetchAPI.ReadableStream({ start(controller) { - iterator = executionPatchResultIterable[Symbol.asyncIterator]() + if (isAsyncIterable(result)) { + iterator = result[Symbol.asyncIterator]() + } else { + let finished = false + iterator = { + next: () => { + if (finished) { + return Promise.resolve({ done: true, value: null }) + } + finished = true + return Promise.resolve({ done: false, value: result }) + }, + } + } controller.enqueue(textEncoder.encode(`---`)) }, async pull(controller) { diff --git a/packages/graphql-yoga/src/plugins/resultProcessor/push.ts b/packages/graphql-yoga/src/plugins/resultProcessor/push.ts index 27e75e53b1..9d46613d6e 100644 --- a/packages/graphql-yoga/src/plugins/resultProcessor/push.ts +++ b/packages/graphql-yoga/src/plugins/resultProcessor/push.ts @@ -3,18 +3,13 @@ import { ExecutionResult } from 'graphql' import { FetchAPI } from '../../types.js' import { ResultProcessorInput } from '../types.js' -export function isPushResult( - request: Request, - result: ResultProcessorInput, -): result is AsyncIterable { - return ( - isAsyncIterable(result) && - !!request.headers.get('accept')?.includes('text/event-stream') - ) +export function isPushResult(request: Request): boolean { + // There should be an explicit accept header for this result type + return !!request.headers.get('accept')?.includes('text/event-stream') } export function processPushResult( - result: AsyncIterable, + result: ResultProcessorInput, fetchAPI: FetchAPI, ): Response { const headersInit: HeadersInit = { @@ -33,7 +28,20 @@ export function processPushResult( const textEncoder = new fetchAPI.TextEncoder() const readableStream = new fetchAPI.ReadableStream({ start() { - iterator = result[Symbol.asyncIterator]() + if (isAsyncIterable(result)) { + iterator = result[Symbol.asyncIterator]() + } else { + let finished = false + iterator = { + next: () => { + if (finished) { + return Promise.resolve({ done: true, value: null }) + } + finished = true + return Promise.resolve({ done: false, value: result }) + }, + } + } }, async pull(controller) { const { done, value } = await iterator.next() diff --git a/packages/graphql-yoga/src/plugins/resultProcessor/regular.ts b/packages/graphql-yoga/src/plugins/resultProcessor/regular.ts index 132039090d..fa1a33be29 100644 --- a/packages/graphql-yoga/src/plugins/resultProcessor/regular.ts +++ b/packages/graphql-yoga/src/plugins/resultProcessor/regular.ts @@ -1,17 +1,25 @@ import { isAsyncIterable } from '@graphql-tools/utils' -import { ExecutionResult } from 'graphql' +import { ExecutionResult, GraphQLError } from 'graphql' import { FetchAPI } from '../../types.js' import { ResultProcessorInput } from '../types.js' export function isRegularResult( request: Request, result: ResultProcessorInput, -): result is ExecutionResult { - return !isAsyncIterable(result) +): boolean { + if (!isAsyncIterable(result)) { + const acceptHeader = request.headers.get('accept') + if (acceptHeader) { + return acceptHeader.includes('application/json') + } + // If there is no header, assume it's a regular result per spec + return true + } + return false } export function processRegularResult( - executionResult: ExecutionResult, + executionResult: ResultProcessorInput, fetchAPI: FetchAPI, ): Response { const textEncoder = new fetchAPI.TextEncoder() diff --git a/packages/graphql-yoga/src/plugins/types.ts b/packages/graphql-yoga/src/plugins/types.ts index 22e9c2206f..0c3be3ae1b 100644 --- a/packages/graphql-yoga/src/plugins/types.ts +++ b/packages/graphql-yoga/src/plugins/types.ts @@ -70,21 +70,21 @@ export type OnResultProcess = ( payload: OnResultProcessEventPayload, ) => PromiseOrValue -export type ResultProcessorInput = PromiseOrValue< - ExecutionResult | AsyncIterable -> +export type ResultProcessorInput = + | ExecutionResult + | AsyncIterable + | AsyncIterable -export type ResultProcessor< - TResult extends ResultProcessorInput = ResultProcessorInput, -> = (result: TResult, fetchAPI: FetchAPI) => PromiseOrValue +export type ResultProcessor = ( + result: ResultProcessorInput, + fetchAPI: FetchAPI, +) => PromiseOrValue export interface OnResultProcessEventPayload { request: Request result: ResultProcessorInput resultProcessor?: ResultProcessor - setResultProcessor( - resultProcessor: ResultProcessor, - ): void + setResultProcessor(resultProcessor: ResultProcessor): void } export type OnResponseHook = ( diff --git a/packages/graphql-yoga/src/plugins/useResultProcessor.ts b/packages/graphql-yoga/src/plugins/useResultProcessor.ts index b55ea20c6c..d1d3eda47c 100644 --- a/packages/graphql-yoga/src/plugins/useResultProcessor.ts +++ b/packages/graphql-yoga/src/plugins/useResultProcessor.ts @@ -1,15 +1,13 @@ import { Plugin, ResultProcessor, ResultProcessorInput } from './types.js' -export interface ResultProcessorPluginOptions< - TResult extends ResultProcessorInput, -> { - processResult: ResultProcessor - match?(request: Request, result: ResultProcessorInput): result is TResult +export interface ResultProcessorPluginOptions { + processResult: ResultProcessor + match?(request: Request, result: ResultProcessorInput): boolean } -export function useResultProcessor< - TResult extends ResultProcessorInput = ResultProcessorInput, ->(options: ResultProcessorPluginOptions): Plugin { +export function useResultProcessor( + options: ResultProcessorPluginOptions, +): Plugin { const matchFn = options.match || (() => true) return { onResultProcess({ request, result, setResultProcessor }) { diff --git a/packages/graphql-yoga/src/processRequest.ts b/packages/graphql-yoga/src/processRequest.ts index 2876ffe17e..d90a60f9eb 100644 --- a/packages/graphql-yoga/src/processRequest.ts +++ b/packages/graphql-yoga/src/processRequest.ts @@ -21,7 +21,7 @@ export async function processResult({ */ onResultProcessHooks: OnResultProcess[] }) { - let resultProcessor: ResultProcessor | undefined + let resultProcessor: ResultProcessor | undefined for (const onResultProcessHook of onResultProcessHooks) { await onResultProcessHook({ @@ -79,7 +79,5 @@ export async function processRequest({ : enveloped.execute // Get the result to be processed - const result = await executeFn(executionArgs) - - return result + return executeFn(executionArgs) } From 321c6715144480a93b4677286e539cd448581138 Mon Sep 17 00:00:00 2001 From: Arda TANRIKULU Date: Fri, 29 Jul 2022 17:03:14 +0300 Subject: [PATCH 2/4] Finally --- packages/graphql-yoga/__tests__/node.spec.ts | 20 +++++++----------- .../src/plugins/resultProcessor/multipart.ts | 2 +- .../src/plugins/resultProcessor/regular.ts | 21 +++++++++++++++---- packages/graphql-yoga/src/server.ts | 8 +++---- 4 files changed, 29 insertions(+), 22 deletions(-) diff --git a/packages/graphql-yoga/__tests__/node.spec.ts b/packages/graphql-yoga/__tests__/node.spec.ts index e3eaecbebb..6a811f43e5 100644 --- a/packages/graphql-yoga/__tests__/node.spec.ts +++ b/packages/graphql-yoga/__tests__/node.spec.ts @@ -621,6 +621,7 @@ describe('Incremental Delivery', () => { body: formData, }) + expect(response.status).toBe(200) const body = await response.json() expect(body.errors).toBeUndefined() @@ -1507,11 +1508,9 @@ describe('Respect Accept headers', () => { const iterator = response.body![Symbol.asyncIterator]() const { value } = await iterator.next() const valueStr = Buffer.from(value).toString('utf-8') - expect(valueStr).toMatchInlineSnapshot(` - "data: {\\"data\\":{\\"ping\\":\\"pong\\"}} - - " - `) + expect(valueStr).toContain( + `data: ${JSON.stringify({ data: { ping: 'pong' } })}`, + ) }) it('should force the server return multipart even if the result is not', async () => { const response = await fetch(`${url}?query=query{ping}`, { @@ -1525,14 +1524,9 @@ describe('Respect Accept headers', () => { const iterator = response.body![Symbol.asyncIterator]() const { value } = await iterator.next() const valueStr = Buffer.from(value).toString('utf-8') - expect(valueStr).toMatchInlineSnapshot(` - "--- - Content-Type: application/json; charset=utf-8 - Content-Length: 24 - - {\\"data\\":{\\"ping\\":\\"pong\\"}} - ---" - `) + expect(valueStr).toContain(`Content-Type: application/json; charset=utf-8`) + expect(valueStr).toContain(`Content-Length: 24`) + expect(valueStr).toContain(`${JSON.stringify({ data: { ping: 'pong' } })}`) }) it('should not allow to return if the result is an async iterable and accept is just json', async () => { const response = await fetch(`${url}?query=subscription{counter}`, { diff --git a/packages/graphql-yoga/src/plugins/resultProcessor/multipart.ts b/packages/graphql-yoga/src/plugins/resultProcessor/multipart.ts index 6a45c3b573..58675eaa9a 100644 --- a/packages/graphql-yoga/src/plugins/resultProcessor/multipart.ts +++ b/packages/graphql-yoga/src/plugins/resultProcessor/multipart.ts @@ -1,6 +1,6 @@ import { isAsyncIterable } from '@envelop/core' import { ExecutionResult } from 'graphql' -import { ExecutionPatchResult, FetchAPI } from '../../types.js' +import { FetchAPI } from '../../types.js' import { ResultProcessorInput } from '../types.js' export function isMultipartResult(request: Request): boolean { diff --git a/packages/graphql-yoga/src/plugins/resultProcessor/regular.ts b/packages/graphql-yoga/src/plugins/resultProcessor/regular.ts index fa1a33be29..e48b69885c 100644 --- a/packages/graphql-yoga/src/plugins/resultProcessor/regular.ts +++ b/packages/graphql-yoga/src/plugins/resultProcessor/regular.ts @@ -1,20 +1,32 @@ import { isAsyncIterable } from '@graphql-tools/utils' -import { ExecutionResult, GraphQLError } from 'graphql' import { FetchAPI } from '../../types.js' import { ResultProcessorInput } from '../types.js' +const acceptHeaderByResult = new WeakMap() + export function isRegularResult( request: Request, result: ResultProcessorInput, ): boolean { if (!isAsyncIterable(result)) { const acceptHeader = request.headers.get('accept') - if (acceptHeader) { - return acceptHeader.includes('application/json') + if (acceptHeader && !acceptHeader.includes('*/*')) { + if (acceptHeader.includes('application/json')) { + acceptHeaderByResult.set(result, 'application/json') + return true + } + if (acceptHeader.includes('application/graphql+json')) { + acceptHeaderByResult.set(result, 'application/graphql+json') + return true + } + // If there is an accept header but this processer doesn't support, reject + return false } // If there is no header, assume it's a regular result per spec + acceptHeaderByResult.set(result, 'application/json') return true } + // If it is not an async iterable, it's not a regular result return false } @@ -25,8 +37,9 @@ export function processRegularResult( const textEncoder = new fetchAPI.TextEncoder() const responseBody = JSON.stringify(executionResult) const decodedString = textEncoder.encode(responseBody) + const contentType = acceptHeaderByResult.get(executionResult) const headersInit: HeadersInit = { - 'Content-Type': 'application/json', + 'Content-Type': contentType || 'application/json', 'Content-Length': decodedString.byteLength.toString(), } const responseInit: ResponseInit = { diff --git a/packages/graphql-yoga/src/server.ts b/packages/graphql-yoga/src/server.ts index 959bb18bd5..84bde881cb 100644 --- a/packages/graphql-yoga/src/server.ts +++ b/packages/graphql-yoga/src/server.ts @@ -387,16 +387,16 @@ export class YogaServer< }), // Middlewares after the GraphQL execution useResultProcessor({ - match: isRegularResult, - processResult: processRegularResult, + match: isMultipartResult, + processResult: processMultipartResult, }), useResultProcessor({ match: isPushResult, processResult: processPushResult, }), useResultProcessor({ - match: isMultipartResult, - processResult: processMultipartResult, + match: isRegularResult, + processResult: processRegularResult, }), ...(options?.plugins ?? []), From e782ca4695d71c4ad0325983d89ca82e63ee7957 Mon Sep 17 00:00:00 2001 From: Arda TANRIKULU Date: Fri, 29 Jul 2022 17:10:01 +0300 Subject: [PATCH 3/4] Wrong accept header in E2E tests :D --- e2e/utils.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/e2e/utils.ts b/e2e/utils.ts index 293ce47b31..78e15f88b4 100644 --- a/e2e/utils.ts +++ b/e2e/utils.ts @@ -111,7 +111,7 @@ export async function assertQuery( const response = await fetch(endpoint, { method: 'POST', headers: { - accept: 'applications/json', + accept: 'application/json', 'content-type': 'application/json', }, body: JSON.stringify({ From 454013c15e29edc18f75418d128acbc566bc58d3 Mon Sep 17 00:00:00 2001 From: Arda TANRIKULU Date: Fri, 29 Jul 2022 17:49:43 +0300 Subject: [PATCH 4/4] Changeset --- .changeset/more-accept-headers.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/more-accept-headers.md diff --git a/.changeset/more-accept-headers.md b/.changeset/more-accept-headers.md new file mode 100644 index 0000000000..51c55e47e8 --- /dev/null +++ b/.changeset/more-accept-headers.md @@ -0,0 +1,5 @@ +--- +'graphql-yoga': major +--- + +Now it is possible to decide the returned `Content-Type` by specifying the `Accept` header. So if `Accept` header has `text/event-stream` without `application/json`, Yoga respects that returns `text/event-stream` instead of `application/json`.