Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix path for errors raised for federated nodes #988

Open
wants to merge 8 commits into
base: version-0.x
Choose a base branch
from

Conversation

lndbaryshnikov
Copy link

This is a solution for the problem described in issue #354.

In our project, we needed error paths to be correct in relation to the final api schema for the reasons described in the above issue (each service in the gateway can raise an error, for example, access error based on user permissions so we have to be able to map received errors to data).

This solution seems questionable in places, and I would be glad if you could help me improve it. The solution is already used in some of our projects and has not caused any particular problems yet.

The code in this PR solves 2 problems:

  1. It fixes path for errors raised for federated nodes, as described above (it composes correct error path by combining the path to the federated entity and the path of the original error);
  2. It generates errors with correct path for each federated entity in case of a network error occurs (the idea is that on the client we don't care about the federation and we want to know why we did not receive each specific field)

And I don't really know how to correctly test the case when one of the services is unavailable so this has not been covered by tests yet, I hope you'll help me with this.

@apollo-cla
Copy link

@lndbaryshnikov: 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/

@col
Copy link

col commented May 4, 2022

I'd really like to see this merged as I'm currently facing the same issue.

@lndbaryshnikov Do you have any issue signing the CLA?

@lndbaryshnikov
Copy link
Author

I'd really like to see this merged as I'm currently facing the same issue.

@lndbaryshnikov Do you have any issue signing the CLA?

@col no problems at all, I've signed the CLA
waiting for the review only

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants