Add timeouts to resolvers #1068
Conversation
| @@ -96,12 +97,14 @@ | |||
| "uuid": "^3.1.0" | |||
| }, | |||
| "resolutions": { | |||
| "babel-core": "^7.0.0-bridge.0" | |||
| "babel-core": "^7.0.0-bridge.0", | |||
| "@types/graphql": "^0.13.1" | |||
There was a problem hiding this comment.
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 🤷♂️
| } | ||
|
|
||
| type Query { | ||
| artwork: Artwork @timeout(ms: ${artworkTimeout}) |
There was a problem hiding this comment.
Here’s an example of explicit timeout for a resolver.
|
Note that this will not cancel side-effects of in process resolvers, it will only stop the resolver and return |
| @@ -0,0 +1,49 @@ | |||
| import { IMiddleware } from "graphql-middleware" | |||
There was a problem hiding this comment.
Yeah this is a very ugly (IMO) TS convention that indicates this is an interface. 🤷♂️
|
This looks great to me, nice tests! Interesting use of Let's hold this prior to merging since we want to test and deploy the new Datadog integration, currently on staging. |
|
Speaking of DataDog integration, is there some way to surface in DD that a query has timed out? Otherwise it may skew aggregates incorrectly? /cc @saolsen |
|
Here we go! |
Closes https://artsyproduct.atlassian.net/browse/PLATFORM-425
After going back and forth a few times on how to approach this (e.g. have a fixed overall timeout, but that’s weird because some queries legitimately take longer than others) I finally settled on the simplest solution, which is to have a timeout per resolver.
I’ve hardcoded a default of 5 seconds, but am open to changing that if people have opinions.
If resolvers need to deviate and take longer, they can technically do so by using a directive, except that will require generating a bit of AST because we don’t use the SDL to define out schema (dear future reader, ping me if you are in need of this).