-
Notifications
You must be signed in to change notification settings - Fork 2.1k
Closed
Labels
Description
I was just looking over the source code around polymorphism and I think I found an issue here:
graphql-js/src/execution/execute.ts
Lines 1000 to 1018 in 7dd7812
const isTypeOfResult = type.isTypeOf(value, contextValue, info); | |
if (isPromise(isTypeOfResult)) { | |
promisedIsTypeOfResults[i] = isTypeOfResult; | |
} else if (isTypeOfResult) { | |
return type.name; | |
} | |
} | |
} | |
if (promisedIsTypeOfResults.length) { | |
return Promise.all(promisedIsTypeOfResults).then((isTypeOfResults) => { | |
for (let i = 0; i < isTypeOfResults.length; i++) { | |
if (isTypeOfResults[i]) { | |
return possibleTypes[i].name; | |
} | |
} | |
}); | |
} |
Specifically: if we're looking over types A, B and C, and A yields a promise, and B returns a match, we will return B.name
and the promise we added to promisedIsTypeOfResults
for A will never be handled. If that promise were to reject then Node would raise an unhandled promise rejection error.
I think we can solve this by adding a line above return type.name
:
} else if (isTypeOfResult) {
+ // Ignore errors from promises already created
+ if (promisedIsTypeOfResults.length) {
+ void Promise.allSettled(promisedIsTypeOfResults);
+ }
return type.name;
}