core: return PersistedQueryNotSupported for Apollo Persisted Queries (#…

…982)

Apollo Persisted Queries is a standard for sending queries as short hashes
instead of full strings, designed to work well with GET requests. It is
implemented by servers including the Apollo Engine Proxy, and by the
apollo-link-persisted-query client.

A common configuration is to set up persisted queries on production servers but
not in development. It is still convenient to leave apollo-link-persisted-query
always on, though. While apollo-link-persisted-query can detect that servers
don't support PQs, it works better if the server actually says it doesn't
support the PQ, instead of trying to process a request without a query and
potentially printing a confusing stack trace.  This commit makes apollo-server
directly return PersistedQueryNotSupported instead of allowing confusing stack
traces to occur.

packages/apollo-server-core/src/runHttpQuery.ts

@@ -101,11 +101,54 @@ export async function runHttpQuery(
   const requests: Array<ExecutionResult> = requestPayload.map(requestParams => {
     try {
       let query = requestParams.query;
+      let extensions = requestParams.extensions;
+
+      if (isGetRequest && extensions) {
+        // For GET requests, we have to JSON-parse extensions. (For POST
+        // requests they get parsed as part of parsing the larger body they're
+        // inside.)
+        try {
+          extensions = JSON.parse(extensions);
+        } catch (error) {
+          throw new HttpQueryError(400, 'Extensions are invalid JSON.');
+        }
+      }
+
+      if (query === undefined && extensions && extensions.persistedQuery) {
+        // It looks like we've received an Apollo Persisted Query. Apollo Server
+        // does not support persisted queries out of the box, so we should fail
+        // fast with a clear error saying that we don't support APQs. (A future
+        // version of Apollo Server may support APQs directly.)
+        throw new HttpQueryError(
+          // Return 200 to simplify processing: we want this to be intepreted by
+          // the client as data worth interpreting, not an error.
+          200,
+          JSON.stringify({
+            errors: [
+              {
+                message: 'PersistedQueryNotSupported',
+              },
+            ],
+          }),
+          true,
+          {
+            'Content-Type': 'application/json',
+          },
+        );
+      }
+
       if (isGetRequest) {
         if (typeof query === 'string') {
           // preparse the query incase of GET so we can assert the operation.
+          // XXX This makes the type of 'query' in this function confused
+          //     which has led to us accidentally supporting GraphQL AST over
+          //     the wire as a valid query, which confuses users. Refactor to
+          //     not do this. Also, for a GET request, query really shouldn't
+          //     ever be anything other than a string or undefined, so this
+          //     set of conditionals doesn't quite make sense.
           query = parse(query);
         } else if (!query) {
+          // Note that we've already thrown a different error if it looks like APQ.
           throw new HttpQueryError(400, 'Must provide query string.');
         }
 
@@ -122,10 +165,13 @@ export async function runHttpQuery(
       }
 
       const operationName = requestParams.operationName;
-      let variables = requestParams.variables;
 
+      let variables = requestParams.variables;
       if (typeof variables === 'string') {
         try {
+          // XXX Really we should only do this for GET requests, but for
+          // compatibility reasons we'll keep doing this at least for now for
+          // broken clients that ship variables in a string for no good reason.
           variables = JSON.parse(variables);
         } catch (error) {
           throw new HttpQueryError(400, 'Variables are invalid JSON.');

packages/apollo-server-integration-testsuite/src/index.ts

@@ -403,6 +403,48 @@ export default (createApp: CreateAppFunc, destroyApp?: DestroyAppFunc) => {
         });
       });
 
+      it('returns PersistedQueryNotSupported to a GET request', async () => {
+        app = await createApp();
+        const req = request(app)
+          .get('/graphql')
+          .query({
+            extensions: JSON.stringify({
+              persistedQuery: {
+                version: 1,
+                sha256Hash:
+                  'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa',
+              },
+            }),
+          });
+        return req.then(res => {
+          expect(res.status).to.equal(200);
+          expect(res.body).to.deep.equal({
+            errors: [{ message: 'PersistedQueryNotSupported' }],
+          });
+        });
+      });
+
+      it('returns PersistedQueryNotSupported to a POST request', async () => {
+        app = await createApp();
+        const req = request(app)
+          .post('/graphql')
+          .send({
+            extensions: {
+              persistedQuery: {
+                version: 1,
+                sha256Hash:
+                  'aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa',
+              },
+            },
+          });
+        return req.then(res => {
+          expect(res.status).to.equal(200);
+          expect(res.body).to.deep.equal({
+            errors: [{ message: 'PersistedQueryNotSupported' }],
+          });
+        });
+      });
+
       it('can handle a request with variables', async () => {
         app = await createApp();
         const expected = {