-
Notifications
You must be signed in to change notification settings - Fork 90
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add timeouts to resolvers #1068
Changes from all commits
6fbe539
6d349eb
511814c
4ff65ba
54bedd9
11059d5
2fa2f67
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,200 @@ | ||
import { graphqlTimeoutMiddleware, fieldFromResolveInfo, timeoutForField } from "../graphqlTimeoutMiddleware" | ||
|
||
import { makeExecutableSchema, IResolvers } from "graphql-tools" | ||
import gql from "../../test/gql" | ||
import { graphql, buildSchema, GraphQLResolveInfo } from "graphql" | ||
import { applyMiddleware } from "graphql-middleware" | ||
|
||
/** | ||
* Combined with async/await, this function can be used to simply add a delay | ||
* between multiple expressions. | ||
*/ | ||
function delay(by: number): Promise<void> { | ||
return new Promise(resolve => setTimeout(resolve, by)) | ||
} | ||
|
||
describe("graphQLTimeoutMiddleware", () => { | ||
const defaultTimeout = 5 | ||
const artworkTimeout = defaultTimeout + 1 | ||
|
||
const typeDefs = gql` | ||
directive @timeout( | ||
ms: Int! | ||
) on FIELD_DEFINITION | ||
|
||
schema { | ||
query: Query | ||
} | ||
|
||
type Query { | ||
artwork: Artwork @timeout(ms: ${artworkTimeout}) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Here’s an example of explicit timeout for a resolver. |
||
} | ||
|
||
type Artwork { | ||
artists: [Artist] | ||
title: String | ||
} | ||
|
||
type Artist { | ||
name: String | ||
} | ||
` | ||
|
||
describe("fieldFromResolveInfo", () => { | ||
it("returns the field definition for the field resolver", () => { | ||
const schema = buildSchema(typeDefs) | ||
const resolveInfo = { | ||
fieldName: "artwork", | ||
parentType: schema.getQueryType(), | ||
} as GraphQLResolveInfo | ||
|
||
const artworkField = schema.getQueryType()!.getFields().artwork | ||
expect(fieldFromResolveInfo(resolveInfo)).toBe(artworkField) | ||
}) | ||
}) | ||
|
||
describe("timeoutForField", () => { | ||
function timeoutField(timeout: object | null) { | ||
const args = timeout && Object.keys(timeout).map(key => `${key}: ${timeout[key]}`).join(", ") | ||
const directive = timeout && `@timeout${args ? `(${args})` : ""}` || "" | ||
const schema = buildSchema(gql` | ||
schema { | ||
query: Query | ||
} | ||
type Query { | ||
field: String ${directive} | ||
} | ||
`) | ||
return schema.getQueryType()!.getFields().field | ||
} | ||
|
||
it("returns null if there was no timeout specified", () => { | ||
expect(timeoutForField(timeoutField(null))).toEqual(null) | ||
}) | ||
|
||
it("returns the specified timeout", () => { | ||
expect(timeoutForField(timeoutField({ ms: 42 }))).toEqual(42) | ||
}) | ||
|
||
it("throws an error if the directive is specified but no ms argument is given", () => { | ||
expect(() => timeoutForField(timeoutField({ sec: 42 }))).toThrowError(/argument is required/) | ||
}) | ||
|
||
it("throws an error if the directive is specified but no integer argument is given", () => { | ||
expect(() => timeoutForField(timeoutField({ ms: `"42"` }))).toThrowError(/Expected.+IntValue.+got.+StringValue/) | ||
}) | ||
}) | ||
|
||
describe("execution", () => { | ||
let resolvers: IResolvers | ||
|
||
const responseData = { | ||
title: "An artwork", | ||
artists: [ | ||
{ name: "An artist" }, | ||
{ name: "Another artist" }, | ||
], | ||
} | ||
|
||
function query() { | ||
const schema = makeExecutableSchema({ | ||
typeDefs, | ||
resolvers, | ||
}) | ||
applyMiddleware(schema, graphqlTimeoutMiddleware(defaultTimeout)) | ||
return graphql( | ||
schema, | ||
gql` | ||
query { | ||
artwork { | ||
title | ||
artists { | ||
name | ||
} | ||
} | ||
} | ||
` | ||
) | ||
} | ||
|
||
it("resolves if execution does not take longer than the set timeout", async () => { | ||
resolvers = { | ||
Query: { | ||
artwork: () => responseData, | ||
}, | ||
} | ||
expect(await query()).toEqual({ | ||
data: { | ||
artwork: responseData, | ||
}, | ||
}) | ||
}) | ||
|
||
it("returns `null` if execution of resolver with specific timeout takes longer than the set timeout", async () => { | ||
resolvers = { | ||
Query: { | ||
// Time out by going over the artworkTimeout | ||
artwork: async () => { | ||
await delay(artworkTimeout + 2) | ||
return responseData | ||
} | ||
}, | ||
} | ||
const response = await query() | ||
expect(response.data).toEqual({ artwork: null }) | ||
expect(response.errors!.length).toEqual(1) | ||
expect(response.errors![0].message).toMatch( | ||
/GraphQL Error: Query\.artwork has timed out after waiting for \d+ms/ | ||
) | ||
}) | ||
|
||
it("returns `null` if execution of resolvers takes longer than their timeout", async () => { | ||
resolvers = { | ||
Query: { | ||
artwork: () => ({}), | ||
}, | ||
Artwork: { | ||
title: () => responseData.title, | ||
artists: () => [{}, {}], | ||
}, | ||
Artist: { | ||
name: async () => { | ||
await delay(defaultTimeout + 2) | ||
return "Some artist" | ||
} | ||
} | ||
} | ||
const response = await query() | ||
expect(response.data).toEqual({ | ||
artwork: { | ||
title: responseData.title, | ||
artists: [{ name: null }, { name: null }] | ||
} | ||
}) | ||
expect(response.errors!.length).toEqual(2) | ||
expect(response.errors![0].message).toMatch( | ||
/GraphQL Error: Artist\.name has timed out after waiting for \d+ms/ | ||
) | ||
}) | ||
|
||
it("once a resolver timeout has passed, no nested resolvers will be invoked", async () => { | ||
const nestedResolver = jest.fn() | ||
resolvers = { | ||
Query: { | ||
// Time out by going over the artworkTimeout | ||
artwork: async () => { | ||
await delay(artworkTimeout + 2) | ||
return responseData | ||
} | ||
}, | ||
Artist: { | ||
// This resolver should not be called once the above has timed out | ||
name: nestedResolver, | ||
} | ||
} | ||
await query() | ||
await delay(artworkTimeout + 4) | ||
expect(nestedResolver).not.toBeCalled() | ||
}) | ||
}) | ||
}) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,49 @@ | ||
import { IMiddleware } from "graphql-middleware" | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 👋 hi middleware. IMatt There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah this is a very ugly (IMO) TS convention that indicates this is an interface. 🤷♂️ |
||
import { GraphQLResolveInfo, GraphQLField } from "graphql" | ||
import invariant from "invariant" | ||
|
||
export function fieldFromResolveInfo(resolveInfo: GraphQLResolveInfo) { | ||
return resolveInfo.parentType.getFields()[resolveInfo.fieldName] | ||
} | ||
|
||
export function timeoutForField(field: GraphQLField<any, any>) { | ||
const fieldDirectives = field.astNode && field.astNode.directives | ||
const directive = fieldDirectives && fieldDirectives.find(directive => directive.name.value === "timeout") | ||
if (directive) { | ||
const args = directive && directive.arguments | ||
const arg = args && args[0] | ||
invariant(arg && arg.name.value === "ms", "graphqlTimeoutMiddleware: The `@timeout(ms: …)` argument is required.") | ||
const value = arg!.value | ||
if (value.kind === "IntValue") { | ||
return parseInt(value.value) | ||
} else { | ||
invariant(false, `graphqlTimeoutMiddleware: Expected \`@timeout(ms: …)\` to be a \`IntValue\`, got \`${value.kind}\` instead.`) | ||
return null | ||
} | ||
} | ||
return null | ||
} | ||
|
||
export const graphqlTimeoutMiddleware = (defaultTimeoutInMS: number) => { | ||
const middleware: IMiddleware = ( | ||
middlewareResolver, | ||
parent, | ||
args, | ||
context, | ||
info | ||
) => { | ||
// TODO: Maybe cache if it turns out to take significant time. | ||
// Should probably be cached on the schema instance. | ||
const timeoutInMS = timeoutForField(fieldFromResolveInfo(info)) || defaultTimeoutInMS | ||
return Promise.race([ | ||
new Promise((_resolve, reject) => { | ||
setTimeout(() => { | ||
const field = `${info.parentType}.${info.fieldName}`; | ||
reject(new Error(`GraphQL Error: ${field} has timed out after waiting for ${timeoutInMS}ms`)) | ||
}, timeoutInMS) | ||
}), | ||
new Promise(resolve => resolve(middlewareResolver(parent, args, context, info))), | ||
]) | ||
} | ||
return middleware | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For some weird reason a few of our deps pull in this types package as a runtime dependency, which leads to there being multiple versions… It’s really a bug upstream, but whatevs, can’t fight every battle 🤷♂️