Comments (11)
Having almost same problem. I try to use type of "Customer" which fetch some data from SQL and some from MongoDb.
export let customerType = new GraphQLObjectType({
name: 'customerType',
description: 'Customer schema',
fields: {
id: {type: GraphQLInt},
createdAt: {type: GraphQLString},
updatedAt: {type: GraphQLString},
deletedAt: {type: GraphQLString},
firstName: {type: GraphQLString},
lastName: {type: GraphQLString},
email: {type: GraphQLString},
password: {type: GraphQLString},
...
...
...
creditCardExpDate: {type: GraphQLString},
numLogins: {type: GraphQLInt},
quiz: {
type: new GraphQLList(quizType),
resolve: (THIS) => Customers.quiz({id: THIS.id})
}
}
});
All fields comes from SQL besides "quiz", resolve method for "quiz" return Mongoose promise which then return array of object. But on this query:
{
getCustomers(id: 45) {
id
firstName
quiz {
message
recommendedProducts
}
}
}
I get:
{
"data": {
"getCustomers": [
{
"id": 45,
"firstName": "Genesis",
"quiz": [
{
"message": null,
"recommendedProducts": null
}
]
}
]
}
}
Any solution so far?
from graphql-js.
Alright, well, I think I have a reasonable handle on the codebase now, thanks to a lot of node-inspectoring ;).
Here's a much simpler proof of the bug:
import {
graphql,
GraphQLSchema,
GraphQLObjectType,
GraphQLString,
GraphQLNonNull,
GraphQLList,
} from 'graphql';
var orgType = new GraphQLObjectType({
name: 'Org',
fields: () => ({
id: {
type: new GraphQLNonNull(GraphQLString),
resolve: () => {
return Promise.resolve(Math.random + '');
}
}
})
});
var schema = new GraphQLSchema({
query: new GraphQLObjectType({
name: 'RootQueryType',
fields: {
orgs: {
type: new GraphQLList(orgType),
resolve: () => {
return [{ id: 1 }]
}
}
}
}),
});
var query = `
query Foo {
orgs {
id,
},
}
`
graphql(schema, query).then(result => {
console.log('RESULT', JSON.stringify(result, null, 2));
}).catch(console.log);
Which results in
RESULT {
"data": {
"orgs": [
{
}
]
}
}
but if you change orgType.id.resolve to just return a value, you get
RESULT {
"data": {
"orgs": [
{
"id": "0.8567653454374522"
}
]
}
}
as expected.
Here's what's happening:
- Because org is a list type, https://github.com/graphql/graphql-js/blob/master/src/executor/executor.js#L599-L603 we hit this line of code, where we're mapping over the list of orgs, and completing fields for each one.
- But each of those ends up returning a promise
- This means that resolveFieldOrError ends up returning something that looks like
[ Promise ]
- Which ends up here: https://github.com/graphql/graphql-js/blob/master/src/executor/executor.js#L470-L476
- Since we have an array with promises in it, we should be waiting for promise resolution, but we don't since an array doesn't pass the isThenable check.
- This means our field ends up just getting the array of promises, and since promises serialize to
{}
we end up with[ {} ]
in our result.
Fix?
The simplest fix would be to add the following between these two lines: https://github.com/graphql/graphql-js/blob/master/src/executor/executor.js#L469-L470 but I don't know the codebase well enough to know if this should move into isThenable in case there are similar issues elsewhere? Or if it should be in resolveFieldOrError maybe?
//convert Array(<Promise>), to Promise(<Array>)
if (Array.isArray(result) && result.some(isThenable)) {
result = Promise.all(result);
}
Thoughts? I can try and add a test to the star wars stuff that demonstrates this if required once I have a little more time.
from graphql-js.
Thanks for your in-depth exploration! There are some minor issues with your fix surrounding nullable vs non-nullable error handling, but it's directionally sound. I'll investigate further.
from graphql-js.
So here's the result of my further analysis:
The library hadn't supported this yet not as a bug but because we assumed if a resolve function would return a list of promises, that the resolve function itself would be responsible for calling Promise.all
, the way we use GraphQL at Facebook has resulted in this particular way of describing field resolution being underserved.
As I was thinking about this further, I think we're also missing a case where a list without non-null item type [T]
should handle a single item failure more gracefully.
So a list of promises:
resolve: () => [ fooPromise(), failingPromise(), barPromise() ]
should result in [ "foo", null, "bar" ]
where I'm pretty sure this currently will bubble up the error and become null
.
More investigation and test writing is necessary, but thanks again for highlighting!
from graphql-js.
Thanks! I'm not sure I agree though, in this case my resolve functions are only ever returning individual promises, but it's the lib itself which is creating an array of promises when it maps over a list type, no?
Philip Roberts
On 4 Jul 2015, at 16:54, Lee Byron [email protected] wrote:
So here's the result of my further analysis:
The library hadn't supported this yet not as a bug but because we assumed if a resolve function would return a list of promises, that the resolve function itself would be responsible for calling Promise.all, the way we use GraphQL at Facebook has resulted in this particular way of describing field resolution being underserved.
As I was thinking about this further, I think we're also missing a case where a list without non-null item type [T] should handle a single item failure more gracefully.
So a list of promises:
resolve: () => [ fooPromise(), failingPromise(), barPromise() ]
should result in [ "foo", null, "bar" ] where I'm pretty sure this currently will bubble up the error and become null.More investigation and test writing is necessary, but thanks again for highlighting!
—
Reply to this email directly or view it on GitHub.
from graphql-js.
Ah yes, I think there are two subtle overlapping issues here. I see from your test case that there is another failure to resolve the promise before continuing. Thanks for providing them, I'll make sure they get encoded as runtime unit tests.
from graphql-js.
Thanks @leebyron! Yeah, it was quite hard to produce a minimal testcase that demonstrated the behaviour, while still demonstrating the intent :)
from graphql-js.
@latentflip please check out 0fd8be1 and let me know if the test cases I added are aligning with the issues you ran into.
from graphql-js.
Version 0.1.2 is on npm that includes that commit
from graphql-js.
@leebyron yup, just ran it on the test-cases + orm integration I've been working on and it's looking good. Thanks for the quick turnaround! 🎉
from graphql-js.
Thank YOU for playing with it in such an early state and investigating. I may not have found this issue otherwise.
from graphql-js.
Related Issues (20)
- IHeyReally.org
- Suggestion: Bundling in v17, ESM, CJS, and the dual package hazard HOT 9
- Just to give my 2c, I'm not sure if `exports.development` and `exports.production` target conditionsa are widely supported, so some `import.env` shenanigans may still be necessary until that's widely adopted. HOT 1
- Collection of libraries and how they import from `graphql` HOT 20
- `process.env`, `globalThis`, and `typeof process` HOT 21
- Introspection queries don't support `@oneOf` HOT 2
- Tutorial data HOT 2
- astFromValue fails with a custom scalar serializing to an object value HOT 5
- In a response to a query about an Issue, the URL and other info is missing for links created with Reference editor button HOT 5
- author/committer -> user fields returning NULL for commits committed by user
- [email protected]
- npm link with graphql package breaks application HOT 2
- IHeyReally.com HOT 1
- Can I ask what is the progress here? Is there a solution being worked on? Do we have some timeline? Or progress with issue? Thanks!
- IHeyReally.com
- Typescript error with 16.9.0 (re ThunkObjMap) HOT 3
- Notice: default branch is now `16.x.x` HOT 2
- <a href="https://api.easycla.lfx.linuxfoundation.org/v2/repository-provider/github/sign/12261526/38307428/4135/#/?version=2"><img src="https://s3.amazonaws.com/cla-project-logo-prod/cla-not-signed.svg" alt="CLA Not Signed" align="left" height="28" width="328"></a><br/><br /><ul><li><a href='https://api.easycla.lfx.linuxfoundation.org/v2/repository-provider/github/sign/12261526/38307428/4135/#/?version=2' target='_blank'>:x:</a> - login: @Heyitsquoracom / name: [email protected] . The commit (b6081f914f0c5c22ee48d26aff4b473bf17627ce) is not authorized under a signed CLA. <a href='https://api.easycla.lfx.linuxfoundation.org/v2/repository-provider/github/sign/12261526/38307428/4135/#/?version=2' target='_blank'>Please click here to be authorized</a>. For further assistance with EasyCLA, <a href='https://jira.linuxfoundation.org/servicedesk/customer/portal/4' target='_blank'>please submit a support request ticket</a>.</li></ul><!-- Date Modified: 2024-06-28 04:40:51.630768 -->
- Error with version v16.9.0: `Can't resolve '@apollo/subgraph/dist/directives''` HOT 1
- Type is incorrect in graphql ExecutionResult with new type GraphQLFormattedError HOT 1
Recommend Projects
-
React
A declarative, efficient, and flexible JavaScript library for building user interfaces.
-
Vue.js
🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.
-
Typescript
TypeScript is a superset of JavaScript that compiles to clean JavaScript output.
-
TensorFlow
An Open Source Machine Learning Framework for Everyone
-
Django
The Web framework for perfectionists with deadlines.
-
Laravel
A PHP framework for web artisans
-
D3
Bring data to life with SVG, Canvas and HTML. 📊📈🎉
-
Recommend Topics
-
javascript
JavaScript (JS) is a lightweight interpreted programming language with first-class functions.
-
web
Some thing interesting about web. New door for the world.
-
server
A server is a program made to process requests and deliver data to clients.
-
Machine learning
Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.
-
Visualization
Some thing interesting about visualization, use data art
-
Game
Some thing interesting about game, make everyone happy.
Recommend Org
-
Facebook
We are working to build community through open source technology. NB: members must have two-factor auth.
-
Microsoft
Open source projects and samples from Microsoft.
-
Google
Google ❤️ Open Source for everyone.
-
Alibaba
Alibaba Open Source for everyone
-
D3
Data-Driven Documents codes.
-
Tencent
China tencent open source team.
from graphql-js.