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

Bug: Infinite looping GraphQL requests, reported from faustjs #558

Open
PabloSzx opened this issue Jan 19, 2022 · 7 comments
Open

Bug: Infinite looping GraphQL requests, reported from faustjs #558

PabloSzx opened this issue Jan 19, 2022 · 7 comments
Assignees
Labels
bug Something isn't working

Comments

@PabloSzx
Copy link
Member

Copying issue reported in wpengine/faustjs#723 (comment) thank you @mediashane

Hi @PabloSzx ! We've set up a Github repo and Vercel deploy that reproduces the bug. I'll also attach a video that shows steps to repro and the bug itself in action.

Github repo: https://github.com/mediashane/graphql-bug After cloning, create a .env file in the base directory of the project and copy + save the contents of .env.local.sample into it. This will configure your local development environment to point towards the live WordPress backend. Then just npm install, npm run build and npm run start.

Here is a Vercel deploy of the repo linked above, where you can jump straight to reproducing the bug without cloning the repo: https://graphql-bug.vercel.app

Steps to reproduce it in Vercel (staging):

  1. Navigate to https://graphql-bug.vercel.app,
  2. Open Network Dev Tools tab
  3. Click “Our Story”
  4. Click “Rugs”
  5. Click “From the Farm”

If you don't see the looping GraphQL requests the first time, return to the home page, do a cache clearing reload and follow the sequence again. There are other occasions that this looping starts, but this is a sequence that we've found reproduces the bug consistently.

We're using the plugin WP GraphQL to expose our WordPress data, and Faust + GQty to do our queries and generate our schema locally. We think this issue is closely related to the issue you encountered and solved here: gqty-dev/gqty#520

Here is the video: https://user-images.githubusercontent.com/14046192/150017319-3c419d6a-5484-4129-b8bf-49802477b033.mp4

Thank you in advance for all of your work, and let us know if there's anything else we can do to help!


I could confirm this bug, and doesn't seem to be a bug on server side, the weirdest thing is that this bug doesn't happen on development mode, only after building Next.js and starting on production mode, this makes debugging way slower since making changes on node_modules takes a whole build+start

@PabloSzx PabloSzx added the bug Something isn't working label Jan 19, 2022
@PabloSzx PabloSzx self-assigned this Jan 19, 2022
@theodesp
Copy link

theodesp commented Jan 24, 2022

Looking at the repo containing the bug It seems that the code triggers the loop originates from this component:
https://github.com/mediashane/graphql-bug/blob/2b1a20dab7214377d56e7e163708d4a88e43a01c/src/koa-framework/AcfModules/AcfModules.tsx#L13-L23

Screenshot 2022-01-24 at 12 59 21

It then triggers something and goes back to the same susbscibe-resolve section here:

const unsubscribeResolve = scheduler.subscribeResolve(
(promise, selection) => {
if (
fetchingPromise.current === null &&
deferredCall.current === null &&
hookSelections.has(selection)
) {
const newPromise = new Promise<void>((resolve) => {
promise.then(({ error }) => {
fetchingPromise.current = null;
if (error && onError) onError(error);
Promise.resolve().then(forceUpdate);
resolve();
});
});
fetchingPromise.current = newPromise;
if (updateOnFetchPromise) {
if (enabledStaleWhileRevalidate && isRendering.current) {
deferredCall.current = forceUpdate;
} else {
deferredCall.current = null;
forceUpdate();
}
}
}
}
);

The stack trace loops again and again with this component.

I suspect it's something to do the way it renders the React Component again and again.

@theodesp
Copy link

theodesp commented Jan 24, 2022

I also suspect that because it uses queries from the previous page, it triggers an effect like this:

  1. A page is loaded. GQty triggers a schedule to request a query. Query is pending.
  2. Before the query finishes the user clicks on another page. React recycles all relevant components and loads the new Page.
  3. GQty somehow gets stuck in a loop trying to resolve the pending query from the previous page. Until that happens it does not see the new subscriptions from the new pages.

You can clearly see that stale promise by inspecting the state of the QueryFetcher.
Screenshot 2022-01-24 at 13 56 20

It loops here requesting parameters from a different page.

@theodesp
Copy link

theodesp commented Jan 24, 2022

The code also seems to loop around this block:

export function useDeferDispatch<F extends (...args: any[]) => void>(
dispatchFn: F
) {
const isMounted = useIsMounted();
const isRendering = useIsRendering();
const pendingDispatch = React.useRef<Array<() => void> | false>(false);
React.useEffect(() => {
if (pendingDispatch.current) {
for (const fn of pendingDispatch.current) {
fn();
}
pendingDispatch.current = false;
}
});
return React.useCallback(
(...args: any[]) => {
if (isRendering.current) {
if (pendingDispatch.current) {
pendingDispatch.current.push(() => {
if (isMounted.current) dispatchFn(...args);
});
}
pendingDispatch.current = [
() => {
if (isMounted.current) dispatchFn(...args);
},
];
} else if (isMounted.current) {
dispatchFn(...args);
}
},
[dispatchFn, isRendering, pendingDispatch, isMounted]
) as F;
}

Screenshot 2022-01-24 at 14 21 56

@theodesp
Copy link

theodesp commented Jan 25, 2022

Looking at the code here it looks like it falls into the troublesome case of data-selections-conditionals: https://gqty.dev/docs/react/troubleshooting#data-selections--conditionals

The AcfModules component is basically the map part of the example. I've rewriten the component to bring all related queries together:

import React from 'react';
import { client, RugIdType } from '../../client';

export interface Props {
  modules?: Array<any>;
  isLoading: boolean;
}
export default function AcfModules({ isLoading }: Props) {
  const rugDetails = client.useQuery().rug({
    id: `/${pageUri.join('/')}`,
    idType: RugIdType.URI,
  });
  const mods = client.usePage().pageBuilder.modules;
  const modss = rugDetails?.rug?.modules;

  // if query contains custom post type 'rug', use the Rug ACF modules
  // otherwise use the Page Builder ACF modules
  const modules = pageUri.includes('rug')
    ? Object.values(modss)
      .filter((e) => typeof e !== 'string')
      .sort((a, b) => {
        return a.order - b.order;
      })
    : mods; // **<--- HERE: Reacting data to be mapped**
  return (
    <>
      {modules.map((module, index) => { **<--- HERE: Mapping starts here**
        const componentName = module?.__typename?.split('_')?.pop() ?? module?.[0]?.__typename?.split('_')?.pop();
        if (!componentName) {
          console.error('[KOA]', 'AcfModules', 'Component name is empty');
          return;
        }
        const moduleData = module.$on ? module?.$on[module?.__typename] : module;
        return renderComponent(componentName, moduleData, index, isLoading);  // **<--- HERE: Data selection behind switch statements**
      })}
    </>
  );
}

The modules constant is the reactive data that change based on the page the user visits. For example it may be:

{"__typename":"Page_Pagebuilder_Modules_ContactBanner"}{"__typename":"Page_Pagebuilder_Modules_CenteredText"}{"__typename":"Page_Pagebuilder_Modules_ContactList"}{"__typename":"Page_Pagebuilder_Modules_ContactList"}

For the Contact-us page and:

{"__typename":"Page_Pagebuilder_Modules_HeroCenterTop"}{"__typename":"Page_Pagebuilder_Modules_FarmPostsList"}

For the From-the-farm page

The renderComponent is basically the acfRenderComponent function which is a big switch statement. The problem here is that they all refer to different components types as they depend on the componentName parameter which is essentially the module?.__typename string.

@PabloSzx is there a recommended way to deal with switch statements of that kind? What do you think?

@theodesp
Copy link

theodesp commented Jan 25, 2022

I also managed to replicated it more consistently. It looks like in the switch statement here:
https://github.com/mediashane/graphql-bug/blob/main/src/helpers/renderComponent.tsx

Any component that shares the same properties with a different component in a switch statement picks up their properties and tries to request them under a different Query Fragment. For example:

The components HeroLeftJustified with HeroRightJustified with ContactBanner share the same image property. Now for some reason when you click on the pages it will pick up the properties from a previous component. For example in the our-story page it will request the following modules:

{"query":"query($id1:ID!$idType2:PageIdType){page0:page(id:$id1 idType:$idType2){__typename id pageBuilder{__typename modules{__typename ...on Page_Pagebuilder_Modules_HeroRightJustified{textHeadline textColor textSubline textMediaLabel image{__typename id mediaItemUrl}mediaIcon{__typename id mediaItemUrl}}...on Page_Pagebuilder_Modules_TwoColumnContent{includeParagraph textHeadline textParagraph image{__typename id mediaItemUrl}}...on Page_Pagebuilder_Modules_OneColumnContent{includeParagraph textHeadline textParagraph textColor image{__typename id mediaItemUrl}}}}}}","variables":{"id1":"our-story","idType2":"URI"}}

Now before the page finishes loading you click on the Contact-us page. It will try to use some of the properties of the Page_Pagebuilder_Modules_HeroRightJustified module:

{"query":"query($id1:ID!$idType2:PageIdType){page0:page(id:$id1 idType:$idType2){__typename pageBuilder{__typename modules{__typename ...on Page_Pagebuilder_Modules_HeroRightJustified{image{__typename id mediaItemUrl}}...on Page_Pagebuilder_Modules_CenteredText{textColor}}}id}}","variables":{"id1":"contact-us","idType2":"URI"}}

However the HeroRightJustified is not within the list of available modules for that page. The correct module is the Page_Pagebuilder_Modules_ContactBanner. If you refresh the Contact-us page you get the correct query:

{"query":"query($first1:Int$where2:RootQueryToMenuItemConnectionWhereArgs$id3:ID!$idType4:PageIdType){menuItems0:menuItems(first:$first1 where:$where2){__typename nodes{__typename id label}}page0:page(id:$id3 idType:$idType4){__typename pageBuilder{__typename modules{__typename ...on Page_Pagebuilder_Modules_ContactBanner{image{__typename id mediaItemUrl}}...on Page_Pagebuilder_Modules_CenteredText{textColor}}}id}}","variables":{"first1":100,"where2":{"location":"PRIMARY"},"id3":"contact-us","idType4":"URI"}}

Notice that both Page_Pagebuilder_Modules_ContactBanner and Page_Pagebuilder_Modules_HeroRightJustified have the same query payload:

image{__typename id mediaItemUrl}

This means that somehow GQty uses the wrong Query Fragment ( it picks up Page_Pagebuilder_Modules_HeroRightJustified because it shares the same query payload and it was recently used).

On the other hand you can also check the CardList Component as it does not share any other properties with the other Component in the switch. If you remove all elements in the switch and render only the CardList component and any other component (for example HeroRightJustified) then it won't trigger a loop because there is no common property within the CardList component that can be used in a Query Fragment of any another Component.

I hope that helps.

@CesarBenavides777
Copy link

@theodesp Having the same issue here:
https://github.com/NoisyTrumpet/TexasYesProject/blob/main/src/features/BlockReturner.tsx
So you're saying is that maybe if we change the generic image or title fields to something more specific like heroTitle or something like that might help?

@jordanmaslyn
Copy link

@PabloSzx is there a way to globally disable cache for GQty to bypass this issue?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

4 participants