Skip to content

Commit

Permalink
feat: overhauling error handling to support the new api error respons…
Browse files Browse the repository at this point in the history
…es (#218)

* feat: overhauling error handling to support the new api error responses

* test: fixing some broken tests
  • Loading branch information
erunion authored Jul 15, 2020
1 parent 634edea commit 0a5e92b
Show file tree
Hide file tree
Showing 15 changed files with 176 additions and 107 deletions.
17 changes: 3 additions & 14 deletions cmds/docs/edit.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ const config = require('config');
const fs = require('fs');
const editor = require('editor');
const { promisify } = require('util');
const APIError = require('../../lib/apiError');

const writeFile = promisify(fs.writeFile);
const readFile = promisify(fs.readFile);
Expand Down Expand Up @@ -61,13 +62,7 @@ exports.run = async function (opts) {
json: true,
...options,
})
.catch(err => {
if (err.statusCode === 404) {
return Promise.reject(err.error);
}

return Promise.reject(err);
});
.catch(err => Promise.reject(new APIError(err)));

await writeFile(filename, existingDoc.body);

Expand All @@ -88,13 +83,7 @@ exports.run = async function (opts) {
await unlink(filename);
return resolve();
})
.catch(err => {
if (err.statusCode === 400) {
return reject(err.error);
}

return reject(err);
});
.catch(err => reject(new APIError(err)));
});
});
};
21 changes: 8 additions & 13 deletions cmds/docs/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ const config = require('config');
const crypto = require('crypto');
const frontMatter = require('gray-matter');
const { promisify } = require('util');
const APIError = require('../../lib/apiError');

const readFile = promisify(fs.readFile);

Expand Down Expand Up @@ -49,6 +50,9 @@ exports.run = function (opts) {
}

const files = fs.readdirSync(folder).filter(file => file.endsWith('.md') || file.endsWith('.markdown'));
if (files.length === 0) {
return Promise.reject(new Error(`We were unable to locate Markdown files in ${folder}.`));
}

const options = {
auth: { user: key },
Expand All @@ -57,14 +61,6 @@ exports.run = function (opts) {
},
};

function validationErrors(err) {
if (err.statusCode === 400) {
return Promise.reject(err.error);
}

return Promise.reject(err);
}

function createDoc(slug, file, hash, err) {
if (err.statusCode !== 404) return Promise.reject(err.error);

Expand All @@ -73,13 +69,14 @@ exports.run = function (opts) {
json: { slug, body: file.content, ...file.data, lastUpdatedHash: hash },
...options,
})
.catch(validationErrors);
.catch(err => Promise.reject(new APIError(err)));
}

function updateDoc(slug, file, hash, existingDoc) {
if (hash === existingDoc.lastUpdatedHash) {
return `\`${slug}\` not updated. No changes.`;
}

return request
.put(`${config.host}/api/v1/docs/${slug}`, {
json: Object.assign(existingDoc, {
Expand All @@ -89,7 +86,7 @@ exports.run = function (opts) {
}),
...options,
})
.catch(validationErrors);
.catch(err => Promise.reject(new APIError(err)));
}

return Promise.all(
Expand All @@ -106,9 +103,7 @@ exports.run = function (opts) {
...options,
})
.then(updateDoc.bind(null, slug, matter, hash), createDoc.bind(null, slug, matter, hash))
.catch(err => {
return Promise.reject(err);
});
.catch(err => Promise.reject(new APIError(err)));
})
);
};
11 changes: 2 additions & 9 deletions cmds/login.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ const { validate: isEmail } = require('isemail');
const { promisify } = require('util');
const read = promisify(require('read'));
const configStore = require('../lib/configstore');
const APIError = require('../lib/apiError');

const testing = process.env.NODE_ENV === 'testing';

Expand Down Expand Up @@ -53,14 +54,6 @@ exports.run = async function (opts) {
return Promise.reject(new Error('You must provide a valid email address.'));
}

function badRequest(err) {
if (err.statusCode === 400) {
return Promise.reject(err.error);
}

return Promise.reject(err);
}

return request
.post(`${config.host}/api/v1/login`, {
json: { email, password, project, token },
Expand All @@ -72,5 +65,5 @@ exports.run = async function (opts) {

return `Successfully logged in as ${email.green} to the ${project.blue} project.`;
})
.catch(badRequest);
.catch(err => Promise.reject(new APIError(err)));
};
7 changes: 4 additions & 3 deletions cmds/swagger.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ const path = require('path');
const config = require('config');
const { prompt } = require('enquirer');
const promptOpts = require('../lib/prompts');
const APIError = require('../lib/apiError');

exports.command = 'swagger';
exports.usage = 'swagger [file] [options]';
Expand Down Expand Up @@ -82,7 +83,7 @@ exports.run = async function (opts) {
function error(err) {
try {
const parsedError = JSON.parse(err.error);
return Promise.reject(new Error(parsedError.description || parsedError.error));
return Promise.reject(new APIError(parsedError));
} catch (e) {
return Promise.reject(new Error('There was an error uploading!'));
}
Expand Down Expand Up @@ -162,8 +163,8 @@ exports.run = async function (opts) {
await request.post(`${config.host}/api/v1/version`, options);

return newVersion;
} catch (e) {
return Promise.reject(e.error);
} catch (err) {
return Promise.reject(new APIError(err));
}
}

Expand Down
13 changes: 3 additions & 10 deletions cmds/versions/create.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ const config = require('config');
const semver = require('semver');
const { prompt } = require('enquirer');
const promptOpts = require('../../lib/prompts');
const APIError = require('../../lib/apiError');

exports.command = 'versions:create';
exports.usage = 'versions:create --version=<version> [options]';
Expand Down Expand Up @@ -69,7 +70,7 @@ exports.run = async function (opts) {
json: true,
auth: { user: key },
})
.catch(e => Promise.reject(e.error));
.catch(err => Promise.reject(new APIError(err)));
}

const versionPrompt = promptOpts.createVersionPrompt(versionList || [{}], {
Expand All @@ -93,13 +94,5 @@ exports.run = async function (opts) {
return request
.post(`${config.host}/api/v1/version`, options)
.then(() => Promise.resolve(`Version ${version} created successfully.`))
.catch(err => {
return Promise.reject(
new Error(
err.error && err.error.description
? err.error.description
: 'Failed to create a new version using your specified parameters.'
)
);
});
.catch(err => Promise.reject(new APIError(err)));
};
8 changes: 3 additions & 5 deletions cmds/versions/delete.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
const request = require('request-promise-native');
const config = require('config');
const semver = require('semver');
const APIError = require('../../lib/apiError');

exports.command = 'versions:delete';
exports.usage = 'versions:delete --version=<version> [options]';
Expand Down Expand Up @@ -37,12 +38,9 @@ exports.run = async function (opts) {

return request
.delete(`${config.host}/api/v1/version/${version}`, {
json: true,
auth: { user: key },
})
.then(() => Promise.resolve(`Version ${version} deleted successfully.`))
.catch(err => {
return Promise.reject(
new Error(err.error && err.error.description ? err.error.description : 'Failed to delete target version.')
);
});
.catch(err => Promise.reject(new APIError(err)));
};
4 changes: 3 additions & 1 deletion cmds/versions/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ const request = require('request-promise-native');
const Table = require('cli-table');
const config = require('config');
const versionsCreate = require('./create');
const APIError = require('../../lib/apiError');

exports.command = 'versions';
exports.usage = 'versions [options]';
Expand Down Expand Up @@ -116,5 +117,6 @@ exports.run = function (opts) {
}

return Promise.resolve(getVersionFormatted(versions[0]));
});
})
.catch(err => Promise.reject(new APIError(err)));
};
6 changes: 4 additions & 2 deletions cmds/versions/update.js
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ const config = require('config');
const semver = require('semver');
const { prompt } = require('enquirer');
const promptOpts = require('../../lib/prompts');
const APIError = require('../../lib/apiError');

exports.command = 'versions:update';
exports.usage = 'versions:update --version=<version> [options]';
Expand Down Expand Up @@ -61,7 +62,7 @@ exports.run = async function (opts) {
json: true,
auth: { user: key },
})
.catch(e => Promise.reject(e.error));
.catch(err => Promise.reject(new APIError(err)));

const promptResponse = await prompt(promptOpts.createVersionPrompt([{}], opts, foundVersion));
const options = {
Expand All @@ -78,5 +79,6 @@ exports.run = async function (opts) {

return request
.put(`${config.host}/api/v1/version/${version}`, options)
.then(() => Promise.resolve(`Version ${version} updated successfully.`));
.then(() => Promise.resolve(`Version ${version} updated successfully.`))
.catch(err => Promise.reject(new APIError(err)));
};
35 changes: 35 additions & 0 deletions lib/apiError.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
module.exports = class extends Error {
constructor(res) {
let err;

// Special handling to for fetch `res` arguments where `res.error` will contain our API error response.
if (typeof res === 'object') {
if ('error' in res && typeof res.error === 'object') {
err = res.error;
} else {
err = res;
}
} else {
err = res;
}

super(err);

if (typeof err === 'object') {
this.code = err.error;

// If we returned help info in the API, show it otherwise don't render out multiple empty lines as we sometimes
// throw `Error('non-api custom error message')` instances and catch them with this class.
if ('help' in err) {
this.message = [err.message, '', err.help].join('\n');
} else {
this.message = err.message;
}

this.name = 'APIError';
} else {
this.code = err;
this.message = err;
}
}
};
26 changes: 3 additions & 23 deletions rdme.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,37 +8,17 @@ require('./cli')(process.argv.slice(2))
})
.catch(e => {
if (e) {
// `err.message` is from locally thrown Error objects
// `err.error` is from remote API errors
const err = e;

// If we've got a remote API error, extract its contents so we can show the user the error.
if (typeof err.error === 'object' && Object.keys(err.error).length === 3) {
err.message = err.error.error;
err.description = err.error.description;
err.errors = err.error.errors;
}

if (!err.description && !err.errors && err.error) {
if ('message' in err) {
console.error(err.message.red);
} else {
console.error(
`Yikes, something went wrong! Please try again and if the problem persists, get in touch with our support team at ${
`[email protected]`.underline
}.\n`.red
);
}

if (err.message && (typeof err.statusCode === 'undefined' || err.statusCode !== 404))
console.error(err.message.red);

if (err.description) console.error(err.description.red);
if (err.errors) {
const errors = Object.keys(err.errors);

console.error(`\nCause${(errors.length > 1 && 's') || ''}:`.red.bold);
errors.forEach(error => {
console.error(` · ${error}: ${err.errors[error]}`.red);
});
}
}

return process.exit(1);
Expand Down
26 changes: 19 additions & 7 deletions test/cmds/docs.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -118,8 +118,10 @@ describe('rdme docs', () => {
.get(`/api/v1/docs/${slug}`)
.basicAuth({ user: key })
.reply(404, {
description: 'No doc found with that slug',
error: 'Not Found',
error: 'DOC_NOTFOUND',
message: `The doc with the slug '${slug}' couldn't be found`,
suggestion: '...a suggestion to resolve the issue...',
help: 'If you need help, email [email protected] and mention log "fake-metrics-uuid".',
});

const postMock = nock(config.host, {
Expand Down Expand Up @@ -211,12 +213,17 @@ describe('rdme docs:edit', () => {

const getMock = nock(config.host)
.get(`/api/v1/docs/${slug}`)
.reply(404, { error: 'Not Found', description: 'No doc found with that slug' });
.reply(404, {
error: 'DOC_NOTFOUND',
message: `The doc with the slug '${slug}' couldn't be found`,
suggestion: '...a suggestion to resolve the issue...',
help: 'If you need help, email [email protected] and mention log "fake-metrics-uuid".',
});

return docsEdit.run({ slug, key, version: '1.0.0' }).catch(err => {
getMock.done();
expect(err.error).toBe('Not Found');
expect(err.description).toBe('No doc found with that slug');
expect(err.code).toBe('DOC_NOTFOUND');
expect(err.message).toContain("The doc with the slug 'no-such-doc' couldn't be found");
});
});

Expand All @@ -226,14 +233,19 @@ describe('rdme docs:edit', () => {

const getMock = nock(config.host).get(`/api/v1/docs/${slug}`).reply(200, {});

const putMock = nock(config.host).put(`/api/v1/docs/${slug}`).reply(400, { error: 'Bad Request' });
const putMock = nock(config.host).put(`/api/v1/docs/${slug}`).reply(400, {
error: 'DOC_INVALID',
message: "We couldn't save this doc ({error})",
suggestion: '...a suggestion to resolve the issue...',
help: 'If you need help, email [email protected] and mention log "fake-metrics-uuid".',
});

function mockEditor(filename, cb) {
return cb(0);
}

return docsEdit.run({ slug, key, version: '1.0.0', mockEditor }).catch(err => {
expect(err.error).toBe('Bad Request');
expect(err.code).toBe('DOC_INVALID');
getMock.done();
putMock.done();
expect(fs.existsSync(`${slug}.md`)).toBe(true);
Expand Down
Loading

0 comments on commit 0a5e92b

Please sign in to comment.