Skip to content

Commit

Permalink
fix: handle 400 responses for missing persisted queries
Browse files Browse the repository at this point in the history
  • Loading branch information
sastan committed Jul 16, 2020
1 parent 7f51692 commit 760818b
Show file tree
Hide file tree
Showing 5 changed files with 158 additions and 15 deletions.
63 changes: 63 additions & 0 deletions packages/graphql/src/exchanges/automatic-persisted-queries.test.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
/* eslint-disable @typescript-eslint/no-unsafe-assignment */
import type { Response } from '../types.dom'
import { automaticPersistedQueries } from './automatic-persisted-queries'
import { GraphQLFetchError } from './fetch'

test('already persisted query', async () => {
const query = '{ viewer { name } }'
Expand Down Expand Up @@ -103,6 +105,67 @@ test('persisted query not found', async () => {
)
})

test('persisted query not found (code: 400)', async () => {
const query = '{ viewer { name } }'
const response = { data: { viewer: { name: 'X' } } }
const next = jest
.fn()
.mockRejectedValueOnce(
new GraphQLFetchError({ status: 400 } as Response, {
errors: [{ message: 'PersistedQueryNotFound' }],
}),
)
.mockResolvedValue(response)

const apq = automaticPersistedQueries()

const result = await apq(
{
operation: {
id: 1,
type: 'query',
name: undefined,
},
query,
variables: {},
extensions: {},
options: {
headers: {},
signal: new AbortController().signal,
},
},
next,
() => {
throw new Error('no update')
},
)

expect(result).toBe(response)

expect(next).toHaveBeenCalledTimes(2)

expect(next.mock.calls[0]).toMatchObject([
{
query: '',
extensions: {
persistedQuery: { version: -1, fnv1a128Hash: 'bd05d1f7d9d0529cfba185ac3ca46a22' },
},
},
])

expect(next).toHaveBeenLastCalledWith(
expect.objectContaining({
query,
extensions: expect.objectContaining({
persistedQuery: { version: -1, fnv1a128Hash: 'bd05d1f7d9d0529cfba185ac3ca46a22' },
}),
options: expect.not.objectContaining({
preferGetForQueries: expect.anything(),
}),
}),
)
})

test('persisted query not supported', async () => {
const query = '{ viewer { name } }'
const response = { data: { viewer: { name: 'X' } } }
Expand Down
15 changes: 13 additions & 2 deletions packages/graphql/src/exchanges/automatic-persisted-queries.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
import type { GraphQLExchange } from '../types'
import type { GraphQLExchange, GraphQLServerResult } from '../types'
import { fnv1a128 } from '../internal/fnv1a'
import { GraphQLFetchError } from './fetch'
import { isString } from '../internal/is'

export interface AutomaticPersistedQuery extends Record<string, string | number> {
version: number
Expand Down Expand Up @@ -57,6 +59,7 @@ const automaticPersistedQueriesExchange = ({
}

const persistedQuery = cachedPersistedQuery(query)

const result = await next({
...request,
query: '',
Expand All @@ -65,7 +68,15 @@ const automaticPersistedQueriesExchange = ({
...options,
preferGetForQueries: preferGETForHashedQueries || options.preferGetForQueries,
},
})
}).catch(
(error: GraphQLFetchError): GraphQLServerResult => {
if (error.message === notFoundError || error.message === notSupportedError) {
return { errors: [error] }
}

throw error
},
)

if (result.errors) {
// If the server doesn't support persisted queries, don't try anymore
Expand Down
58 changes: 58 additions & 0 deletions packages/graphql/src/exchanges/fetch.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -112,3 +112,61 @@ test('prefer get for queries', async () => {
headers: { Accept: 'application/json' },
})
})

test('send 400 with errors', async () => {
const uri = 'http://test.local/graphql'

const query = `query {
hero(episode: $episode) {
name
heroFriends: friends {
id
name
}
}
}`

const variables = { episode: 10 }

fetchMock.post(uri, {
status: 400,
body: { errors: [{ message: 'PersistedQueryNotFound' }], data: null },
})

const fetch = fetchExchange({ uri })

const result = fetch(
{
operation: {
id: 1,
type: 'query',
name: undefined,
},
query,
variables,
extensions: {},
options: {
headers: {},
signal: new AbortController().signal,
},
},
() => {
throw new Error('no next')
},
() => {
throw new Error('no update')
},
)

await expect(result).rejects.toThrow('PersistedQueryNotFound')
await expect(result).rejects.toMatchObject({
status: 400,
body: { errors: [{ message: 'PersistedQueryNotFound' }], data: null },
})

expect(fetchMock.lastOptions(uri)).toMatchObject({
method: 'POST',
headers: { 'Content-Type': 'application/json', Accept: 'application/json' },
body: JSON.stringify({ query, variables }),
})
})
36 changes: 23 additions & 13 deletions packages/graphql/src/exchanges/fetch.ts
Original file line number Diff line number Diff line change
@@ -1,18 +1,22 @@
import type { Response } from '../types.dom'
import type { GraphQLExchange, GraphQLRequestOptions } from '../types'
import type { Response, Headers } from '../types.dom'
import type { GraphQLExchange, GraphQLRequestOptions, GraphQLServerResult } from '../types'
import stableStringify from 'fast-json-stable-stringify'
import { isString } from '../internal/is'

export class GraphQLFetchError extends Error {
readonly url: string
readonly request: GraphQLRequestOptions
readonly response: Response
readonly status: number
readonly body: string | GraphQLServerResult
readonly headers: Headers

constructor(url: string, request: GraphQLRequestOptions, response: Response, message?: string) {
super(`fetch failed (code: ${response.status}): ${message || response.statusText}`)
constructor(response: Response, body: string | GraphQLServerResult) {
super(
(isString(body) ? body : (body.errors || [])[0]?.message) ||
`[${response.status}] ${response.statusText}`,
)
this.name = 'GraphQLFetchError'
this.url = url
this.request = request
this.response = response
this.status = response.status
this.body = body
this.headers = response.headers
}
}

Expand All @@ -22,6 +26,9 @@ const emptyToUndefined = <T extends Record<string, unknown>>(

const MAX_URL_LENGTH = 2000

const isJsonResponse = (response: Response): boolean | undefined =>
response.headers.get('Content-Type')?.startsWith('application/json')

/**
* A default exchange for fetching GraphQL requests.
*/
Expand Down Expand Up @@ -56,7 +63,7 @@ const fetchExchange = (config: GraphQLRequestOptions = {}): GraphQLExchange => a
// Add all defined args as search parameters
for (const [key, value] of Object.entries(args)) {
if (value) {
url.searchParams.set(key, typeof value === 'string' ? value : stableStringify(value))
url.searchParams.set(key, isString(value) ? value : stableStringify(value))
}
}

Expand All @@ -77,11 +84,14 @@ const fetchExchange = (config: GraphQLRequestOptions = {}): GraphQLExchange => a
})
}

if (response.ok && response.headers.get('Content-Type')?.startsWith('application/json')) {
if (response.ok && isJsonResponse(response)) {
return response.json()
}

throw new GraphQLFetchError(uri, init, response, await response.text())
throw new GraphQLFetchError(
response,
await (isJsonResponse(response) ? response.json() : response.text()),
)
}

return next()
Expand Down
1 change: 1 addition & 0 deletions packages/graphql/src/internal/is.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export const isString = (value: unknown): value is string => typeof value === 'string'

0 comments on commit 760818b

Please sign in to comment.