From f9daf0f8bef1c7154cd8061e1fb0da989129dc5e Mon Sep 17 00:00:00 2001 From: bill bittner Date: Thu, 2 Nov 2017 11:50:05 -0700 Subject: [PATCH] updated error handling --- controllers/publishController.js | 2 +- helpers/errorHandlers.js | 59 ++++++++++++++++++++------------ routes/api-routes.js | 16 ++++----- routes/page-routes.js | 12 +++---- 4 files changed, 49 insertions(+), 40 deletions(-) diff --git a/controllers/publishController.js b/controllers/publishController.js index 84c68750..d8ee0bf6 100644 --- a/controllers/publishController.js +++ b/controllers/publishController.js @@ -67,7 +67,7 @@ module.exports = { resolve(publishResults); // resolve the promise with the result from lbryApi.publishClaim; }) .catch(error => { - logger.error('publishController.publish, error', error); + logger.error('publishController.publish error', error); publishHelpers.deleteTemporaryFile(publishParams.file_path); // delete the local file reject(error); }); diff --git a/helpers/errorHandlers.js b/helpers/errorHandlers.js index 3778eeeb..5bc73edf 100644 --- a/helpers/errorHandlers.js +++ b/helpers/errorHandlers.js @@ -2,38 +2,53 @@ const logger = require('winston'); const { postToStats } = require('../controllers/statsController.js'); module.exports = { - handleRequestError (action, originalUrl, ip, error, res) { - logger.error(`Request Error: ${originalUrl}`, module.exports.useObjectPropertiesIfNoKeys(error)); - postToStats(action, originalUrl, ip, null, null, error); - if (error.response) { - res.status(error.response.status).render('requestError', {message: error.response.data.error.message, status: error.response.status}); - } else if (error.code === 'ECONNREFUSED') { - res.status(503).render('requestError', {message: 'Connection refused. The daemon may not be running.', status: 503}); - } else if (error.message) { - res.status(500).render('requestError', {message: error.message, status: 500}); - } else { - res.status(400).render('requestError', {message: error, status: 400}); - } - }, - handlePublishError (error) { - logger.error('Publish Error:', module.exports.useObjectPropertiesIfNoKeys(error)); + returnErrorMessageAndStatus: function (error) { + let status; + let message; if (error.code === 'ECONNREFUSED') { - return 'Connection refused. The daemon may not be running.'; + status = 503; + message = 'Connection refused. The daemon may not be running.'; } else if (error.response) { + status = error.response.status || 500; if (error.response.data) { if (error.response.data.message) { - return error.response.data.message; + message = error.response.data.message; } else if (error.response.data.error) { - return error.response.data.error.message; + message = error.response.data.error.message; + } else { + message = error.response.data; } - return error.response.data; + } else { + message = error.response; } - return error.response; } else { - return error; + message = error; } + return [status, message]; }, - useObjectPropertiesIfNoKeys (err) { + handleRequestError: function (action, originalUrl, ip, error, res) { + logger.error(`Request Error on ${originalUrl}`, module.exports.useObjectPropertiesIfNoKeys(error)); + postToStats(action, originalUrl, ip, null, null, error); + const errorStatusAndMessage = this.returnErrorMessageAndStatus(error); + res + .status(errorStatusAndMessage[0]) + .render('requestError', { + status : errorStatusAndMessage[0], + message: errorStatusAndMessage[1], + }); + }, + handleApiError: function (action, originalUrl, ip, error, res) { + logger.error(`Api ${action} Error on ${originalUrl}`, module.exports.useObjectPropertiesIfNoKeys(error)); + postToStats(action, originalUrl, ip, null, null, error); + const errorStatusAndMessage = this.returnErrorMessageAndStatus(error); + res + .status(errorStatusAndMessage[0]) + .json({ + success: false, + message: errorStatusAndMessage[1], + }); + }, + useObjectPropertiesIfNoKeys: function (err) { if (Object.keys(err).length === 0) { let newErrorObject = {}; Object.getOwnPropertyNames(err).forEach((key) => { diff --git a/routes/api-routes.js b/routes/api-routes.js index a5ca5c17..f020fc26 100644 --- a/routes/api-routes.js +++ b/routes/api-routes.js @@ -21,7 +21,7 @@ module.exports = (app) => { res.status(200).json(claimsList); }) .catch(error => { - errorHandlers.handleRequestError('publish', originalUrl, ip, error, res); + errorHandlers.handleApiError('claim_list', originalUrl, ip, error, res); }); }); // route to check whether spee.ch has published to a claim @@ -67,15 +67,12 @@ module.exports = (app) => { res.status(200).json(resolvedUri); }) .catch(error => { - errorHandlers.handleRequestError('publish', originalUrl, ip, error, res); + errorHandlers.handleApiError('resolve', originalUrl, ip, error, res); }); }); // route to run a publish request on the daemon - app.post('/api/publish', multipartMiddleware, (req, res) => { - logger.debug('req:', req.body, req.files); + app.post('/api/publish', multipartMiddleware, ({ body, files, ip, originalUrl }, res) => { // validate that mandatory parts of the request are present - const body = req.body; - const files = req.files; try { validateApiPublishRequest(body, files); } catch (error) { @@ -138,8 +135,7 @@ module.exports = (app) => { }); }) .catch(error => { - logger.error('publish api error', error); - res.status(400).json({success: false, message: error.message}); + errorHandlers.handleApiError('publish', originalUrl, ip, error, res); }); }); @@ -157,7 +153,7 @@ module.exports = (app) => { }); }); // route to get a short channel id from long channel Id - app.get('/api/shortChannelId/:longId/:name', ({ params }, res) => { + app.get('/api/shortChannelId/:longId/:name', ({ ip, originalUrl, params }, res) => { // serve content db.Certificate.getShortChannelIdFromLongChannelId(params.longId, params.name) .then(shortId => { @@ -166,7 +162,7 @@ module.exports = (app) => { }) .catch(error => { logger.error('api error getting short channel id', error); - res.status(400).json(error.message); + errorHandlers.handleApiError('short channel id', originalUrl, ip, error, res); }); }); }; diff --git a/routes/page-routes.js b/routes/page-routes.js index 6b0368e9..ac56b889 100644 --- a/routes/page-routes.js +++ b/routes/page-routes.js @@ -24,7 +24,7 @@ module.exports = (app) => { app.get('/trending', (req, res) => { res.status(301).redirect('/popular'); }); - app.get('/popular', (req, res) => { + app.get('/popular', ({ ip, originalUrl }, res) => { const startDate = new Date(); startDate.setDate(startDate.getDate() - 1); const dateTime = startDate.toISOString().slice(0, 19).replace('T', ' '); @@ -36,20 +36,18 @@ module.exports = (app) => { }); }) .catch(error => { - errorHandlers.handleRequestError(null, null, null, error, res); + errorHandlers.handleRequestError('popular', originalUrl, ip, error, res); }); }); // route to display a list of the trending images - app.get('/new', (req, res) => { + app.get('/new', ({ ip, originalUrl }, res) => { getRecentClaims() .then(result => { // logger.debug(result); - res.status(200).render('new', { - newClaims: result, - }); + res.status(200).render('new', { newClaims: result }); }) .catch(error => { - errorHandlers.handleRequestError(null, null, null, error, res); + errorHandlers.handleRequestError('new', originalUrl, ip, error, res); }); }); // route to send embedable video player (for twitter)