From 613ae36b26b70b8c99c331e229c2d7306bd90eaf Mon Sep 17 00:00:00 2001 From: Andrew Fischer Date: Sat, 23 Nov 2019 12:41:31 -0500 Subject: [PATCH 1/4] initial pass at some preemptive error checks --- server/errors/index.js | 22 ++++++++++++++++++++++ server/errors/messages.json | 7 +++++++ server/index.js | 3 +++ server/userAuth.js | 13 +++++++++---- 4 files changed, 41 insertions(+), 4 deletions(-) create mode 100644 server/errors/index.js create mode 100644 server/errors/messages.json diff --git a/server/errors/index.js b/server/errors/index.js new file mode 100644 index 00000000..1d751e48 --- /dev/null +++ b/server/errors/index.js @@ -0,0 +1,22 @@ +const messages = require('./messages') + +const isDev = process.env.NODE_ENV === 'development' + +module.exports = () => { + const {envMessages} = messages + const { APPROVED_DOMAINS, DRIVE_TYPE, DRIVE_ID } = process.env + const errors = [] + + if (!APPROVED_DOMAINS) errors.push(envMessages.noApprovedDomains) + if (!DRIVE_TYPE) errors.push(envMessages.noDriveType) + if (!DRIVE_ID) errors.push(envMessages.noDriveID) + + if (errors.length) { + console.log('***********************************************') + console.log('Your library instance has configuration issues:') + errors.forEach((message) => console.error(` > ${message}`)) + console.log('Address these issues and restart the app.') + console.log('***********************************************') + process.exit(1) + } +} diff --git a/server/errors/messages.json b/server/errors/messages.json new file mode 100644 index 00000000..933804d3 --- /dev/null +++ b/server/errors/messages.json @@ -0,0 +1,7 @@ +{ + "envMessages": { + "noApprovedDomains": "No approved domains set. Add an APPROVED_DOMAINS environment variable.", + "noDriveType": "No DRIVE_TYPE env variable set. Please set it to 'team' or 'folder'.", + "noDriveID": "No DRIVE_ID env variable set. Set this environment variable and restart the app." + } +} diff --git a/server/index.js b/server/index.js index 7cdf4d32..613070b2 100644 --- a/server/index.js +++ b/server/index.js @@ -7,6 +7,7 @@ const csp = require('helmet-csp') const {middleware: cache} = require('./cache') const {getMeta} = require('./list') const {allMiddleware, requireWithFallback} = require('./utils') +const checkErrors = require('./errors') const userInfo = require('./routes/userInfo') const pages = require('./routes/pages') const categories = require('./routes/categories') @@ -14,6 +15,8 @@ const playlists = require('./routes/playlists') const readingHistory = require('./routes/readingHistory') const errorPages = require('./routes/errors') +checkErrors() + const userAuth = requireWithFallback('userAuth') const customCsp = requireWithFallback('csp') diff --git a/server/userAuth.js b/server/userAuth.js index 9794372e..feb92b7d 100644 --- a/server/userAuth.js +++ b/server/userAuth.js @@ -11,9 +11,15 @@ const {stringTemplate: template} = require('./utils') const router = require('express-promise-router')() const domains = new Set(process.env.APPROVED_DOMAINS.split(/,\s?/g)) +const isDev = process.env.NODE_ENV === 'development' + +// some value must be passed to passport, but in dev this value does not matter +const clientId = isDev ? ' ' : process.env.GOOGLE_CLIENT_ID +const clientSecret = isDev ? ' ' : process.env.GOOGLE_CLIENT_SECRET + passport.use(new GoogleStrategy.Strategy({ - clientID: process.env.GOOGLE_CLIENT_ID, - clientSecret: process.env.GOOGLE_CLIENT_SECRET, + clientID: clientId, + clientSecret: clientSecret, callbackURL: '/auth/redirect', userProfileURL: 'https://www.googleapis.com/oauth2/v3/userinfo', passReqToCallback: true @@ -51,7 +57,6 @@ router.get('/auth/redirect', passport.authenticate('google'), (req, res) => { }) router.use((req, res, next) => { - const isDev = process.env.NODE_ENV === 'development' const passportUser = (req.session.passport || {}).user || {} if (isDev || (req.isAuthenticated() && isAuthorized(passportUser))) { @@ -81,7 +86,7 @@ function isAuthorized(user) { } function setUserInfo(req) { - if (process.env.NODE_ENV === 'development') { + if (isDev) { // userInfo shim for development req.userInfo = { email: process.env.TEST_EMAIL || template('footer.defaultEmail'), userId: '10', From c08460a17777b9c0775a907fb73545511ffbf3b0 Mon Sep 17 00:00:00 2001 From: Andrew Fischer Date: Sat, 23 Nov 2019 12:52:19 -0500 Subject: [PATCH 2/4] Less noise in logs, pass error to 500 page --- layouts/errors/500.ejs | 3 +++ server/errors/index.js | 2 +- server/routes/errors.js | 1 + server/utils.js | 7 +++++-- 4 files changed, 10 insertions(+), 3 deletions(-) diff --git a/layouts/errors/500.ejs b/layouts/errors/500.ejs index 55950401..993836bb 100644 --- a/layouts/errors/500.ejs +++ b/layouts/errors/500.ejs @@ -8,6 +8,9 @@

<%- template('error.500.heading') %>

<%- template('error.500.message') %>

+ <% if (locals.isDev && locals.err && locals.err.message) { %> +

Error: <%- locals.err.message %>

+ <% } %>
<%- include('../partials/search', {style: 'homepage'}) %> diff --git a/server/errors/index.js b/server/errors/index.js index 1d751e48..373d6075 100644 --- a/server/errors/index.js +++ b/server/errors/index.js @@ -1,6 +1,6 @@ const messages = require('./messages') -const isDev = process.env.NODE_ENV === 'development' +// For now, this should just throw for things that would stop the app from booting. module.exports = () => { const {envMessages} = messages diff --git a/server/routes/errors.js b/server/routes/errors.js index de2cca83..9a1bce5b 100644 --- a/server/routes/errors.js +++ b/server/routes/errors.js @@ -65,6 +65,7 @@ module.exports = async (err, req, res, next) => { const inlined = await loadInlineAssets() res.status(code).render(`errors/${code}`, { inlineCSS: inlined.css, + isDev: process.env.NODE_ENV === 'development', err, template: inlined.stringTemplate }) diff --git a/server/utils.js b/server/utils.js index 78e8a1bb..f7b09eab 100644 --- a/server/utils.js +++ b/server/utils.js @@ -44,8 +44,11 @@ exports.requireWithFallback = (attemptPath) => { return require(customPath) } catch (err) { // if the file exists but we failed to pull it in, log that error at a warning level - const level = fs.existsSync(customPath) ? 'warn' : 'debug' - log[level](`Failed pulling in custom file ${attemptPath} @ ${customPath}. Error was:`, err) + // with no stacktrace. + const fileExists = fs.existsSync(customPath) + const level = fileExists ? 'warn' : 'debug' + const toLog = fileExists ? err : 'No override file given.' + log[level](`Failed pulling in custom file ${attemptPath} @ ${customPath}. Error was:`, toLog) return require(serverPath) } } From aa46ebada2a60b583e76620410f23f6c3a3bd227 Mon Sep 17 00:00:00 2001 From: Andrew Fischer Date: Sat, 23 Nov 2019 14:42:09 -0500 Subject: [PATCH 3/4] Error for empty tree --- server/routes/pages.js | 1 + 1 file changed, 1 insertion(+) diff --git a/server/routes/pages.js b/server/routes/pages.js index 7b74dc25..595570bf 100644 --- a/server/routes/pages.js +++ b/server/routes/pages.js @@ -57,6 +57,7 @@ async function handlePage(req, res) { if (page === 'categories' || page === 'index') { const tree = await getTree() + if (!tree.children) throw new Error('No files found. Ensure your DRIVE_ID is correct and that your service account email is shared with your drive or folder.') const categories = buildDisplayCategories(tree) res.render(template, { ...categories, template: stringTemplate }) return From 7825c2f3c92c7b4eff49c690cc06f7cfc304dc75 Mon Sep 17 00:00:00 2001 From: Andrew Fischer Date: Sun, 26 Jul 2020 19:17:14 -0400 Subject: [PATCH 4/4] pull email from auth client for error message --- server/errors/index.js | 8 ++++---- server/errors/messages.json | 9 +++++---- server/routes/pages.js | 11 +++++++++-- 3 files changed, 18 insertions(+), 10 deletions(-) diff --git a/server/errors/index.js b/server/errors/index.js index 373d6075..da338158 100644 --- a/server/errors/index.js +++ b/server/errors/index.js @@ -3,13 +3,13 @@ const messages = require('./messages') // For now, this should just throw for things that would stop the app from booting. module.exports = () => { - const {envMessages} = messages + const { errorMessages } = messages const { APPROVED_DOMAINS, DRIVE_TYPE, DRIVE_ID } = process.env const errors = [] - if (!APPROVED_DOMAINS) errors.push(envMessages.noApprovedDomains) - if (!DRIVE_TYPE) errors.push(envMessages.noDriveType) - if (!DRIVE_ID) errors.push(envMessages.noDriveID) + if (!APPROVED_DOMAINS) errors.push(errorMessages.noApprovedDomains) + if (!DRIVE_TYPE) errors.push(errorMessages.noDriveType) + if (!DRIVE_ID) errors.push(errorMessages.noDriveID) if (errors.length) { console.log('***********************************************') diff --git a/server/errors/messages.json b/server/errors/messages.json index 933804d3..b639249c 100644 --- a/server/errors/messages.json +++ b/server/errors/messages.json @@ -1,7 +1,8 @@ { - "envMessages": { - "noApprovedDomains": "No approved domains set. Add an APPROVED_DOMAINS environment variable.", - "noDriveType": "No DRIVE_TYPE env variable set. Please set it to 'team' or 'folder'.", - "noDriveID": "No DRIVE_ID env variable set. Set this environment variable and restart the app." + "errorMessages": { + "noApprovedDomains": "You must set the APPROVED_DOMAINS environment variable to a list of domains or a regular expression.", + "noDriveType": "No DRIVE_TYPE env variable set. Please set it to 'team' or 'folder.'", + "noDriveID": "No DRIVE_ID env variable set. Set this environment variable to the ID of your drive or folder and restart the app.", + "noFilesFound": "No files found. Ensure your DRIVE_ID and DRIVE_TYPE environment variables are set correctly and that your service account's email is shared with your drive or folder.\n\nIf you just added the account, wait a minute and try again." } } diff --git a/server/routes/pages.js b/server/routes/pages.js index 595570bf..acd2ae54 100644 --- a/server/routes/pages.js +++ b/server/routes/pages.js @@ -2,6 +2,8 @@ const search = require('../search') const move = require('../move') +const { getAuth } = require('../auth') +const { errorMessages } = require('../errors/messages.json') const router = require('express-promise-router')() @@ -14,7 +16,7 @@ router.get('/:page', handlePage) router.get('/filename-listing.json', async (req, res) => { res.header('Cache-Control', 'public, must-revalidate') // override no-cache const filenames = await getFilenames() - res.json({filenames: filenames}) + res.json({ filenames: filenames }) }) module.exports = router @@ -57,7 +59,12 @@ async function handlePage(req, res) { if (page === 'categories' || page === 'index') { const tree = await getTree() - if (!tree.children) throw new Error('No files found. Ensure your DRIVE_ID is correct and that your service account email is shared with your drive or folder.') + if (!tree.children) { + // pull the auth client email to make debugging easier: + const authClient = await getAuth() + const errMsg = errorMessages.noFilesFound.replace('email', `email (${authClient.email})`) + throw new Error(errMsg) + } const categories = buildDisplayCategories(tree) res.render(template, { ...categories, template: stringTemplate }) return