Merge pull request #2 from ef-eng/fix-express-graphql-pretty-print-bug

fix: fixing bug where responses don't get rewritten with pretty-printing

Author: chanind

src/index.ts

@@ -10,7 +10,8 @@ interface RewriterMiddlewareOpts {
 const rewriteResJson = (res: Response) => {
   const originalJsonFunc = res.json.bind(res);
   res.json = function(body: any) {
-    if (!this.req || !this.req._rewriteHandler) return originalJsonFunc(body);
+    if (!this.req || !this.req._rewriteHandler || this._isRewritten) return originalJsonFunc(body);
+    this._isRewritten = true;
     const rewriteHandler = this.req._rewriteHandler;
     if (typeof body === 'object' && !(body instanceof Buffer) && body.data) {
       const newResponseData = rewriteHandler.rewriteResponse(body.data);
@@ -21,6 +22,43 @@ const rewriteResJson = (res: Response) => {
   };
 };
 
+const rewriteResRaw = (res: Response) => {
+  const originalEndFunc = res.end.bind(res);
+  res.end = function(body: any) {
+    if (!this.req || !this.req._rewriteHandler || this._isRewritten || this.headersSent) {
+      return originalEndFunc(body);
+    }
+    this._isRewritten = true;
+    const existingHeaders = this.getHeaders();
+    const isJsonContent = existingHeaders['content-type'] === 'application/json; charset=utf-8';
+    const rewriteHandler = this.req._rewriteHandler;
+    if (isJsonContent && body instanceof Buffer) {
+      try {
+        const bodyJson = JSON.parse(body.toString('utf8'));
+        if (bodyJson && bodyJson.data) {
+          const newResponseData = rewriteHandler.rewriteResponse(bodyJson.data);
+          const newResBodyJson = { ...bodyJson, data: newResponseData };
+          // assume this was pretty-printed if we're here and not in the res.json handler
+          const newResBodyStr = JSON.stringify(newResBodyJson, null, 2);
+          const newResChunk = Buffer.from(newResBodyStr, 'utf8');
+          this.setHeader('Content-Length', String(newResChunk.length));
+          return originalEndFunc(newResChunk);
+        }
+      } catch (err) {
+        // if we can't decode the response as json, just forward it along
+        return originalEndFunc(body);
+      }
+    }
+    return originalEndFunc(body);
+  };
+};
+
+const rewriteRes = (res: Response) => {
+  rewriteResJson(res);
+  // if res.json isn't available, or pretty-printing is enabled, express-graphql uses raw res.end()
+  rewriteResRaw(res);
+};
+
 const graphqlRewriterMiddleware = ({
   rewriters,
   ignoreParsingErrors = true
@@ -46,7 +84,7 @@ const graphqlRewriterMiddleware = ({
         req.body = newBody;
       }
       req._rewriteHandler = rewriteHandler;
-      rewriteResJson(res);
+      rewriteRes(res);
     } catch (err) {
       if (!ignoreParsingErrors) return next(err);
     }

src/types.d.ts

@@ -3,4 +3,8 @@ declare namespace Express {
   interface Request {
     _rewriteHandler?: import('graphql-query-rewriter').RewriteHandler;
   }
+
+  interface Response {
+    _isRewritten?: boolean;
+  }
 }

test/index.test.ts

@@ -166,7 +166,7 @@ describe('middleware test', () => {
     });
   });
 
-  const setupMutationApp = () => {
+  const setupMutationApp = (extraExpressGraphqlOpts: any = {}) => {
     const app = express();
 
     app.use(
@@ -190,7 +190,8 @@ describe('middleware test', () => {
       '/graphql',
       graphqlHTTP({
         schema,
-        rootValue
+        rootValue,
+        ...extraExpressGraphqlOpts
       })
     );
     return app;
@@ -362,7 +363,7 @@ describe('middleware test', () => {
     expect(invalidQueryRes.body.data).toEqual({ random: true });
   });
 
-  it('ignores invalid json responses', async () => {
+  it('ignores invalid json responses sent via response.json()', async () => {
     const app = express();
 
     app.use(
@@ -401,6 +402,106 @@ describe('middleware test', () => {
     expect(invalidQueryRes.body).toEqual('jimmy');
   });
 
+  it('ignores invalid json responses sent via response.end()', async () => {
+    const app = express();
+
+    app.use(
+      '/graphql',
+      graphqlRewriterMiddleware({
+        rewriters: [
+          new FieldArgsToInputTypeRewriter({
+            fieldName: 'makePokemon',
+            argNames: ['name']
+          }),
+          new NestFieldOutputsRewriter({
+            fieldName: 'makePokemon',
+            newOutputName: 'pokemon',
+            outputsToNest: ['id', 'name']
+          })
+        ]
+      })
+    );
+
+    app.use('/graphql', (req, res) => {
+      const messedUpRes = Buffer.from('jisdhfiods{{{{', 'utf8');
+      res.setHeader('Content-Type', 'application/json; charset=utf-8');
+      res.setHeader('Content-Length', String(messedUpRes.length));
+      res.end(messedUpRes);
+    });
+
+    const deprecatedQuery = `
+      mutation {
+        makePokemon(name: "Squirtle") {
+          id
+          name
+        }
+      }
+    `;
+
+    const invalidQueryRes = await request(app)
+      .post('/graphql')
+      .send({ query: deprecatedQuery })
+      // disable supertest json parsing
+      .buffer(true)
+      .parse((res, cb) => {
+        let data = Buffer.from('');
+        res.on('data', chunk => {
+          data = Buffer.concat([data, chunk]);
+        });
+        res.on('end', () => {
+          cb(null, data.toString());
+        });
+      });
+
+    expect(invalidQueryRes.body).toEqual('jisdhfiods{{{{');
+  });
+
+  it('is able to rewriter responses with pretty printing enabled on express-graphql', async () => {
+    const app = setupMutationApp({ pretty: true });
+    // in the past, we didn't use input or output types correctly
+    // so we need to rewrite the query to this old query will work still
+    const deprecatedQuery = `
+      mutation makePokemonWithWrongType($name: String!) {
+        makePokemon(name: $name) {
+          id
+          name
+        }
+      }
+    `;
+
+    const deprecatedRes = await request(app)
+      .post('/graphql')
+      .send({ query: deprecatedQuery, variables: { name: 'Squirtle' } });
+    expect(deprecatedRes.body.errors).toBe(undefined);
+    expect(deprecatedRes.body.data.makePokemon).toEqual({
+      id: '17',
+      name: 'Squirtle'
+    });
+
+    // the new version of the query should still work with no problem though
+    const newQuery = `
+      mutation makePokemon($input: MakePokemonInput!) {
+        makePokemon(input: $input) {
+          pokemon {
+            id
+            name
+          }
+        }
+      }
+    `;
+
+    const newRes = await request(app)
+      .post('/graphql')
+      .send({ query: newQuery, variables: { input: { name: 'Squirtle' } } });
+    expect(newRes.body.errors).toBe(undefined);
+    expect(newRes.body.data.makePokemon).toEqual({
+      pokemon: {
+        id: '17',
+        name: 'Squirtle'
+      }
+    });
+  });
+
   it('throws on invalid graphql if ignoreParsingErrors === false', async () => {
     const app = express();