New API responses with good errors

This commit is contained in:
Jori Lallo
2016-09-17 14:53:30 -07:00
parent e631025887
commit fcc83dd2d6
11 changed files with 86 additions and 50 deletions

View File

@ -73,7 +73,7 @@ class MarkdownEditor extends React.Component {
formData.append('file', file); formData.append('file', file);
} }
fetch(data.upload_url, { fetch(data.uploadUrl, {
method: 'post', method: 'post',
body: formData, body: formData,
}) })

View File

@ -64,22 +64,14 @@ class ApiClient {
// Handle 401, log out user // Handle 401, log out user
if (response.status === 401) { if (response.status === 401) {
stores.user.logout(); return stores.user.logout();
} }
// Handle failed responses // Handle failed responses
let error; const error = {};
try {
// Expect API to return JSON
error = JSON.parse(response);
} catch (e) {
// Expect call to fail without JSON response
error = { error: response.statusText };
}
error.statusCode = response.status; error.statusCode = response.status;
error.response = response; error.response = response;
return reject(error); throw error;
}) })
.then((response) => { .then((response) => {
return response.json(); return response.json();
@ -91,8 +83,12 @@ class ApiClient {
} }
resolve(json); resolve(json);
}) })
.catch(() => { .catch(error => {
reject({ error: 'Unknown error' }); error.response.json()
.then(json => {
error.data = json;
reject(error);
});
}); });
}); });
} }

View File

@ -18,49 +18,63 @@ Object {
exports[`#auth.login should require either username or email 1`] = ` exports[`#auth.login should require either username or email 1`] = `
Object { Object {
"error": "username or email is required", "error": "validation_error",
"ok": false "message": "username/email is required",
"ok": false,
"status": 400
} }
`; `;
exports[`#auth.login should require password 1`] = ` exports[`#auth.login should require password 1`] = `
Object { Object {
"error": "password is required", "error": "validation_error",
"ok": false "message": "username/email is required",
"ok": false,
"status": 400
} }
`; `;
exports[`#auth.login should validate password 1`] = ` exports[`#auth.login should validate password 1`] = `
Object { Object {
"error": "Invalid password", "error": "validation_error",
"ok": false "message": "username/email is required",
"ok": false,
"status": 400
} }
`; `;
exports[`#auth.signup should require params 1`] = ` exports[`#auth.signup should require params 1`] = `
Object { Object {
"error": "name is required", "error": "validation_error",
"ok": false "message": "name is required",
"ok": false,
"status": 400
} }
`; `;
exports[`#auth.signup should require unique email 1`] = ` exports[`#auth.signup should require unique email 1`] = `
Object { Object {
"error": "User already exists with this email", "error": "user_exists_with_email",
"ok": false "message": "User already exists with this email",
"ok": false,
"status": 400
} }
`; `;
exports[`#auth.signup should require unique username 1`] = ` exports[`#auth.signup should require unique username 1`] = `
Object { Object {
"error": "User already exists with this username", "error": "user_exists_with_username",
"ok": false "message": "User already exists with this username",
"ok": false,
"status": 400
} }
`; `;
exports[`#auth.signup should require valid email 1`] = ` exports[`#auth.signup should require valid email 1`] = `
Object { Object {
"error": "email is invalid", "error": "validation_error",
"ok": false "message": "email is invalid",
"ok": false,
"status": 400
} }
`; `;

View File

@ -1,7 +1,9 @@
exports[`#user.info should require authentication 1`] = ` exports[`#user.info should require authentication 1`] = `
Object { Object {
"error": "Authentication required", "error": "authentication_required",
"ok": false "message": "Authentication required",
"ok": false,
"status": 401
} }
`; `;
@ -13,6 +15,7 @@ Object {
"name": "User 1", "name": "User 1",
"username": "user1" "username": "user1"
}, },
"ok": true "ok": true,
"status": 200
} }
`; `;

View File

@ -1,5 +1,6 @@
import Router from 'koa-router'; import Router from 'koa-router';
import httpErrors from 'http-errors'; import Sequelize from 'sequelize';
import apiError, { httpErrors } from '../errors';
import fetch from 'isomorphic-fetch'; import fetch from 'isomorphic-fetch';
import querystring from 'querystring'; import querystring from 'querystring';
@ -18,11 +19,11 @@ router.post('auth.signup', async (ctx) => {
ctx.assertPresent(password, 'password is required'); ctx.assertPresent(password, 'password is required');
if (await User.findOne({ where: { email } })) { if (await User.findOne({ where: { email } })) {
throw httpErrors.BadRequest('User already exists with this email'); throw apiError(400, 'user_exists_with_email', 'User already exists with this email');
} }
if (await User.findOne({ where: { username } })) { if (await User.findOne({ where: { username } })) {
throw httpErrors.BadRequest('User already exists with this username'); throw apiError(400, 'user_exists_with_username', 'User already exists with this username');
} }
const user = await User.create({ const user = await User.create({
@ -39,21 +40,31 @@ router.post('auth.signup', async (ctx) => {
}); });
router.post('auth.login', async (ctx) => { router.post('auth.login', async (ctx) => {
const { username, email, password } = ctx.request.body; const { username, password } = ctx.request.body;
ctx.assertPresent(username, 'username/email is required');
ctx.assertPresent(password, 'password is required'); ctx.assertPresent(password, 'password is required');
let user; let user;
if (username) { if (username) {
user = await User.findOne({ where: { username } }); user = await User.findOne({ where: Sequelize.or(
} else if (email) { { email: username },
user = await User.findOne({ where: { email } }); { username },
) });
} else { } else {
throw httpErrors.BadRequest('username or email is required'); throw apiError(400, 'invalid_credentials', 'username or email is invalid');
}
if (!user) {
throw apiError(400, 'username or email is invalid');
}
if (!user.passwordDigest) {
throw apiError(400, 'no_password', 'No password set');
} }
if (!await user.verifyPassword(password)) { if (!await user.verifyPassword(password)) {
throw httpErrors.BadRequest('Invalid password'); throw apiError(400, 'invalid_password', 'Invalid password');
} }
ctx.body = { data: { ctx.body = { data: {
@ -152,8 +163,6 @@ router.post('auth.slackCommands', async (ctx) => {
} }
if (!data.ok) throw httpErrors.BadRequest(data.error); if (!data.ok) throw httpErrors.BadRequest(data.error);
ctx.body = { success: true };
}); });

View File

@ -91,7 +91,7 @@ describe('#auth.login', () => {
await seed(); await seed();
const res = await server.post('/api/auth.login', { const res = await server.post('/api/auth.login', {
body: { body: {
email: 'user1@example.com', username: 'user1@example.com',
password: 'test123!', password: 'test123!',
}, },
}); });

View File

@ -2,6 +2,7 @@ import bodyParser from 'koa-bodyparser';
import Koa from 'koa'; import Koa from 'koa';
import Router from 'koa-router'; import Router from 'koa-router';
import Sequelize from 'sequelize'; import Sequelize from 'sequelize';
import _ from 'lodash';
import auth from './auth'; import auth from './auth';
import user from './user'; import user from './user';
@ -41,7 +42,9 @@ api.use(async (ctx, next) => {
ctx.body = { ctx.body = {
ok: false, ok: false,
error: message, error: _.snakeCase(err.id || err.message),
status: err.status,
message,
}; };
} }
}); });

View File

@ -6,6 +6,7 @@ export default function apiWrapper(_options) {
ctx.body = { ctx.body = {
...ctx.body, ...ctx.body,
status: ctx.status,
ok, ok,
}; };
}; };

View File

@ -1,23 +1,23 @@
import httpErrors from 'http-errors'; import apiError from '../../errors';
import validator from 'validator'; import validator from 'validator';
export default function validation() { export default function validation() {
return function validationMiddleware(ctx, next) { return function validationMiddleware(ctx, next) {
ctx.assertPresent = function assertPresent(value, message) { ctx.assertPresent = function assertPresent(value, message) {
if (value === undefined || value === null || value === '') { if (value === undefined || value === null || value === '') {
throw httpErrors.BadRequest(message); throw apiError(400, 'validation_error', message);
} }
}; };
ctx.assertEmail = function assertEmail(value, message) { ctx.assertEmail = function assertEmail(value, message) {
if (!validator.isEmail(value)) { if (!validator.isEmail(value)) {
throw httpErrors.BadRequest(message); throw apiError(400, 'validation_error', message);
} }
}; };
ctx.assertUuid = function assertUuid(value, message) { ctx.assertUuid = function assertUuid(value, message) {
if (!validator.isUUID(value)) { if (!validator.isUUID(value)) {
throw httpErrors.BadRequest(message); throw apiError(400, 'validation_error', message);
} }
}; };

View File

@ -25,7 +25,7 @@ router.post('user.s3Upload', auth(), async (ctx) => {
const policy = makePolicy(); const policy = makePolicy();
ctx.body = { data: { ctx.body = { data: {
max_upload_size: process.env.AWS_S3_UPLOAD_MAX_SIZE, maxUploadSize: process.env.AWS_S3_UPLOAD_MAX_SIZE,
upload_url: process.env.AWS_S3_UPLOAD_BUCKET_URL, upload_url: process.env.AWS_S3_UPLOAD_BUCKET_URL,
form: { form: {
AWSAccessKeyId: process.env.AWS_ACCESS_KEY_ID, AWSAccessKeyId: process.env.AWS_ACCESS_KEY_ID,
@ -37,7 +37,7 @@ router.post('user.s3Upload', auth(), async (ctx) => {
policy, policy,
}, },
asset: { asset: {
content_type: kind, contentType: kind,
url: `${process.env.AWS_S3_UPLOAD_BUCKET_URL}${s3Key}/${filename}`, url: `${process.env.AWS_S3_UPLOAD_BUCKET_URL}${s3Key}/${filename}`,
name: filename, name: filename,
size, size,

10
server/errors.js Normal file
View File

@ -0,0 +1,10 @@
import httpErrors from 'http-errors';
const apiError = (code, id, message) => {
return httpErrors(code, message, { id });
};
export default apiError;
export {
httpErrors,
};