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 error handling when having multiple nodes in query #286

Open
istvanfazakas opened this issue Oct 14, 2024 · 0 comments
Open

Fix error handling when having multiple nodes in query #286

istvanfazakas opened this issue Oct 14, 2024 · 0 comments

Comments

@istvanfazakas
Copy link

istvanfazakas commented Oct 14, 2024

I was using the gem in on of my previous projects, and while I was testing out multiple scenarios, I found a query where the error handling could be improved.
In case we are querying for multiple nodes, and one of the nodes fail, we receive the same error message for both nodes. In my example we are using encoded IDs, and the failure happens on the ID decoding.
Query example:

query Nodes {
  nodes(ids: [
    "VjEtSm9iLTMyMTUwNg" # valid:   ["User", "1234"]
    "VjEtSm9iLXF3ZQ"     # invalid: ["User", "qwe"]
  ]) {
    __typename
    ... on Node {
      id
    }
  }
}

As mentioned in the query above, one of the IDs is correct and can be decoded, the other one is incorrect, and when we try to decode the ID, it will raise an error.
As I mentioned, the same error will be returned for both nodes (which is incorrect):

{
  "errors": [
    {
      "message": "Invalid User id decoded: \"VjEtSm9iLXF3ZQ\" -> \"qwe\" (expected an integer)",
      "path": [
        "nodes",
        0
      ],
    },
    {
      "message": "Invalid User id decoded: \"VjEtSm9iLXF3ZQ\" -> \"qwe\" (expected an integer)",
      "path": [
        "nodes",
        1
      ],
    }
  ],
  "data": {
    "nodes": [
      null,
      null
    ]
  }
}

There might be a few possible solutions for this case:

  1. Return the data that can be fetched for one of the nodes, and return the error for the second one
  2. Return the error for the node that fails and do not return any data for the other one (maybe for security reasons?)
  3. Add a config for this, so we would be able to either do 1. or 2.

As a workaround I updated the code in ApolloFederation::EntitiesField._entities where I did rescue from StandardError, and simply just returned the error instead of raising it. That way the above mentioned 1. solution applies.
Inside the loop I just implemented it like:

rescue StandardError => e
  e.class.new(e.message)
@istvanfazakas istvanfazakas changed the title [IMPROVEMENT] - Improve error handling when having multiple nodes in query Fix error handling when having multiple nodes in query Nov 2, 2024
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

No branches or pull requests

1 participant