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

Querying a field that is required but fails throws inside apollo client #10345

Closed
AndreasHald opened this issue Dec 9, 2022 · 7 comments · Fixed by #10362
Closed

Querying a field that is required but fails throws inside apollo client #10345

AndreasHald opened this issue Dec 9, 2022 · 7 comments · Fixed by #10362
Assignees

Comments

@AndreasHald
Copy link

Hello.

I've encountered an issue that I thought was related to the apollo server - but after opening (apollographql/apollo-server#7235) it appears that the issue is actually on the client.

It stems from the fact that the response object from the server differs when the field is required and not when the resolver throws.

This is the response when the field is required

{
  errors: [
    {
      message: 'something went wrong',
      locations: [Array],
      path: [Array],
      extensions: [Object]
    }
  ],
  data: null
}

This is the response when the field is not required

{
  errors: [
    {
      message: 'something went wrong',
      locations: [Array],
      path: [Array],
      extensions: [Object]
    }
  ],
  data: { willFailFieldIsNotRequired: null }
}

However the apollo client attempts to read the field from the data object regardless of whether or not the field is required. causing it to throw unexpectedly when the field is required

Intended outcome:
Attempting to query a field that is required but the resolver throws - I expected apollo client to function as usually

Actual outcome:
the client throws null is not an object (evaluating 'rootValue[aliasedFieldName]') because it is attempting to read the query name from the root object.

How to reproduce the issue:
Refer to apollographql/apollo-server#7235

Versions
System:
OS: macOS 12.6
Binaries:
Node: 16.17.0 - ~/.nvm/versions/node/v16.17.0/bin/node
npm: 8.19.2 - ~/.nvm/versions/node/v16.17.0/bin/npm
Browsers:
Chrome: 108.0.5359.98
Safari: 16.0
npmPackages:
@apollo/rover: ^0.7.0 => 0.7.0
@apollo/server: ^4.1.1 => 4.1.1
@apollo/client: ^3.6.10

@glasser
Copy link
Member

glasser commented Dec 9, 2022

I'm not personally an expert in Apollo Client but I was curious about this since it came from your previous Apollo Server issue. I strongly suspect that you will get better help with this Apollo Client issue if you provide them with a full reproduction. The link you provide here to the AS issue links to a good server reproduction but it contains no client code. The error you're stating appears to come from the LocalState class and while I'm not an expert I think that might be something used only by one particular sub-feature of Apollo Client.

@bignimbus
Copy link
Contributor

Hi @AndreasHald 👋🏻 thanks for opening this issue! To echo @glasser's helpful comment, it would expedite a resolution to have a runnable reproduction to refer to so we can compare expected vs. actual behavior. Thanks so much!

@AndreasHald
Copy link
Author

Hi again @glasser & @bignimbus, it took some doing and narrowing of the issue but I think this reproducible example correctly demonstrates the issue.

https://stackblitz.com/edit/sveltejs-kit-template-default-8mylq3?file=src%2Froutes%2F%2Bpage.svelte&terminal=dev

To reproduce simply run npm run dev and click the two buttons to see the difference in errors throws

During the debugging I also learned that it has to do with the @client directive and client side resolvers colliding with required fields.

So querying

query { 
   willFailFieldIsRequired { 
      value
      value2 @client
   } 
}

Will throw an error from apollo client

"Cannot read properties of null (reading 'willFailFieldIsRequired')"

While the same query will not throw from apollo but return the correct error from the server if the field is not required.

@bignimbus
Copy link
Contributor

Thanks @AndreasHald 🙏🏻 I'm able to see the issue and can see how removing value2 @client changes the behavior. The team will take a closer look at this at some point but I'm not sure when. Thanks for your patience!

@bignimbus bignimbus added 🔍 investigate Investigate further and removed ⁉️ question labels Dec 12, 2022
@AndreasHald
Copy link
Author

@bignimbus happy to help, but please keep in mind that to my mind atleast this is quite a serious issue. Because if it happens in production the "actual" error will be swallowed and the user will most likely be shown a nonsense error that makes no sense.

@mccraveiro
Copy link
Contributor

@AndreasHald please check #10362 😉

mccraveiro added a commit to mccraveiro/apollo-client that referenced this issue Dec 13, 2022
alessbell added a commit that referenced this issue Feb 2, 2023
…eld (#10362)

* fix: error when server returns an error and we are also querying for a local field

closes #10345

* Apply suggestions from code review

Co-authored-by: Jerel Miller <[email protected]>

---------

Co-authored-by: Alessia Bellisario <[email protected]>
Co-authored-by: Jerel Miller <[email protected]>
@github-actions
Copy link
Contributor

github-actions bot commented Mar 5, 2023

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.
For general questions, we recommend using StackOverflow or our discord server.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Mar 5, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants