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

Infinite loop caused by Type Policy with read function #7028

Open
timothyarmes opened this issue Sep 16, 2020 · 13 comments · May be fixed by #11430
Open

Infinite loop caused by Type Policy with read function #7028

timothyarmes opened this issue Sep 16, 2020 · 13 comments · May be fixed by #11430

Comments

@timothyarmes
Copy link

timothyarmes commented Sep 16, 2020

My app has a User type for which I've defined a type policy:

    User: {
      fields: {
        createdAt:  { read: (date) => date ? parseISO(date) : null },
      },
    },

I also have a query to fetch the currently logged in user, and a HOC is used to fetch that user and render sub components.

const GET_SELF = gql`
  query me {
    me {
      ...userFragment
      prefs { ...userPrefsFragment }
    }
  }
  
  ${Users.userFragment}
  ${Users.userPrefsFragment}
`;

const UserProvider = ({ render }) => {
  const { error, loading, data: { me } = {}, refetch, subscribeToMore } = useQuery(GET_SELF, { fetchPolicy: 'cache-first' });

  <snip>

  if (loading) return <Loading />;
  return render(me);
}

Now, in a subcomponent I'm running a query which fetches a set of users:

export const GET_MISSION = gql`
  query mission($id: ID!) {
    mission(_id: $id) {
      users { ...userDisplayFragment }
    }
  }

 ${userDisplayFragment}
`;

When that query returns the currently logged-in user as part of the users list, the code enters into an infinite loop.

The fields returned for the currently logged in user are merged into the user object in the cache. Despite the fact that these fields have not changed the value of that user object, this causes the 'me' query to rerun, which reruns the 'mission' query above, which leads to the infinite loop.

However, if I remove the createdAt field from the Type Policy for the User object then everything works.

I thought that I be able to work around the issue be returning the exact same parsed date object when the date doesn't change:


class DatePolicy {
  constructor() {
    this.prevDate = null;
    this.prevParsedDate = null;
  }

  read = (date) => {
    if (date !== this.prevDate) {
      this.prevDate = date;
      this.prevParsedDate = date ? parseISO(date) : null;
    }

    return this.prevParsedDate;
  }
}

And using this class in the Type Policy:

User: {
  fields: {
    createdAt: new DatePolicy(),
  },
},

however that doesn't solve the problem.

If however the read function returns the object unchanged then the loop is avoided:

read = (date) => date

So, it seems that when a read function that returns a value other than that passed in will force the query to re-run.

Versions

System:
OS: macOS 10.15
Binaries:
Node: 12.13.1 - ~/.nvm/versions/node/v12.13.1/bin/node
Yarn: 1.19.2 - /usr/local/bin/yarn
npm: 6.12.1 - ~/.nvm/versions/node/v12.13.1/bin/npm
Browsers:
Chrome: 85.0.4183.102
Firefox: 77.0.1
Safari: 13.0.2
npmPackages:
@apollo/client: ^3.2.0 => 3.2.0
apollo-link-logger: ^1.2.3 => 1.2.3
apollo-server-express: ^2.17.0 => 2.17.0

@timothyarmes timothyarmes changed the title Infinity loop due to cache merging Infinity loop due to Type Policy Sep 17, 2020
@timothyarmes timothyarmes changed the title Infinity loop due to Type Policy Infinite loop caused by Type Policy Sep 17, 2020
@timothyarmes timothyarmes changed the title Infinite loop caused by Type Policy Infinite loop caused by Type Policy with read function Sep 17, 2020
@benjamn
Copy link
Member

benjamn commented Sep 17, 2020

@timothyarmes Can you put this setup into a reproduction? That will help us give you the right advice without guessing (though I can keep guessing if a repro is not feasible).

As an initial note, that DatePolicy pattern seems risky because you'll be using the same DatePolicy instance (with only one this.prevDate) for all User.createdAt fields, even for different users. I don't think that's the key to solving the original problem, but it seemed worth mentioning.

@timothyarmes
Copy link
Author

timothyarmes commented Sep 17, 2020

Hi Ben. I'll try to find time to create a repo.

You're right, the DatePolicy class instance won't work.

@ziggy6792
Copy link

ziggy6792 commented Apr 4, 2021

Hey @timothyarmes

Thanks for sharing this I was having the exact same issue.
I came across this issue and @theodorDiaconu has done a brilliant job creating a small package that does what we want here. I followed his example and it works beautifully.

Good luck :)

@brainkim
Copy link
Contributor

brainkim commented Apr 9, 2021

Gentle reminder that we’re looking for reproductions. I greatly prioritize any issue which I can run, and I got a lot of issues on my plate at the moment. If not, I will get to this soon no worries 😇

@rcbevans
Copy link

I'm hitting the same issue, just spent 3 hours trying to debug.

The trigger seems to be a mutation using an update function to merge a field onto a type with a read function specified on the type policy.

@benjamn benjamn assigned benjamn and unassigned brainkim Aug 11, 2021
@garrettg123
Copy link

garrettg123 commented Mar 29, 2022

This took a lot of trial/error, but for me it was printing out the options argument for the merge function:

              merge(existing = [], incoming, options) {
                const merged = _.uniqBy([...existing, ...incoming], '__ref')

                console.log('Merged audioPosts', {
                  existing,
                  incoming,
                  options,
                  variables, // ADDING `: options.variables,` FIXED THE ISSUE (along with a React Native printing error)
                  merged,
                })

EDIT: Nevermind, still getting 7 of the same queries. It only makes 1 request and the rest hit cache, but the merge is annoying/slow.

@WallisonHenrique
Copy link

WallisonHenrique commented Feb 17, 2023

Your UserType or fields within UserType (createdAt) probably don't have an ID for Apollo to deduplicate the data in the cache.
https://www.apollographql.com/docs/kotlin/caching/troubleshooting

What is probably happening is that Apollo saves the data of the logged User in the cache and the other request that lists the users modifies the data of this logged user (with different data) forcing a cache update. A single missing field in the cache for the query querying the logged in user will cause a new request. The infinite loop happens because the UserType data in separate queries are different and each request updates the cache with the expected format.

This comment reinforces my point.
#6760 (comment)

You can confirm that you are having a cache update on the user type using the apollo client devTools extension in the cache tab.
https://chrome.google.com/webstore/detail/apollo-client-devtools/jdkknkkbebbapilgoeccciglkfbmbnfm

Complement

Apollo Client identifies similar data from different requests by associating the field type plus the object id.
Example:
User:2 => Type + Id
user {
'__ref': 'User:2'
}
It uses this reference to associate data from the cache and when requested by another request it reuses identifying the _ref.

It may be that a schema of a request has an id (or the field used to reference it) and the other request that has the same type does not have this id. The first request adds the id and the second modifies the cache without the id. When the cache data modifies it calls again the first request and modifying the cache again calls the second. Infinite loop.

@alessbell alessbell added the 🏓 awaiting-contributor-response requires input from a contributor label Apr 3, 2023
@alessbell
Copy link
Member

Hi everyone 👋 In case it's helpful, we have a CodeSandbox and forkable GitHub repo for creating reproductions, which would greatly aid in prioritizing/assessing this issue. Thanks!

@marco2216
Copy link

marco2216 commented Oct 25, 2023

I experienced the same issue today. Adding a read function to a field would cause two queries to be fetched in an infinite loop.
I figured out that it was caused by one of those two queries that queried data in this structure:

 product {
        id
        details {
          id
          type
          title
        }
      }

The backend returned the same id for product and details, and our default dataIdFromObject function just returns the object id directly, so they would have the same cache id. Adding the typename to the dataIdFromObject function fixed the issue.
Quite an odd issue, the read field was added to a completely different field, a field that was not even queried by the query that the snippet above is from. It was queried by the other query which was a part of the infinite loop though.

@marco2216
Copy link

It started happening again with different queries, now I realised that the infinite loop also happens if there are some queries getting data from types that don't have ID where there is no merge function defined in the cache, if the queries override the data of each other.
For some reason, this doesn't cause any issues usually, but as soon as the read function that returns new data is added, if one of the queries queries the field the read function is added to, it starts causing the infinite loop.

@marco2216
Copy link

marco2216 commented Oct 31, 2023

@alessbell
Here is a reproduction of the issue with the cause being that different data in two queries is being fetched from the same type that doesn't have an ID/ or merge function in the cache combined with the custom read function on Person.name
Upon opening the Sandbox, if you open the browser console, you should see query AllPeople being logged continuously, since the query is being repeated infinitely.
If you remove line 121 in index.jsx name: () => "name_", and reload the page, you should see that the query only fires once.
Sandbox link

@marco2216
Copy link

@benjamn I would greatly appreciate it if you have time to look at this issue and my reproduction example above, this is blocking us from using custom cache read functions as I can't risk this happening in production.

@phryneas phryneas removed the 🏓 awaiting-contributor-response requires input from a contributor label Dec 14, 2023
@phryneas phryneas linked a pull request Dec 14, 2023 that will close this issue
3 tasks
@phryneas
Copy link
Member

Hi @marco2216, I've taken a look at this and believe #11430 might get closer to solving this problem. The problem of potential feuds between queries is very complex, though, so this will likely need a lot more investigation before we can get it in - just as a fair warning.

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

Successfully merging a pull request may close this issue.

10 participants