Gateway: pass through errors encountered when running final execute #4523
Conversation
|
@princed: Thank you for submitting a pull request! Before we can merge it, you'll need to sign the Apollo Contributor License Agreement here: https://contribute.apollographql.com/ |
|
Hi @princed!,
We’re in the process of moving federation-related concerns out of this repository (where it has historically lived) into a new home specifically for Apollo Federation. This PR is affected by the transition since it touches code which has been moved. I’ve done some preparations to make this as easy as possible, but we’ll need to close this PR, and I could use your help in re-opening it on the new Apollo Federation repository. In apollographql/federation#134 (which has a lot more details, if you’re interested), we introduced the same code that was in this repository to the new Federation repository (maintaining its history) and removed the code from this repository in #4561. While this PR still needs to get reviewed and approved to land, it should no longer live in this repository in its current state since it cannot merge in anymore. To facilitate the movement of this PR to the new home, I’ve automatically generated branches on the new repository using the same commit authorship and messages that you originally included on this PR. Pull-requests can’t be moved on GitHub in the same way Issues can be. While I could just create the PRs from these new branches too, the contribution belongs to you! Original PR author only: Click this button to open a new PR from the auto-created (You will have an opportunity to confirm, and this way you can continue to be the author and track its progress on the new Federation repository!) There’s no easy way to bring over any existing comments on this PR, so I encourage you to copy and paste those onto the new issue if possible. Overall, I hope this process is relatively easy for you while maintaining your commit contribution authorship. I apologize for any inconvenience caused by this shuffle, but please ping me if I can help in any way! 🚀 |
|
Thank you @abernix, this the best migration I've ever seen! |
|
Thanks for being the first to test it. 😉 Hoping we can get your PR reviewed on the other side! |
|
Thanks! Looking forward to hearing from reviewers 😄 |
…xecute (#159)" This reverts commit 9045f55, which was from #159. See the comment in #981 which explains the justification as well, but briefly: While that PR was an improvement in principle, it: - has a bug that was first captured in #159 (comment) To quote that here: > I think there's a bug here. Let's say we have a query `{ x }` where `x` is a non-nullable field, and the federated execution has `x` throw an error. This turns all of `data` into `null` (not `{ x: null }`). But then this re-execution adds another error saying that the non-null field is null. > > Take a look at https://codesandbox.io/s/angry-raman-uuzuf?file=/src/index.js for an example of what's going on here. - Sometimes — unexpectedly to current users, at least — breaks client expectations. As of now, the pain points seem to be outweighing the gains. We'll need to revert this and revisit this when time allows with a slightly different approach and #981 tracks the need to do so. Ref: #159 Ref: apollographql/apollo-server#5550 Ref: #974 Ref: apollographql/apollo-server#4523
…xecute (#159)" (#982) This reverts commit 9045f55, which was from #159. See the comment in #981 which explains the justification as well, but briefly: While that PR was an improvement in principle, it: - has a bug that was first captured in #159 (comment) To quote that here: > I think there's a bug here. Let's say we have a query `{ x }` where `x` is a non-nullable field, and the federated execution has `x` throw an error. This turns all of `data` into `null` (not `{ x: null }`). But then this re-execution adds another error saying that the non-null field is null. > > Take a look at https://codesandbox.io/s/angry-raman-uuzuf?file=/src/index.js for an example of what's going on here. - Sometimes — unexpectedly to current users, at least — breaks client expectations. As of now, the pain points seem to be outweighing the gains. We'll need to revert this and revisit this when time allows with a slightly different approach and #981 tracks the need to do so. Ref: #159 Ref: apollographql/apollo-server#5550 Ref: #974 Ref: apollographql/apollo-server#4523
Fixes a problem where
@apollo/gatewayhides errors encountered when running finalexecute()fromgraphql-js, which can potentially both throw (e.g. from here) and returnerrorsarray in following cases: