feat: automatically parse the api response based on content-type (#240)

* refactor: switch to using memfs instead of mock-fs

mock-fs has limitations which means you can use console.log during test runs
which is not ideal! Ended up doing something like this to get it to work:

tschaub/mock-fs#234 (comment)

* feat: automatically parse the api response based on content-type

This allows us to remove the `.then(res => res.json())` line from our
code samples.

```
const sdk = require('api')('@readme/5f7ceacf43621b0311080a58');

sdk.getAPISpecification({perPage: '10', page: '1'})
  .then(res => console.log(res))
  .catch(err => console.error(err));
```

We only perform the parsing if it's a successful response. If there's an error
case then we return with the original response object so you can access
the status as well as the actual body.

The code and tests for this have been stolen verbatim from the explorer:
https://github.com/readmeio/api-explorer/blob/77b90ebed4673f168354cdcd730e34b7ee016360/packages/api-explorer/src/lib/parse-response.js#L13-L30

BREAKING CHANGE: this is a breaking change.

* chore: relax commitlint rules on body and footer length

Taken from main codebase

* feat: remove res.json() line from the httpsnippet client

* fix: always output `.then(res => console.log(res))` in code sample

Since we dont know if the response is json or not, we can't make
assumptions. In an ideal world we'd conditionally do this based
on the accept header in the response, but Operation.getHeaders() only
returns with an array of headers and not their actual values. I think
this is good enough for now!

#240 (comment)

package.json

@@ -32,6 +32,18 @@
   "commitlint": {
     "extends": [
       "@commitlint/config-conventional"
-    ]
+    ],
+    "rules": {
+      "body-max-line-length": [
+        0,
+        "always",
+        "Infinity"
+      ],
+      "footer-max-line-length": [
+        0,
+        "always",
+        "Infinity"
+      ]
+    }
   }
 }

packages/api/__tests__/auth.test.js

@@ -42,17 +42,11 @@ describe('#auth()', () => {
           return sdk
             .auth(apiKey)
             .getSomething()
-            .then(res => {
-              expect(res.status).toBe(200);
-              mock.done();
-            });
+            .then(() => mock.done());
         }
 
         sdk.auth(apiKey);
-        return sdk.getSomething().then(res => {
-          expect(res.status).toBe(200);
-          mock.done();
-        });
+        return sdk.getSomething().then(() => mock.done());
       });
 
       it('should throw if you supply multiple auth keys', () => {
@@ -89,17 +83,11 @@ describe('#auth()', () => {
           return sdk
             .auth(apiKey)
             .getSomething()
-            .then(res => {
-              expect(res.status).toBe(200);
-              mock.done();
-            });
+            .then(() => mock.done());
         }
 
         sdk.auth(apiKey);
-        return sdk.getSomething().then(res => {
-          expect(res.status).toBe(200);
-          mock.done();
-        });
+        return sdk.getSomething().then(() => mock.done());
       });
 
       it('should throw if you supply multiple auth keys', () => {
@@ -135,21 +123,21 @@ describe('#auth()', () => {
           reqheaders: { authorization: `Basic ${Buffer.from(`${user}:${pass}`).toString('base64')}` },
         })
           .get('/')
-          .reply(200, {});
+          .reply(200, { id: 1 });
 
         if (chained) {
           return sdk
             .auth(user, pass)
             .getSomething()
             .then(res => {
-              expect(res.status).toBe(200);
+              expect(res.id).toBe(1);
               mock.done();
             });
         }
 
         sdk.auth(user, pass);
         return sdk.getSomething().then(res => {
-          expect(res.status).toBe(200);
+          expect(res.id).toBe(1);
           mock.done();
         });
       });
@@ -165,10 +153,7 @@ describe('#auth()', () => {
         return sdk
           .auth(user)
           .getSomething()
-          .then(res => {
-            expect(res.status).toBe(200);
-            mock.done();
-          });
+          .then(() => mock.done());
       });
     });
 
@@ -199,17 +184,11 @@ describe('#auth()', () => {
           return sdk
             .auth(apiKey)
             .getSomething()
-            .then(res => {
-              expect(res.status).toBe(200);
-              mock.done();
-            });
+            .then(() => mock.done());
         }
 
         sdk.auth(apiKey);
-        return sdk.getSomething().then(res => {
-          expect(res.status).toBe(200);
-          mock.done();
-        });
+        return sdk.getSomething().then(() => mock.done());
       });
 
       it('should throw if you pass in multiple bearer tokens', () => {
@@ -246,17 +225,11 @@ describe('#auth()', () => {
         return sdk
           .auth(apiKey)
           .getSomething()
-          .then(res => {
-            expect(res.status).toBe(200);
-            mock.done();
-          });
+          .then(() => mock.done());
       }
 
       sdk.auth(apiKey);
-      return sdk.getSomething().then(res => {
-        expect(res.status).toBe(200);
-        mock.done();
-      });
+      return sdk.getSomething().then(() => mock.done());
     });
 
     it('should throw if you pass in multiple bearer tokens', () => {

packages/api/__tests__/index.test.js

@@ -173,7 +173,7 @@ describe('#fetch', () => {
   });
 
   it('should contain a custom user agent for the library in requests', () => {
-    expect.assertions(2);
+    expect.assertions(1);
 
     const userAgent = `${pkg.name} (node)/${pkg.version}`;
     const mock = nock(petstoreServerUrl, {
@@ -186,10 +186,7 @@ describe('#fetch', () => {
         expect(this.req.headers['user-agent']).toStrictEqual([userAgent]);
       });
 
-    return petstoreSdk.deletePet({ id: petId }).then(res => {
-      expect(res.status).toBe(200);
-      mock.done();
-    });
+    return petstoreSdk.deletePet({ id: petId }).then(() => mock.done());
   });
 
   describe('operationId', () => {
@@ -201,26 +198,17 @@ describe('#fetch', () => {
 
       const mock = nock(petstoreServerUrl).delete(`/pets/${petId}`).reply(200, response);
 
-      return petstoreSdk
-        .deletePet({ id: petId })
-        .then(res => {
-          expect(res.status).toBe(200);
-          return res.json();
-        })
-        .then(res => {
-          expect(res).toStrictEqual(response);
-          mock.done();
-        });
+      return petstoreSdk.deletePet({ id: petId }).then(res => {
+        expect(res).toStrictEqual(response);
+        mock.done();
+      });
     });
 
     it('should pass through body for operationId', () => {
       const body = { name: 'Buster' };
       const mock = nock(petstoreServerUrl).post('/pets', body).reply(200);
 
-      return petstoreSdk.addPet(body).then(res => {
-        expect(res.status).toBe(200);
-        mock.done();
-      });
+      return petstoreSdk.addPet(body).then(() => mock.done());
     });
 
     it('should pass through parameters and body for operationId', () => {
@@ -232,10 +220,7 @@ describe('#fetch', () => {
 
       const mock = nock('https://dash.readme.io/api/v1').put(`/changelogs/${slug}`, body).reply(200);
 
-      return readmeSdk.updateChangelog(body, { slug }).then(res => {
-        expect(res.status).toBe(200);
-        mock.done();
-      });
+      return readmeSdk.updateChangelog(body, { slug }).then(() => mock.done());
     });
   });
 
@@ -252,26 +237,16 @@ describe('#fetch', () => {
           };
         });
 
-      return petstoreSdk
-        .post('/pets', body)
-        .then(res => {
-          expect(res.status).toBe(200);
-          return res.json();
-        })
-        .then(res => {
-          expect(res).toStrictEqual({ id: 100, name: body.name });
-          mock.done();
-        });
+      return petstoreSdk.post('/pets', body).then(res => {
+        expect(res).toStrictEqual({ id: 100, name: body.name });
+        mock.done();
+      });
     });
 
     it('should pass through parameters for method + path', () => {
       const slug = 'new-release';
       const mock = nock('https://dash.readme.io/api/v1').put(`/changelogs/${slug}`).reply(200);
-      return readmeSdk.put('/changelogs/{slug}', { slug }).then(res => {
-        expect(res.status).toBe(200);
-        expect(res.url).toBe(`https://dash.readme.io/api/v1/changelogs/${slug}`);
-        mock.done();
-      });
+      return readmeSdk.put('/changelogs/{slug}', { slug }).then(() => mock.done());
     });
 
     it('should pass through parameters and body for method + path', () => {
@@ -290,17 +265,10 @@ describe('#fetch', () => {
           };
         });
 
-      return readmeSdk
-        .put('/changelogs/{slug}', body, { slug })
-        .then(res => {
-          expect(res.status).toBe(200);
-          expect(res.url).toBe(`https://dash.readme.io/api/v1/changelogs/${slug}`);
-          return res.json();
-        })
-        .then(res => {
-          expect(res).toStrictEqual({ ...body, slug });
-          mock.done();
-        });
+      return readmeSdk.put('/changelogs/{slug}', body, { slug }).then(res => {
+        expect(res).toStrictEqual({ ...body, slug });
+        mock.done();
+      });
     });
   });
 });

packages/api/__tests__/lib/parseResponse.test.js

@@ -0,0 +1,75 @@
+// Nabbed these test cases from here:
+// https://github.com/readmeio/api-explorer/blob/77b90ebed4673f168354cdcd730e34b7ee016360/packages/api-explorer/__tests__/lib/parse-response.test.js#L182-L210
+const { Response } = require('node-fetch');
+const parseResponse = require('../../src/lib/parseResponse');
+
+const responseBody = JSON.stringify({
+  id: 9205436248879918000,
+  category: { id: 0 },
+  name: '1',
+  photoUrls: ['1'],
+  tags: [],
+});
+
+let response;
+
+beforeEach(() => {
+  response = new Response(responseBody, {
+    headers: {
+      'Content-Type': 'application/json',
+      'x-custom-header': 'application/json',
+    },
+  });
+});
+
+describe('#parseResponse', () => {
+  it('should parse application/json response as json', async () => {
+    expect(await parseResponse(response)).toStrictEqual(JSON.parse(responseBody));
+  });
+
+  it('should parse application/vnd.api+json as json', async () => {
+    response.headers['Content-Type'] = 'application/vnd.api+json';
+    expect(await parseResponse(response)).toStrictEqual(JSON.parse(responseBody));
+  });
+
+  it('should parse non-json response as text', async () => {
+    const nonJsonResponseBody = '<xml-string />';
+    const nonJsonResponse = new Response('<xml-string />', {
+      headers: {
+        'Content-Type': 'application/xml',
+      },
+    });
+
+    expect(await parseResponse(nonJsonResponse)).toStrictEqual(nonJsonResponseBody);
+  });
+
+  it('should not error if invalid json is returned', async () => {
+    const invalidJsonResponse = new Response('plain text', {
+      headers: {
+        'Content-Type': 'application/json',
+      },
+    });
+
+    expect(await parseResponse(invalidJsonResponse)).toBe('plain text');
+  });
+
+  it('should default to JSON with wildcard content-type', async () => {
+    const wildcardResponse = new Response(responseBody, {
+      headers: {
+        'Content-Type': '*/*',
+      },
+    });
+
+    expect(await parseResponse(wildcardResponse)).toStrictEqual(JSON.parse(responseBody));
+  });
+
+  it('should return with empty string if there is no response', async () => {
+    const emptyResponse = new Response(null, {
+      headers: {
+        'Content-Type': 'application/json',
+      },
+    });
+
+    expect(await parseResponse(emptyResponse)).toStrictEqual('');
+  });
+});

packages/api/src/index.js

@@ -5,7 +5,7 @@ const oasToHar = require('@readme/oas-to-har');
 const pkg = require('../package.json');
 
 const Cache = require('./cache');
-const { prepareAuth, prepareParams } = require('./lib/index');
+const { prepareAuth, prepareParams, parseResponse } = require('./lib/index');
 
 global.fetch = fetch;
 global.Request = fetch.Request;
@@ -48,7 +48,7 @@ class Sdk {
             throw res;
           }
 
-          return res;
+          return parseResponse(res);
         });
       });
     }

packages/api/src/lib/index.js

@@ -1,7 +1,9 @@
 const prepareAuth = require('./prepareAuth');
 const prepareParams = require('./prepareParams');
+const parseResponse = require('./parseResponse');
 
 module.exports = {
   prepareAuth,
   prepareParams,
+  parseResponse,
 };

packages/api/src/lib/parseResponse.js

@@ -0,0 +1,22 @@
+// Nabbed from here:
+// https://github.com/readmeio/api-explorer/blob/77b90ebed4673f168354cdcd730e34b7ee016360/packages/api-explorer/src/lib/parse-response.js#L13-L30
+const { matchesMimeType } = require('oas/tooling/utils');
+
+module.exports = async function getResponseBody(response) {
+  const contentType = response.headers.get('Content-Type');
+  const isJson = contentType && (matchesMimeType.json(contentType) || matchesMimeType.wildcard(contentType));
+
+  // We have to clone it before reading, just incase
+  // we cannot parse it as JSON later, then we can
+  // re-read again as plain text
+  const clonedResponse = response.clone();
+  let responseBody;
+
+  try {
+    responseBody = await response[isJson ? 'json' : 'text']();
+  } catch (e) {
+    responseBody = await clonedResponse.text();
+  }
+
+  return responseBody;
+};

packages/httpsnippet-client-api/__tests__/__fixtures__/output/application-form-encoded.js

@@ -1,6 +1,5 @@
 const sdk = require('api')('https://example.com/openapi.json');
 
 sdk.post('/har', {foo: 'bar', hello: 'world'})
-  .then(res => res.json())
-  .then(json => console.log(json))
+  .then(res => console.log(res))
   .catch(err => console.error(err));

packages/httpsnippet-client-api/__tests__/__fixtures__/output/application-json.js

@@ -8,6 +8,5 @@ sdk.post('/har', {
   arr_mix: [1, 'a', {arr_mix_nested: {}}],
   boolean: false
 })
-  .then(res => res.json())
-  .then(json => console.log(json))
+  .then(res => console.log(res))
   .catch(err => console.error(err));