From ae309c833c3209c238fd6122091431274b2042c0 Mon Sep 17 00:00:00 2001 From: Joe Turner Date: Mon, 26 Mar 2018 15:33:37 +0100 Subject: [PATCH 1/3] Add prettier config --- package.json | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/package.json b/package.json index f032a84..f39c5e5 100644 --- a/package.json +++ b/package.json @@ -7,6 +7,11 @@ "start": "node server.js", "test": "mocha --exit" }, + "prettier": { + "tabWidth": 2, + "singleQuote": true, + "bracketSpacing": false + }, "keywords": [], "author": "", "license": "ISC", From 336fb691108529015cf9348e0e113a7c70ee9268 Mon Sep 17 00:00:00 2001 From: Joe Turner Date: Mon, 26 Mar 2018 14:03:03 -0400 Subject: [PATCH 2/3] Make all errors send back json --- auth/router.js | 10 ++++++++-- server.js | 50 +++++++++++++++++++++++++++++++++++++------------- 2 files changed, 45 insertions(+), 15 deletions(-) diff --git a/auth/router.js b/auth/router.js index e7852a4..fc29445 100644 --- a/auth/router.js +++ b/auth/router.js @@ -15,7 +15,10 @@ const createAuthToken = function(user) { }); }; -const localAuth = passport.authenticate('local', {session: false}); +const localAuth = passport.authenticate('local', { + session: false, + failWithError: true +}); router.use(bodyParser.json()); // The user provides a username and password to login router.post('/login', localAuth, (req, res) => { @@ -23,7 +26,10 @@ router.post('/login', localAuth, (req, res) => { res.json({authToken}); }); -const jwtAuth = passport.authenticate('jwt', {session: false}); +const jwtAuth = passport.authenticate('jwt', { + session: false, + failWithError: true +}); // The user exchanges a valid JWT for a new one with a later expiration router.post('/refresh', jwtAuth, (req, res) => { diff --git a/server.js b/server.js index b0bb920..7054aee 100644 --- a/server.js +++ b/server.js @@ -12,12 +12,12 @@ const passport = require('passport'); // const { james: jimmy, robert: bobby } = actorSurnames; // console.log(jimmy); // Stewart - the variable name is jimmy, not james // console.log(bobby); // De Niro - the variable name is bobby, not robert -const { router: usersRouter } = require('./users'); -const { router: authRouter, localStrategy, jwtStrategy } = require('./auth'); +const {router: usersRouter} = require('./users'); +const {router: authRouter, localStrategy, jwtStrategy} = require('./auth'); mongoose.Promise = global.Promise; -const { PORT, DATABASE_URL } = require('./config'); +const {PORT, DATABASE_URL} = require('./config'); const app = express(); @@ -25,7 +25,7 @@ const app = express(); app.use(morgan('common')); // CORS -app.use(function (req, res, next) { +app.use(function(req, res, next) { res.header('Access-Control-Allow-Origin', '*'); res.header('Access-Control-Allow-Headers', 'Content-Type,Authorization'); res.header('Access-Control-Allow-Methods', 'GET,POST,PUT,PATCH,DELETE'); @@ -41,7 +41,10 @@ passport.use(jwtStrategy); app.use('/api/users/', usersRouter); app.use('/api/auth/', authRouter); -const jwtAuth = passport.authenticate('jwt', { session: false }); +const jwtAuth = passport.authenticate('jwt', { + session: false, + failWithError: true +}); // A protected endpoint which needs a valid JWT to access it app.get('/api/protected', jwtAuth, (req, res) => { @@ -50,8 +53,29 @@ app.get('/api/protected', jwtAuth, (req, res) => { }); }); -app.use('*', (req, res) => { - return res.status(404).json({ message: 'Not Found' }); +app.use('*', (req, res, next) => { + const err = new Error('Not Found'); + err.status = 404; + next(err); +}); + +app.use((err, req, res, next) => { + if (!err.status || err.status >= 500) { + return next(err); + } + res.status(err.status).json( + Object.assign({}, err, { + message: err.message + }) + ); +}); + +app.use((err, req, res, next) => { + console.error(err); + + return res.status(req.status || 500).json({ + message: 'Internal Server Error' + }); }); // Referenced by both runServer and closeServer. closeServer @@ -59,16 +83,16 @@ app.use('*', (req, res) => { let server; function runServer(databaseUrl, port = PORT) { - return new Promise((resolve, reject) => { mongoose.connect(databaseUrl, err => { if (err) { return reject(err); } - server = app.listen(port, () => { - console.log(`Your app is listening on port ${port}`); - resolve(); - }) + server = app + .listen(port, () => { + console.log(`Your app is listening on port ${port}`); + resolve(); + }) .on('error', err => { mongoose.disconnect(); reject(err); @@ -95,4 +119,4 @@ if (require.main === module) { runServer(DATABASE_URL).catch(err => console.error(err)); } -module.exports = { app, runServer, closeServer }; +module.exports = {app, runServer, closeServer}; From db0c1b9e017d9b26aa459295c47cb3d5c3f9eca8 Mon Sep 17 00:00:00 2001 From: Joe Turner Date: Mon, 26 Mar 2018 15:05:46 -0400 Subject: [PATCH 3/3] Switch validation errors to use next --- users/router.js | 83 +++++++++++++++++++++---------------------------- 1 file changed, 36 insertions(+), 47 deletions(-) diff --git a/users/router.js b/users/router.js index 597e97a..e81bee0 100644 --- a/users/router.js +++ b/users/router.js @@ -9,17 +9,16 @@ const router = express.Router(); const jsonParser = bodyParser.json(); // Post to register a new user -router.post('/', jsonParser, (req, res) => { +router.post('/', jsonParser, (req, res, next) => { const requiredFields = ['username', 'password']; const missingField = requiredFields.find(field => !(field in req.body)); if (missingField) { - return res.status(422).json({ - code: 422, - reason: 'ValidationError', - message: 'Missing field', - location: missingField - }); + const err = new Error('Missing field'); + err.status = 422; + err.reason = 'ValidationError'; + err.location = missingField; + return next(err); } const stringFields = ['username', 'password', 'firstName', 'lastName']; @@ -28,12 +27,11 @@ router.post('/', jsonParser, (req, res) => { ); if (nonStringField) { - return res.status(422).json({ - code: 422, - reason: 'ValidationError', - message: 'Incorrect field type: expected string', - location: nonStringField - }); + const err = new Error('Incorrect field type: expected string'); + err.status = 422; + err.reason = 'ValidationError'; + err.location = nonStringField; + return next(err); } // If the username and password aren't trimmed we give an error. Users might @@ -49,12 +47,11 @@ router.post('/', jsonParser, (req, res) => { ); if (nonTrimmedField) { - return res.status(422).json({ - code: 422, - reason: 'ValidationError', - message: 'Cannot start or end with whitespace', - location: nonTrimmedField - }); + const err = new Error('Cannot start or end with whitespace'); + err.status = 422; + err.reason = 'ValidationError'; + err.location = nonTrimmedField; + return next(err); } const sizedFields = { @@ -71,25 +68,24 @@ router.post('/', jsonParser, (req, res) => { const tooSmallField = Object.keys(sizedFields).find( field => 'min' in sizedFields[field] && - req.body[field].trim().length < sizedFields[field].min + req.body[field].trim().length < sizedFields[field].min ); const tooLargeField = Object.keys(sizedFields).find( field => 'max' in sizedFields[field] && - req.body[field].trim().length > sizedFields[field].max + req.body[field].trim().length > sizedFields[field].max ); if (tooSmallField || tooLargeField) { - return res.status(422).json({ - code: 422, - reason: 'ValidationError', - message: tooSmallField - ? `Must be at least ${sizedFields[tooSmallField] - .min} characters long` - : `Must be at most ${sizedFields[tooLargeField] - .max} characters long`, - location: tooSmallField || tooLargeField - }); + const err = new Error( + tooSmallField + ? `Must be at least ${sizedFields[tooSmallField].min} characters long` + : `Must be at most ${sizedFields[tooLargeField].max} characters long` + ); + err.status = 422; + err.reason = 'ValidationError'; + err.location = tooSmallField || tooLargeField; + return next(err); } let {username, password, firstName = '', lastName = ''} = req.body; @@ -103,12 +99,12 @@ router.post('/', jsonParser, (req, res) => { .then(count => { if (count > 0) { // There is an existing user with the same username - return Promise.reject({ - code: 422, - reason: 'ValidationError', - message: 'Username already taken', - location: 'username' - }); + const err = new Error('Username already taken'); + err.status = 422; + err.reason = 'ValidationError'; + err.message = 'Username already taken'; + err.location = 'username'; + return Promise.reject(err); } // If there is no existing user, hash the password return User.hashPassword(password); @@ -124,24 +120,17 @@ router.post('/', jsonParser, (req, res) => { .then(user => { return res.status(201).json(user.serialize()); }) - .catch(err => { - // Forward validation errors on to the client, otherwise give a 500 - // error because something unexpected has happened - if (err.reason === 'ValidationError') { - return res.status(err.code).json(err); - } - res.status(500).json({code: 500, message: 'Internal server error'}); - }); + .catch(err => next(err)); }); // Never expose all your users like below in a prod application // we're just doing this so we have a quick way to see // if we're creating users. keep in mind, you can also // verify this in the Mongo shell. -router.get('/', (req, res) => { +router.get('/', (req, res, next) => { return User.find() .then(users => res.json(users.map(user => user.serialize()))) - .catch(err => res.status(500).json({message: 'Internal server error'})); + .catch(err => next(err)); }); module.exports = {router};