fix(openapi): return rejected Promise if spec uploads fail (#409)

* fix(openapi): properly return rejected promise if spec uploads fail * test: enhance test suite for upload errors - Refactored tests to check for promise rejections and to inspect error objects, rather than checking for console.log outputs - Also added tests for edge cases (i.e. non-JSON error messages, spec timeout errors) * chore: reformat error messages to be surrounded by newlines We were doing this in the console logging that I removed in 0f2372e, but I liked that formatting so I'm just having us do that globally. * fix: bring back some instructions that I don't think were ever being logged
readmeio · Dec 17, 2021 · e282b54 · e282b54
1 parent fa1ccfd
commit e282b54
Show file tree

Hide file tree

Showing 4 changed files with 80 additions and 45 deletions.
diff --git a/__tests__/cmds/openapi.test.js b/__tests__/cmds/openapi.test.js
@@ -4,6 +4,7 @@ const fs = require('fs');
 const promptHandler = require('../../src/lib/prompts');
 const swagger = require('../../src/cmds/swagger');
 const openapi = require('../../src/cmds/openapi');
+const APIError = require('../../src/lib/apiError');
 
 const key = 'API_KEY';
 const id = '5aa0409b7cf527a93bfb44df';
@@ -272,7 +273,14 @@ describe('rdme openapi', () => {
       ).rejects.toMatchSnapshot();
     });
 
-    it('should throw an error if an in valid Swagger definition is supplied', () => {
+    it('should throw an error if an invalid Swagger definition is supplied', async () => {
+      const errorObject = {
+        error: 'INTERNAL_ERROR',
+        message: 'Unknown error (README VALIDATION ERROR "x-samples-languages" must be of type "Array")',
+        suggestion: '...a suggestion to resolve the issue...',
+        help: 'If you need help, email [email protected] and mention log "fake-metrics-uuid".',
+      };
+
       const mock = nock(config.host)
         .get(`/api/v1/version/${version}`)
         .basicAuth({ user: key })
@@ -283,30 +291,48 @@ describe('rdme openapi', () => {
         .post('/api/v1/api-specification', body => body.match('form-data; name="spec"'))
         .delayConnection(1000)
         .basicAuth({ user: key })
-        .reply(500, {
-          error: 'INTERNAL_ERROR',
-          message: 'Unknown error (README VALIDATION ERROR "x-samples-languages" must be of type "Array")',
-          suggestion: '...a suggestion to resolve the issue...',
-          help: 'If you need help, email [email protected] and mention log "fake-metrics-uuid".',
-        });
+        .reply(500, errorObject);
 
-      return openapi
-        .run({
+      await expect(
+        openapi.run({
           spec: './__tests__/__fixtures__/swagger-with-invalid-extensions.json',
           key,
           version,
         })
-        .then(() => {
-          expect(console.log).toHaveBeenCalledTimes(1);
+      ).rejects.toStrictEqual(new APIError(errorObject));
 
-          const output = getCommandOutput();
-          expect(output).toMatch(/Unknown error \(README VALIDATION ERROR "x-samples-languages" /);
+      mock.done();
+    });
 
-          mock.done();
-        });
+    it('should error if API errors', async () => {
+      const errorObject = {
+        error: 'SPEC_VERSION_NOTFOUND',
+        message:
+          "The version you specified ({version}) doesn't match any of the existing versions ({versions_list}) in ReadMe.",
+        suggestion: '...a suggestion to resolve the issue...',
+        help: 'If you need help, email [email protected] and mention log "fake-metrics-uuid".',
+      };
+
+      const mock = nock(config.host)
+        .get(`/api/v1/version/${version}`)
+        .basicAuth({ user: key })
+        .reply(200, { version: '1.0.0' })
+        .get('/api/v1/api-specification')
+        .basicAuth({ user: key })
+        .reply(200, [])
+        .post('/api/v1/api-specification', body => body.match('form-data; name="spec"'))
+        .delayConnection(1000)
+        .basicAuth({ user: key })
+        .reply(400, errorObject);
+
+      await expect(
+        openapi.run({ spec: require.resolve('@readme/oas-examples/2.0/json/petstore.json'), key, version })
+      ).rejects.toStrictEqual(new APIError(errorObject));
+
+      mock.done();
     });
 
-    it('should error if API errors', () => {
+    it('should error if API errors (generic upload error)', async () => {
       const mock = nock(config.host)
         .get(`/api/v1/version/${version}`)
         .basicAuth({ user: key })
@@ -317,24 +343,37 @@ describe('rdme openapi', () => {
         .post('/api/v1/api-specification', body => body.match('form-data; name="spec"'))
         .delayConnection(1000)
         .basicAuth({ user: key })
-        .reply(400, {
-          error: 'SPEC_VERSION_NOTFOUND',
-          message:
-            "The version you specified ({version}) doesn't match any of the existing versions ({versions_list}) in ReadMe.",
-          suggestion: '...a suggestion to resolve the issue...',
-          help: 'If you need help, email [email protected] and mention log "fake-metrics-uuid".',
-        });
+        .reply(400, 'some non-JSON upload error');
 
-      return openapi
-        .run({ spec: require.resolve('@readme/oas-examples/2.0/json/petstore.json'), key, version })
-        .then(() => {
-          expect(console.log).toHaveBeenCalledTimes(1);
+      await expect(
+        openapi.run({ spec: require.resolve('@readme/oas-examples/2.0/json/petstore.json'), key, version })
+      ).rejects.toStrictEqual(new Error('There was an error uploading!'));
 
-          const output = getCommandOutput();
-          expect(output).toMatch(/The version you specified/);
+      mock.done();
+    });
 
-          mock.done();
-        });
+    it('should error if API errors (request timeout)', async () => {
+      const mock = nock(config.host)
+        .get(`/api/v1/version/${version}`)
+        .basicAuth({ user: key })
+        .reply(200, { version: '1.0.0' })
+        .get('/api/v1/api-specification')
+        .basicAuth({ user: key })
+        .reply(200, [])
+        .post('/api/v1/api-specification', body => body.match('form-data; name="spec"'))
+        .delayConnection(1000)
+        .basicAuth({ user: key })
+        .reply(400, '<html></html>');
+
+      await expect(
+        openapi.run({ spec: require.resolve('@readme/oas-examples/2.0/json/petstore.json'), key, version })
+      ).rejects.toStrictEqual(
+        new Error(
+          "We're sorry, your upload request timed out. Please try again or split your file up into smaller chunks."
+        )
+      );
+
+      mock.done();
     });
   });
 });

diff --git a/bin/rdme b/bin/rdme
@@ -11,7 +11,7 @@ require('../src')(process.argv.slice(2))
       const err = e;
 
       if ('message' in err) {
-        console.error(chalk.red(err.message));
+        console.error(chalk.red(`\n${err.message}\n`));
       } else {
         console.error(
           chalk.red(

diff --git a/src/cmds/openapi.js b/src/cmds/openapi.js
@@ -150,23 +150,19 @@ exports.run = async function (opts) {
 
     function createSpec() {
       options.method = 'post';
-      return fetch(`${config.host}/api/v1/api-specification`, options)
-        .then(res => {
-          if (res.ok) return success(res);
-          return error(res);
-        })
-        .catch(err => console.log(chalk.red(`\n ${err.message}\n`)));
+      return fetch(`${config.host}/api/v1/api-specification`, options).then(res => {
+        if (res.ok) return success(res);
+        return error(res);
+      });
     }
 
     function updateSpec(specId) {
       isUpdate = true;
       options.method = 'put';
-      return fetch(`${config.host}/api/v1/api-specification/${specId}`, options)
-        .then(res => {
-          if (res.ok) return success(res);
-          return error(res);
-        })
-        .catch(err => console.log(chalk.red(`\n ${err.message}\n`)));
+      return fetch(`${config.host}/api/v1/api-specification/${specId}`, options).then(res => {
+        if (res.ok) return success(res);
+        return error(res);
+      });
     }
 
     /*

diff --git a/src/index.js b/src/index.js
@@ -113,7 +113,7 @@ module.exports = processArgv => {
     return bin.run(cmdArgv);
   } catch (e) {
     if (e.message === 'Command not found.') {
-      e.description = `Type \`${chalk.yellow(`${config.cli} help`)}\` ${chalk.red('to see all commands')}`;
+      e.message = `${e.message}\n\nType \`${chalk.yellow(`${config.cli} help`)}\` ${chalk.red('to see all commands')}`;
     }
 
     return Promise.reject(e);