From e282b5425adcf33516ceb2fec47db048d4f65488 Mon Sep 17 00:00:00 2001 From: Kanad Gupta Date: Fri, 17 Dec 2021 14:59:17 -0800 Subject: [PATCH] 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 0f2372ed1fc0b94ddba546a5a162b35297c0444b, 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 --- __tests__/cmds/openapi.test.js | 101 +++++++++++++++++++++++---------- bin/rdme | 2 +- src/cmds/openapi.js | 20 +++---- src/index.js | 2 +- 4 files changed, 80 insertions(+), 45 deletions(-) diff --git a/__tests__/cmds/openapi.test.js b/__tests__/cmds/openapi.test.js index ab640bda8..39fe86df0 100644 --- 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 support@readme.io 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 support@readme.io 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 support@readme.io 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 support@readme.io 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, ''); + + 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 index 562142f4b..a4df41dec 100755 --- 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 index 00a63205b..7b9aa9937 100644 --- 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 index 710a15e9d..1dd650037 100644 --- 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);