Skip to content
This repository has been archived by the owner on Jan 27, 2022. It is now read-only.

Running Apollo Engine in development for persisted query support #12

Open
mxstbr opened this issue Feb 19, 2018 · 9 comments
Open

Running Apollo Engine in development for persisted query support #12

mxstbr opened this issue Feb 19, 2018 · 9 comments
Labels

Comments

@mxstbr
Copy link

mxstbr commented Feb 19, 2018

EDIT: This turned out to not be an issue with the RetryLink! See the resulting conversation with @jbaxleyiii. Old issue text:

I'm not sure how to use the persisted query link in combination with the retry link. The problem is that the retry link ends up retrying the first query with the hash over and over, even if Apollo Engine hasn't seen it yet and returns an error.

I tried a custom retry link configuration that doesn't retry the happy path from persisted queries, but if the retry link doesn't retry the query the persisted query link doesn't send the full query text either 😕

Any clues how to use them together?

Click to see snippet of my apollo client code
  const retryLink = new RetryLink({
    attempts: (count, operation, error) => {
      const isPersistedQuery =
        operation.extensions &&
        operation.extensions.persistedQuery &&
        operation.extensions.persistedQuery.sha256Hash;
      // Don't retry persisted query tries since the persisted query link will
      // retry those will the full query text
      if (isPersistedQuery) return false;

      const isMutation =
        operation &&
        operation.query &&
        operation.query.definitions &&
        Array.isArray(operation.query.definitions) &&
        operation.query.definitions.some(
          def =>
            def.kind === 'OperationDefinition' && def.operation === 'mutation'
        );

      // Retry mutations for a looong time, those are very important to us so we want them to go through eventually
      if (isMutation) {
        return !!error && count < 25;
      }

      // Retry queries for way less long as this just ends up showing
      // loading indicators for that whole time which is v annoying
      return !!error && count < 6;
    },
  });

  // HTTP Link for queries and mutations including file uploads
  const httpLink = createPersistedQueryLink().concat(
    retryLink.concat(
      createUploadLink({
        uri: API_URI,
        credentials: 'include',
        headers,
      })
    )
  );
@jbaxleyiii
Copy link
Contributor

@mxstbr I think the issue here is with the attempts function. When persisted query link fails, it doesn't strip the extensions field, but rather adds the query field and tells http to send both. So this bit:

const isPersistedQuery =
        operation.extensions &&
        operation.extensions.persistedQuery &&
        operation.extensions.persistedQuery.sha256Hash;
      // Don't retry persisted query tries since the persisted query link will
      // retry those will the full query text
      if (isPersistedQuery) return false;

will always return false even after failure since the persisted query link is before it. If you remove that, it should work! Here is a first run test to see if I understood what is happening! https://github.com/apollographql/apollo-link-persisted-queries/pull/13/files#diff-a15885c1b39ba5a6c3bfc050319fdaf3R56

@mxstbr
Copy link
Author

mxstbr commented Feb 21, 2018

The problem is, if I remove that bit the RetryLink will keep retrying the persisted query call with the hash, rather than letting the PersistedQueryLink retry will the full query text.

@mxstbr
Copy link
Author

mxstbr commented Feb 21, 2018

This is what happens with that bit of code removed:

persisted-queries

Note how all the network requests only send the hash, they never send the full query text. That's what I'm trying to fix here.

Some notes:

  • I'm using v7.1.0-alpha.1 of the apollo-upload-client link by @jaydenseric, which added support for persisted queries. (ref: Persisted queries? jaydenseric/apollo-upload-client#61)
  • I observe the same behavior when I replace the createUploadLink call with createHttpLink, so that's not the culprit here.
  • I observe the same behavior when I remove all the custom options from RetryLink

Here is my entire Apollo Client code, maybe that helps:

Click to reveal client.js
export const createClient = (options) => {
  const cache = new InMemoryCache({
    fragmentMatcher: new IntrospectionFragmentMatcher({
      introspectionQueryResultData,
    }),
    cacheResolvers,
    dataIdFromObject,
  });

  const headers = options.token
    ? {
        authorization: `Bearer ${options.token}`,
      }
    : undefined;

  const retryLink = new RetryLink({
    attempts: (count, operation, error) => {
      const isMutation =
        operation &&
        operation.query &&
        operation.query.definitions &&
        Array.isArray(operation.query.definitions) &&
        operation.query.definitions.some(
          def =>
            def.kind === 'OperationDefinition' && def.operation === 'mutation'
        );

      // Retry mutations for a looong time, those are very important to us so we want them to go through eventually
      if (isMutation) {
        return !!error && count < 25;
      }

      // Retry queries for way less long as this just ends up showing
      // loading indicators for that whole time which is v annoying
      return !!error && count < 6;
    },
  });

  const persistedQueryLink = createPersistedQueryLink();

  const uploadLink = createUploadLink({
    uri: API_URI,
    credentials: 'include',
    headers,
  });

  const httpLink = ApolloLink.from([persistedQueryLink, retryLink, uploadLink])

  // Websocket link for subscriptions
  const wsLink = new WebSocketLink({
    uri: `${IS_PROD ? `wss://${window.location.host}` : 'ws://localhost:3001'}/websocket`,
    options: {
      reconnect: true,
    },
  });

  // Switch between the two links based on operation
  const link = split(
    ({ query }) => {
      const { kind, operation } = getMainDefinition(query);
      return kind === 'OperationDefinition' && operation === 'subscription';
    },
    wsLink,
    httpLink
  );

  return new ApolloClient({
    link,
    cache: window.__DATA__ ? cache.restore(window.__DATA__) : cache,
    ssrForceFetchDelay: 100,
    queryDeduplication: true,
  });
};

@mxstbr
Copy link
Author

mxstbr commented Feb 21, 2018

Hmm, maybe this actually isn't related to the RetryLink at all. Even without the RetryLink the PersistedQueryLink never sends the full query text. Digging in.

@mxstbr
Copy link
Author

mxstbr commented Feb 21, 2018

Okay so this might not be related to the RetryLink after all, sorry for the confusion.

Looking at the verbose logs in the browser console I see "Cross-Origin Request Blocked: The Same Origin Policy disallows reading the remote resource at http://localhost:3001/api. (Reason: CORS header ‘Access-Control-Allow-Origin’ missing)." (our client runs at localhost:3000, API at localhost:3001)

This doesn't happen before the persisted query link PR. The only changes I've made are:

  1. Run Apollo Engine in development (otherwise persisted queries don't work)
  2. Add the persisted query link
Click for the full diff

engine.js:

 // @flow
 import { Engine } from 'apollo-engine';
 
+// In development don't send any performance data to Engine, we only start it
+// for the persisted queries
+const origins =
+  process.env.NODE_ENV === 'development'
+    ? {
+        maxConcurrentRequests: 0,
+      }
+    : undefined;
+
 export default new Engine({
   graphqlPort: 3001,
   endpoint: '/api',
   engineConfig: {
+    stores: [
+      {
+        name: 'pq',
+        inMemory: {
+          cacheSize: '5000000',
+        },
+      },
+    ],
+    persistedQueries: {
+      store: 'pq',
+    },
     logging: {
       level: 'WARN',
     },
     apiKey: process.env.APOLLO_ENGINE_API_KEY,
   },
+  origins,
 });

api/middlewares.js:

 // Start apollo engine
-if (process.env.NODE_ENV === 'production' && !process.env.FORCE_DEV) {
-  const engine = require('./engine').default;
-  middlewares.use(engine.expressMiddleware());
-}
+const engine = require('./engine').default;
+middlewares.use(engine.expressMiddleware());

client.js:

-  // HTTP Link for queries and mutations including file uploads
-  const httpLink = retryLink.concat(
-    createUploadLink({
-      uri: API_URI,
-      credentials: 'include',
-      headers,
-    })
-  );
+  const persistedQueryLink = createPersistedQueryLink();
+
+  const uploadLink = createUploadLink({
+    uri: API_URI,
+    credentials: 'include',
+    headers,
+  });
+
+  const httpLink = ApolloLink.from([persistedQueryLink, retryLink, uploadLink]);

Maybe the problem is that the proxy blocks cross-origin requests for some reason? I couldn't find anything in the documentation on how to add support for that, but our API server does use the cors middleware.

@mxstbr mxstbr changed the title Usage with the RetryLink Running Apollo Engine in development for persisted query support Feb 21, 2018
@wireforce
Copy link

@mxstbr Did you ever find a solution for this? I suspect that there is something fishy going on with this library and CORS. I cant get it to accept cross origin requests...

@declanelcocks
Copy link

declanelcocks commented Sep 5, 2018

@jbaxleyiii Going to drop my experience here too as it seems closely related to @mxstbr (came here via his post on apollo-upload-client). The problem for me is that my second query does not pass the correct payload to the API.

First query:

------WebKitFormBoundaryvNYFgEzAXZqcZmYL
Content-Disposition: form-data; name="operations"

{"operationName":"UploadFile","variables":{"file":null},"extensions":{"persistedQuery":{"version":1,"sha256Hash":"c08bdd3daf357d9b08033d4f338f09d6cd88dee402772683d51dcaa0eea7d88c"}}}
------WebKitFormBoundaryvNYFgEzAXZqcZmYL
Content-Disposition: form-data; name="map"

{"0":["variables.file"]}
------WebKitFormBoundaryvNYFgEzAXZqcZmYL
Content-Disposition: form-data; name="0"; filename="blob"
Content-Type: image/png

This one will respond as expected with PersistedQueryNotFound, but here's the payload of the client's response to that:

{"operationName":"UploadFile","variables":{"file":null},"extensions":{"persistedQuery":{"version":1,"sha256Hash":"c08bdd3daf357d9b08033d4f338f09d6cd88dee402772683d51dcaa0eea7d88c"}},"query":"mutation UploadFile($file: Upload!) {\n  uploadFile(file: $file) {\n    success\n    __typename\n  }\n}\n"}

The file itself is not actually passed during the retry, which will result in the retry throwing an error that $file is null. I can also confirm that after removing createPersistedQueryLink, the mutation works fine in every situation.

@jaydenseric
Copy link

jaydenseric commented Sep 5, 2018

@declanelcocks Keep in mind that the HTTP link from apollo-upload-client modifies the operation for the request, see jaydenseric/apollo-upload-client#81. I don't know if this is what is happening in your situation, but if the operation gets used again after that link all the files will have been replaced with null.

@declanelcocks
Copy link

declanelcocks commented Sep 5, 2018

@jaydenseric from what you wrote, I assume that using files specifically would not work at all with persisted queries, since the multipart form data will not be persisted across to the retry query? I think my only option at the moment would be to disable persisted queries for that session if the operation is UploadFile as the retry will always fail.

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

No branches or pull requests

5 participants