fix: improve handling of promises in defaultTypeResolver
Currently it is possible to have unhandled promise rejections that arise from a mix of sync and async isTypeOf checks.
This should fix that issue.
This means that a mixture of sync/async calls becomes sync unnecessarily, as we return sync value…. Maybe catch handlers should be added to the promises so they reject silently? Is that possible?
I had considered that, but silently swallowing errors seems like a bad idea. I think it makes sense for execution to become async if any async functions are called. This should be a pretty rare case, but if these errors were swallowed silently it could become very tricky to debug.
Maybe there should be a side channel for logging these errors then so as they are not silent? In fact, this can be addressed currently in user space, as long as isTypeOf functions catch their errors and log them…
this differs from field or type resolvers which must never reject, but it’s not quite a problem for an isTypeOf resolver fail if another succeeds.
not 100% sure what the right answer is
A side channel might be useful as well in context of https://github.com/graphql/graphql-js/issues/2974 to report the first error to client and potentially log all errors
But as there currently is no side channel, I think I would approve these changes, and then refactor later
The latest changes of this PR are available on NPM as graphql@16.3.0-canary.pr.3494.25e1e8c8de6b13288ac61febf75d6f415f81a044 Note: no gurantees provided so please use your own discretion.
Also you can depend on latest version built from this PR:
npm install --save graphql@canary-pr-3494
Deploy Preview for compassionate-pike-271cb3 ready!
| Name | Link |
|---|---|
| Latest commit | 68a04f2b9a69d9f0ced6c555e45cee5bae5aef2b |
| Latest deploy log | https://app.netlify.com/sites/compassionate-pike-271cb3/deploys/64fddef2b40a23000876ade7 |
| Deploy Preview | https://deploy-preview-3494--compassionate-pike-271cb3.netlify.app |
| Preview on mobile | Toggle QR Code...Use your smartphone camera to open QR code link. |
To edit notification comments on pull requests, go to your Netlify site configuration.
kinda forgot about this PR, but I think this change would still be good to get in