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

useLazyQuery throw error when error data is null or undefined #11149

Closed
quocluongha opened this issue Aug 14, 2023 · 23 comments · Fixed by #11252
Closed

useLazyQuery throw error when error data is null or undefined #11149

quocluongha opened this issue Aug 14, 2023 · 23 comments · Fixed by #11252

Comments

@quocluongha
Copy link

Issue Description

After I upgraded to version 3.8, I noticed that if I call the function returned by useLazyQuery hook and the data field of the error is null, my app instantly crash even if I wrapped the function call in try and catch block.

Simulator Screenshot - iPhone 14 - 2023-08-14 at 22 17 55

Link to Reproduction

https://github.com/quocluongha/rn-apollo-test

Reproduction Steps

  1. Install React Native CLI.
  2. Run npm install or yarn install.
  3. Run npm run start or yarn start.
  4. Run npm run android / yarn android for Android or npm run ios / yarn ios for iOS.
  5. Press the "Get Data" button.
@bignimbus bignimbus added 🏓 awaiting-team-response requires input from the apollo team 🌐 React Native labels Aug 14, 2023
@bignimbus
Copy link
Contributor

Hi @quocluongha 👋🏻 I tried running this example but need to debug my local environment further and can't budget the time at the moment. Would you be able to confirm that downgrading to a prior version of @apollo/client resolves the issue?

@bignimbus bignimbus added 🏓 awaiting-contributor-response requires input from a contributor and removed 🏓 awaiting-team-response requires input from the apollo team labels Aug 14, 2023
@quocluongha
Copy link
Author

Yes, downgrading to 3.7 resolves the issue.

@bignimbus
Copy link
Contributor

Hmm, that's interesting - I was not able to resolve this in your test repo by downgrading, though I admit I might be doing something wrong since I'm definitely not a React Native expert by any stretch of the imagination. When I inspect the network request in the simulator, here's what I see:

Screenshot 2023-08-14 at 1 27 03 PM

That doesn't appear to be a valid response per the GraphQL spec. But maybe there's something in my environment that doesn't line up with what you're seeing? Thanks in advance for helping me narrow this down.

@bignimbus bignimbus added 🏓 awaiting-contributor-response requires input from a contributor and removed 🏓 awaiting-contributor-response requires input from a contributor labels Aug 14, 2023
@quocluongha
Copy link
Author

quocluongha commented Aug 15, 2023

I'm sorry for my late response. I have create a branch with version 3.7, you can check out onto that branch and start testing.
About the network inspection, I used Flipper for debugging since I am not quite familiar with the built-in network inspection, and the response looked like this.

Screenshot 2023-08-15 at 9 28 45 AM

Btw, I was expecting the data should be undefined and version 3.7 is working fine, but the 3.8 is not.

const [getData] = useLazyQuery(gql`
    query aa {
      getOrders {
        error
        message
        result
      }
    }
  `);

  const handleGetData = async () => {
    try {
      const {data} = await getData();

      console.log(data); // <-- The data is undefined when downgrading to 3.7
    } catch (error) {
      console.log(error);
    }
  };

@plausible-phry
Copy link

plausible-phry commented Aug 15, 2023

This seems to be triggered by this subscription handler if complete is reached before next was ever reached:

concast.subscribe({
next: (value) => {
result = value;
},
error: () => {
resolve(this.toQueryResult(this.observable.getCurrentResult()));
},
complete: () => {
resolve(this.toQueryResult(result));
},
});

Wildly guessing here: I assume that was not possible before we had multipart response support, and now we need to guard against that case.

@quocluongha The error in the screenshot has nothing to do with data being null though, but with the response itself being undefined. Are you sure that is the trigger here? In the screenshot you are sharing, data is not null, but not present at all, too (which is perfectly fine and should not cause any errors).

@github-actions github-actions bot removed the 🏓 awaiting-contributor-response requires input from a contributor label Aug 15, 2023
@quocluongha
Copy link
Author

@plausible-phry I tested it again with the another endpoint contain data field in the response and it threw the same error.

Screenshot 2023-08-15 at 6 12 58 PM

@quocluongha quocluongha changed the title useLazyQuery throw error when error data is null useLazyQuery throw error when error data is null or undefined Aug 15, 2023
@JosephLu2022
Copy link

+1

@versatile-panda
Copy link

versatile-panda commented Aug 22, 2023

I've tested multiple times and it seems this error is not happening on 3.7.16 but is occurring on 3.7.17.

@mrlexz
Copy link

mrlexz commented Sep 7, 2023

same

@phryneas
Copy link
Member

phryneas commented Sep 8, 2023

@versatile-panda sorry, I missed your comment, as I was on vacation. If you are experiencing an error like this with 3.7.17, but not with 3.7.16, it is definitely a different bug as described here. The files with the bug described here were not touched by 3.7.17.

Could you please open a new Issue with a more thorough description?

@mrlexz I am sorry, but "same" doesn't give me the necessary information to give you any feedback. You might be seeing this bug, you might be seeing a different bug with a similar error surfacing.
Could you please provide more context?

@matinzd
Copy link

matinzd commented Sep 18, 2023

I encountered the same problem starting with version 3.7.17. When our API response has { data: null, error: [...] }, it triggers an internal crash in useLazyQuery. Everything works without any problem on version 3.7.16.

@phryneas
Copy link
Member

phryneas commented Sep 19, 2023

@matinzd as I said above, if this started happening with 3.7.17, it is likely a different problem than the exact problem you are seeing here - please open a new issue, with the exact error message you are getting, and if possible a reproduction - as I am not able to reproduce what you are seeing here.

I can see that in 3.7.17, data: null will result in a Missing field '...' while writing result {} error, but that also already happened in 3.7.16 - so I believe you are seeing a different error than I am.

@matinzd
Copy link

matinzd commented Sep 19, 2023

I'll try to open a new issue. The issue is exactly the same as in the screenshot:

image

@phryneas
Copy link
Member

So I have tried to make another reproduction - again, without any chance of seeing this error myself.

You can see my attempt here:
https://github.com/apollographql/react-apollo-error-template/tree/repro/11149

Could one of you please take this as a starting point to recreate their problem?

@quocluongha
Copy link
Author

Hi @phryneas, I'm sorry for my late response. The repo which you provided was using React, but I encountered the error on React Native.
Can you use my reproduction repo I provided or create a new repo using React Native instead.

@phryneas
Copy link
Member

phryneas commented Sep 25, 2023

@quocluongha thanks for the ping! I have to admit, I missed your reproduction - when you originally filed the issue, I was on vacation (I'm @plausible-phry), and I scrolled by it every time after.

With your reproduction, I can see the error, and it seems to be an internal timing problem that for some curious reason only happens in React Native.

I am working on a fix (still digging for the original reason of this!), but here is a workaround you can already use: you need to add a link that slightly delays the onComplete callback:

-import {ApolloClient, ApolloProvider, InMemoryCache} from '@apollo/client';
+import {ApolloClient, ApolloProvider, HttpLink, InMemoryCache, Observable, ApolloLink} from '@apollo/client';
 import {loadDevMessages, loadErrorMessages} from '@apollo/client/dev';
 import React from 'react';
 import {StyleSheet} from 'react-native';
 import {TestScreen} from './TestScreen';
 
+const delayCompletionLink = new ApolloLink((operation, forward) => {
+  const observable = forward(operation);
+  const observer = new Observable(obs => {
+    observable.subscribe({
+      next(r) {
+        obs.next(r);
+      },
+      error(e) {
+        obs.error(e);
+      },
+      complete() {
+        setTimeout(() => {
+          obs.complete();
+        }, 100);
+      },
+    });
+  });
+  return observer;
+});
+
 const client = new ApolloClient({
-  uri: 'https://hasura.sukimashopping.com/v1/graphql',
   cache: new InMemoryCache(),
+  link: delayCompletionLink.concat(
+    new HttpLink({uri: 'https://hasura.sukimashopping.com/v1/graphql'}),
+  ),
 });

@quocluongha
Copy link
Author

Thank you so much! @phryneas

@matinzd
Copy link

matinzd commented Sep 25, 2023

In our case we are using onError callback.
It seems I need to delay on Error as well. I feel like it's related to Hermes engine.
@phryneas

@phryneas
Copy link
Member

I think I might have solved this.

Could you please try the build from #11249 ?

npm i @apollo/[email protected]

@matinzd
Copy link

matinzd commented Sep 25, 2023

I think I might have solved this.

Could you please try the build from #11249 ?

npm i @apollo/[email protected]

I'll give it a try tomorrow!

@jgillick
Copy link

I think I might have solved this.

Could you please try the build from #11249 ?

npm i @apollo/[email protected]

This seemed to fix the error for me.

@phryneas
Copy link
Member

Just to keep you in the loop here - I have created a PR to fix this more thoroughly over in #11252.
You can expect that to get reviewed and released at some point during next week.

Copy link
Contributor

github-actions bot commented Nov 4, 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 Nov 4, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.