From 2f094801ca1ff8569035ef201a852844d8706a1a Mon Sep 17 00:00:00 2001 From: Trevor Buckner Date: Tue, 16 Mar 2021 00:06:01 -0400 Subject: [PATCH 1/4] Move "get brew" logic to common function Also add centralized error handling middleware --- server.js | 179 ++++++++++++++++++---------------------- server/googleActions.js | 8 +- 2 files changed, 87 insertions(+), 100 deletions(-) diff --git a/server.js b/server.js index 2b460ccf2..f33b27f1d 100644 --- a/server.js +++ b/server.js @@ -9,6 +9,45 @@ const GoogleActions = require('./server/googleActions.js'); const serveCompressedStaticAssets = require('./server/static-assets.mv.js'); const sanitizeFilename = require('sanitize-filename'); +// //Custom Error +// class ErrorHandler extends Error { +// constructor(statusCode, message) { +// super(); +// this.statusCode = statusCode; +// this.message = message; +// } +// } + +//Format brew source for viewing in the browser as plain text +// const sanitizeSource = (text)=>{ +// const replaceStrings = { '&' : '&', +// '<' : '<', +// '>' : '>' }; +// for (const replaceStr in replaceStrings) { +// text = text.replaceAll(replaceStr, replaceStrings[replaceStr]); +// } +// return `
${text}
`; +// }; + +//Get the brew object from the HB database or Google Drive +const getBrewFromId = async (id, accessType)=>{ + if(accessType !== 'edit' && accessType !== 'share') + throw ('Invalid Access Type when getting brew'); + let brew; + if(id.length > 12) { + const googleId = id.slice(0, -12); + id = id.slice(-12); + brew = await GoogleActions.readFileMetadata(config.get('google_api_key'), googleId, id, accessType) + .catch((err)=>{throw err;}); + } else { + brew = await HomebrewModel.get(accessType == 'edit' ? { editId: id } : { shareId: id }) + .catch((err)=>{throw err;}); + brew.sanatize(true); + } + + return brew; +}; + app.use('/', serveCompressedStaticAssets(`${__dirname}/build`)); process.chdir(__dirname); @@ -65,6 +104,10 @@ app.get('/robots.txt', (req, res)=>{ return res.sendFile(`${__dirname}/robots.txt`); }); +// app.get('/error', (req, res)=>{ +// throw new ErrorHandler(404, 'User with the specified email does not exist'); +// }); + //Source page app.get('/source/:id', (req, res)=>{ @@ -170,119 +213,48 @@ app.get('/user/:username', async (req, res, next)=>{ }); //Edit Page -app.get('/edit/:id', (req, res, next)=>{ +app.get('/edit/:id', async (req, res, next)=>{ res.header('Cache-Control', 'no-cache, no-store'); //reload the latest saved brew when pressing back button, not the cached version before save. - if(req.params.id.length > 12) { - const googleId = req.params.id.slice(0, -12); - const editId = req.params.id.slice(-12); - GoogleActions.readFileMetadata(config.get('google_api_key'), googleId, editId, 'edit') - .then((brew)=>{ - req.brew = brew; //TODO Need to sanitize later - return next(); - }) - .catch((err)=>{ - console.log(err); - return res.status(400).send('Can\'t get brew from Google'); - }); - } else { - HomebrewModel.get({ editId: req.params.id }) - .then((brew)=>{ - req.brew = brew.sanatize(); - return next(); - }) - .catch((err)=>{ - console.log(err); - return res.status(400).send(`Can't get that`); - }); - } + const brew = await getBrewFromId(req.params.id, 'edit') + .catch((err)=>{next(err);}); + + req.brew = brew; + return next(); }); //New Page -app.get('/new/:id', (req, res, next)=>{ - if(req.params.id.length > 12) { - const googleId = req.params.id.slice(0, -12); - const shareId = req.params.id.slice(-12); - GoogleActions.readFileMetadata(config.get('google_api_key'), googleId, shareId, 'share') - .then((brew)=>{ - req.brew = brew; //TODO Need to sanitize later - return next(); - }) - .catch((err)=>{ - console.log(err); - return res.status(400).send('Can\'t get brew from Google'); - }); - } else { - HomebrewModel.get({ shareId: req.params.id }) - .then((brew)=>{ - req.brew = brew; - return next(); - }) - .catch((err)=>{ - console.log(err); - return res.status(400).send(`Can't get that`); - }); - } +app.get('/new/:id', async (req, res, next)=>{ + const brew = await getBrewFromId(req.params.id, 'share') + .catch((err)=>{next(err);}); + + req.brew = brew; + return next(); }); //Share Page -app.get('/share/:id', (req, res, next)=>{ +app.get('/share/:id', async (req, res, next)=>{ + const brew = await getBrewFromId(req.params.id, 'share') + .catch((err)=>{next(err);}); + if(req.params.id.length > 12) { const googleId = req.params.id.slice(0, -12); const shareId = req.params.id.slice(-12); - GoogleActions.readFileMetadata(config.get('google_api_key'), googleId, shareId, 'share') - .then((brew)=>{ - GoogleActions.increaseView(googleId, shareId, 'share', brew); - return brew; - }) - .then((brew)=>{ - req.brew = brew; //TODO Need to sanitize later - return next(); - }) - .catch((err)=>{ - console.log(err); - return res.status(400).send('Can\'t get brew from Google'); - }); + await GoogleActions.increaseView(googleId, shareId, 'share', brew) + .catch((err)=>{next(err);}); } else { - HomebrewModel.get({ shareId: req.params.id }) - .then((brew)=>{ - return brew.increaseView(); - }) - .then((brew)=>{ - req.brew = brew.sanatize(true); - return next(); - }) - .catch((err)=>{ - console.log(err); - return res.status(400).send(`Can't get that`); - }); + await brew.increaseView(); } + + req.brew = brew; + return next(); }); //Print Page -app.get('/print/:id', (req, res, next)=>{ - if(req.params.id.length > 12) { - const googleId = req.params.id.slice(0, -12); - const shareId = req.params.id.slice(-12); - GoogleActions.readFileMetadata(config.get('google_api_key'), googleId, shareId, 'share') - .then((brew)=>{ - req.brew = brew; //TODO Need to sanitize later - return next(); - }) - .catch((err)=>{ - console.log(err); - return res.status(400).send('Can\'t get brew from Google'); - }); - } else { - HomebrewModel.get({ shareId: req.params.id }) - .then((brew)=>{ - req.brew = brew.sanatize(true); - return next(); - }) - .catch((err)=>{ - console.log(err); - return res.status(400).send(`Can't get that`); - }); - } +app.get('/print/:id', async (req, res, next)=>{ + const brew = await getBrewFromId(req.params.id, 'share'); + + req.brew = brew; + return next(); }); //Render the page @@ -308,6 +280,17 @@ app.use((req, res)=>{ }); }); +//Error Handling Middleware +app.use((err, req, res, next)=>{ + const { statusCode, message } = err; + console.log('CUSTOM ERROR HANDLER'); + console.error(err); + res.status(statusCode).json({ + status : 'error', + statusCode, + message + }); +}); const PORT = process.env.PORT || config.get('web_port') || 8000; app.listen(PORT); diff --git a/server/googleActions.js b/server/googleActions.js index bd288ceea..301e6d31c 100644 --- a/server/googleActions.js +++ b/server/googleActions.js @@ -240,6 +240,7 @@ GoogleActions = { }, readFileMetadata : async (auth, id, accessId, accessType)=>{ + const drive = google.drive({ version: 'v3', auth: auth }); const obj = await drive.files.get({ @@ -248,7 +249,7 @@ GoogleActions = { }) .catch((err)=>{ console.log('Error loading from Google'); - console.error(err); + throw (err); return; }); @@ -344,7 +345,10 @@ GoogleActions = { increaseView : async (id, accessId, accessType, brew)=>{ //service account because this is modifying another user's file properties //so we need extended scope - const keys = JSON.parse(config.get('service_account')); + const keys = typeof(config.get('service_account')) == 'string' ? + JSON.parse(config.get('service_account')) : + config.get('service_account'); + const auth = google.auth.fromJSON(keys); auth.scopes = ['https://www.googleapis.com/auth/drive']; From b98586150f5caba04290b8d2300b7d551420e5ca Mon Sep 17 00:00:00 2001 From: Trevor Buckner Date: Wed, 17 Mar 2021 12:35:50 -0400 Subject: [PATCH 2/4] Condense Download and Source paths also --- server.js | 102 ++++++++++++------------------------------------------ 1 file changed, 22 insertions(+), 80 deletions(-) diff --git a/server.js b/server.js index f33b27f1d..7010bc585 100644 --- a/server.js +++ b/server.js @@ -18,17 +18,6 @@ const sanitizeFilename = require('sanitize-filename'); // } // } -//Format brew source for viewing in the browser as plain text -// const sanitizeSource = (text)=>{ -// const replaceStrings = { '&' : '&', -// '<' : '<', -// '>' : '>' }; -// for (const replaceStr in replaceStrings) { -// text = text.replaceAll(replaceStr, replaceStrings[replaceStr]); -// } -// return `
${text}
`; -// }; - //Get the brew object from the HB database or Google Drive const getBrewFromId = async (id, accessType)=>{ if(accessType !== 'edit' && accessType !== 'share') @@ -110,81 +99,34 @@ app.get('/robots.txt', (req, res)=>{ //Source page -app.get('/source/:id', (req, res)=>{ - if(req.params.id.length > 12) { - const googleId = req.params.id.slice(0, -12); - const shareId = req.params.id.slice(-12); - GoogleActions.readFileMetadata(config.get('google_api_key'), googleId, shareId, 'share') - .then((brew)=>{ - const replaceStrings = { '&': '&', '<': '<', '>': '>' }; - let text = brew.text; - for (const replaceStr in replaceStrings) { - text = text.replaceAll(replaceStr, replaceStrings[replaceStr]); - } - text = `
${text}
`; - res.status(200).send(text); - }) - .catch((err)=>{ - console.log(err); - return res.status(400).send('Can\'t get brew from Google'); - }); - } else { - HomebrewModel.get({ shareId: req.params.id }) - .then((brew)=>{ - const replaceStrings = { '&': '&', '<': '<', '>': '>' }; - let text = brew.text; - for (const replaceStr in replaceStrings) { - text = text.replaceAll(replaceStr, replaceStrings[replaceStr]); - } - text = `
${text}
`; - res.status(200).send(text); - }) - .catch((err)=>{ - console.log(err); - return res.status(404).send('Could not find Homebrew with that id'); - }); +app.get('/source/:id', async (req, res)=>{ + const brew = await getBrewFromId(req.params.id, 'share') + .catch((err)=>{next(err);}); + + const replaceStrings = { '&': '&', '<': '<', '>': '>' }; + let text = brew.text; + for (const replaceStr in replaceStrings) { + text = text.replaceAll(replaceStr, replaceStrings[replaceStr]); } + text = `
${text}
`; + res.status(200).send(text); }); //Download brew source page -app.get('/download/:id', (req, res)=>{ +app.get('/download/:id', async (req, res)=>{ + const brew = await getBrewFromId(req.params.id, 'share') + .catch((err)=>{next(err);}); + const prefix = 'HB - '; - if(req.params.id.length > 12) { - const googleId = req.params.id.slice(0, -12); - const shareId = req.params.id.slice(-12); - GoogleActions.readFileMetadata(config.get('google_api_key'), googleId, shareId, 'share') - .then((brew)=>{ - let fileName = sanitizeFilename(`${prefix}${brew.title}`).replaceAll(' ', ''); - if(!fileName || !fileName.length) { fileName = `${prefix}-Untitled-Brew`; }; - res.set({ - 'Cache-Control' : 'no-cache', - 'Content-Type' : 'text/plain', - 'Content-Disposition' : `attachment; filename="${fileName}.txt"` - }); - res.status(200).send(brew.text); - }) - .catch((err)=>{ - console.log(err); - return res.status(400).send('Can\'t get brew from Google'); - }); - } else { - HomebrewModel.get({ shareId: req.params.id }) - .then((brew)=>{ - let fileName = sanitizeFilename(`${prefix}${brew.title}`).replaceAll(' ', ''); - if(!fileName || !fileName.length) { fileName = `${prefix}-Untitled-Brew`; }; - res.set({ - 'Cache-Control' : 'no-cache', - 'Content-Type' : 'text/plain', - 'Content-Disposition' : `attachment; filename="${fileName}.txt"` - }); - res.status(200).send(brew.text); - }) - .catch((err)=>{ - console.log(err); - return res.status(404).send('Could not find Homebrew with that id'); - }); - } + let fileName = sanitizeFilename(`${prefix}${brew.title}`).replaceAll(' ', ''); + if(!fileName || !fileName.length) { fileName = `${prefix}-Untitled-Brew`; }; + res.set({ + 'Cache-Control' : 'no-cache', + 'Content-Type' : 'text/plain', + 'Content-Disposition' : `attachment; filename="${fileName}.txt"` + }); + res.status(200).send(brew.text); }); //User Page From 83c444ce114278bd3c5849f267fd26898196643c Mon Sep 17 00:00:00 2001 From: Trevor Buckner Date: Fri, 26 Mar 2021 22:50:03 -0400 Subject: [PATCH 3/4] Central Error Handling --- package-lock.json | 5 +++ package.json | 5 +-- server.js | 89 +++++++++++++++++++++-------------------------- 3 files changed, 47 insertions(+), 52 deletions(-) diff --git a/package-lock.json b/package-lock.json index 26cec70c3..b2498a736 100644 --- a/package-lock.json +++ b/package-lock.json @@ -3869,6 +3869,11 @@ } } }, + "express-async-handler": { + "version": "1.1.4", + "resolved": "https://registry.npmjs.org/express-async-handler/-/express-async-handler-1.1.4.tgz", + "integrity": "sha512-HdmbVF4V4w1q/iz++RV7bUxIeepTukWewiJGkoCKQMtvPF11MLTa7It9PRc/reysXXZSEyD4Pthchju+IUbMiQ==" + }, "express-static-gzip": { "version": "2.1.1", "resolved": "https://registry.npmjs.org/express-static-gzip/-/express-static-gzip-2.1.1.tgz", diff --git a/package.json b/package.json index bcaac21e0..b89d631a0 100644 --- a/package.json +++ b/package.json @@ -49,17 +49,18 @@ "cookie-parser": "^1.4.5", "create-react-class": "^15.7.0", "express": "^4.17.1", + "express-async-handler": "^1.1.4", "express-static-gzip": "2.1.1", "fs-extra": "9.1.0", "googleapis": "67.1.1", "jwt-simple": "^0.5.6", "less": "^3.13.1", "lodash": "^4.17.21", + "marked": "2.0.1", + "markedLegacy": "npm:marked@^0.3.19", "moment": "^2.29.1", "mongoose": "^5.12.0", "nanoid": "3.1.21", - "markedLegacy": "npm:marked@^0.3.19", - "marked": "2.0.1", "nconf": "^0.11.2", "prop-types": "15.7.2", "query-string": "6.14.1", diff --git a/server.js b/server.js index 7010bc585..5560ef498 100644 --- a/server.js +++ b/server.js @@ -8,15 +8,7 @@ const homebrewApi = require('./server/homebrew.api.js'); const GoogleActions = require('./server/googleActions.js'); const serveCompressedStaticAssets = require('./server/static-assets.mv.js'); const sanitizeFilename = require('sanitize-filename'); - -// //Custom Error -// class ErrorHandler extends Error { -// constructor(statusCode, message) { -// super(); -// this.statusCode = statusCode; -// this.message = message; -// } -// } +const asyncHandler = require('express-async-handler'); //Get the brew object from the HB database or Google Drive const getBrewFromId = async (id, accessType)=>{ @@ -33,7 +25,6 @@ const getBrewFromId = async (id, accessType)=>{ .catch((err)=>{throw err;}); brew.sanatize(true); } - return brew; }; @@ -93,15 +84,9 @@ app.get('/robots.txt', (req, res)=>{ return res.sendFile(`${__dirname}/robots.txt`); }); -// app.get('/error', (req, res)=>{ -// throw new ErrorHandler(404, 'User with the specified email does not exist'); -// }); - - //Source page -app.get('/source/:id', async (req, res)=>{ - const brew = await getBrewFromId(req.params.id, 'share') - .catch((err)=>{next(err);}); +app.get('/source/:id', asyncHandler(async (req, res)=>{ + const brew = await getBrewFromId(req.params.id, 'share'); const replaceStrings = { '&': '&', '<': '<', '>': '>' }; let text = brew.text; @@ -110,13 +95,11 @@ app.get('/source/:id', async (req, res)=>{ } text = `
${text}
`; res.status(200).send(text); -}); +})); //Download brew source page -app.get('/download/:id', async (req, res)=>{ - const brew = await getBrewFromId(req.params.id, 'share') - .catch((err)=>{next(err);}); - +app.get('/download/:id', asyncHandler(async (req, res)=>{ + const brew = await getBrewFromId(req.params.id, 'share'); const prefix = 'HB - '; let fileName = sanitizeFilename(`${prefix}${brew.title}`).replaceAll(' ', ''); @@ -127,7 +110,7 @@ app.get('/download/:id', async (req, res)=>{ 'Content-Disposition' : `attachment; filename="${fileName}.txt"` }); res.status(200).send(brew.text); -}); +})); //User Page app.get('/user/:username', async (req, res, next)=>{ @@ -155,28 +138,23 @@ app.get('/user/:username', async (req, res, next)=>{ }); //Edit Page -app.get('/edit/:id', async (req, res, next)=>{ +app.get('/edit/:id', asyncHandler(async (req, res, next)=>{ res.header('Cache-Control', 'no-cache, no-store'); //reload the latest saved brew when pressing back button, not the cached version before save. - const brew = await getBrewFromId(req.params.id, 'edit') - .catch((err)=>{next(err);}); - + const brew = await getBrewFromId(req.params.id, 'edit'); req.brew = brew; return next(); -}); +})); //New Page -app.get('/new/:id', async (req, res, next)=>{ - const brew = await getBrewFromId(req.params.id, 'share') - .catch((err)=>{next(err);}); - +app.get('/new/:id', asyncHandler(async (req, res, next)=>{ + const brew = await getBrewFromId(req.params.id, 'share'); req.brew = brew; return next(); -}); +})); //Share Page -app.get('/share/:id', async (req, res, next)=>{ - const brew = await getBrewFromId(req.params.id, 'share') - .catch((err)=>{next(err);}); +app.get('/share/:id', asyncHandler(async (req, res, next)=>{ + const brew = await getBrewFromId(req.params.id, 'share'); if(req.params.id.length > 12) { const googleId = req.params.id.slice(0, -12); @@ -189,18 +167,16 @@ app.get('/share/:id', async (req, res, next)=>{ req.brew = brew; return next(); -}); +})); //Print Page -app.get('/print/:id', async (req, res, next)=>{ +app.get('/print/:id', asyncHandler(async (req, res, next)=>{ const brew = await getBrewFromId(req.params.id, 'share'); - req.brew = brew; return next(); -}); +})); //Render the page -//const render = require('.build/render'); const templateFn = require('./client/template.js'); app.use((req, res)=>{ const props = { @@ -217,22 +193,35 @@ app.use((req, res)=>{ templateFn('homebrew', title = req.brew ? req.brew.title : '', props) .then((page)=>{ res.send(page); }) .catch((err)=>{ + console.log('TEMPLATE ERROR'); console.log(err); return res.sendStatus(500); }); }); -//Error Handling Middleware +//v=====----- Error-Handling Middleware -----=====v// +//Format Errors so all fields will be sent +const replaceErrors = (key, value)=>{ + if(value instanceof Error) { + const error = {}; + Object.getOwnPropertyNames(value).forEach(function (key) { + error[key] = value[key]; + }); + return error; + } + return value; +}; + +const getPureError = (error)=>{ + return JSON.parse(JSON.stringify(error, replaceErrors)); +}; + app.use((err, req, res, next)=>{ - const { statusCode, message } = err; - console.log('CUSTOM ERROR HANDLER'); + const status = err.status || 500; console.error(err); - res.status(statusCode).json({ - status : 'error', - statusCode, - message - }); + res.status(status).send(getPureError(err)); }); +//^=====--------------------------------------=====^// const PORT = process.env.PORT || config.get('web_port') || 8000; app.listen(PORT); From ab473b12daa892a83be7b9637854acee46b544db Mon Sep 17 00:00:00 2001 From: Trevor Buckner Date: Fri, 26 Mar 2021 22:55:46 -0400 Subject: [PATCH 4/4] Apply asyncHandler to getBrewFromId --- server.js | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/server.js b/server.js index 5560ef498..c5861238e 100644 --- a/server.js +++ b/server.js @@ -11,22 +11,20 @@ const sanitizeFilename = require('sanitize-filename'); const asyncHandler = require('express-async-handler'); //Get the brew object from the HB database or Google Drive -const getBrewFromId = async (id, accessType)=>{ +const getBrewFromId = asyncHandler(async (id, accessType)=>{ if(accessType !== 'edit' && accessType !== 'share') throw ('Invalid Access Type when getting brew'); let brew; if(id.length > 12) { const googleId = id.slice(0, -12); id = id.slice(-12); - brew = await GoogleActions.readFileMetadata(config.get('google_api_key'), googleId, id, accessType) - .catch((err)=>{throw err;}); + brew = await GoogleActions.readFileMetadata(config.get('google_api_key'), googleId, id, accessType); } else { - brew = await HomebrewModel.get(accessType == 'edit' ? { editId: id } : { shareId: id }) - .catch((err)=>{throw err;}); + brew = await HomebrewModel.get(accessType == 'edit' ? { editId: id } : { shareId: id }); brew.sanatize(true); } return brew; -}; +}); app.use('/', serveCompressedStaticAssets(`${__dirname}/build`));