-
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;
}