From 8104b0894507a5840038ad32f9411c0039644a01 Mon Sep 17 00:00:00 2001 From: wickathou Date: Wed, 5 Jun 2024 17:27:45 +0300 Subject: [PATCH 1/2] Improved error messaging logic for the backend with standarized responses and logging for errors Moved sumitVote logic from routes to controller Improved the error handling for the submitVote logic Initiated error handling improvements for the frontend when receiving errors from the backend Made userId into a required property when submitting a new winner-vote Removed commented and unnused code Improved session verification useEffect logic in App.js --- backend/common/errors/errorHandler.js | 64 +++++++++---- backend/common/errors/errors.js | 33 ++++--- backend/modules/voting-token/controller.js | 3 +- backend/modules/winner-votes/controller.js | 46 ++++++++- backend/modules/winner-votes/model.js | 3 +- backend/modules/winner-votes/routes.js | 94 +++++-------------- .../organiser/projects/winners/index.js | 13 ++- .../participant/finalist-voting/index.js | 38 ++++---- 8 files changed, 165 insertions(+), 129 deletions(-) diff --git a/backend/common/errors/errorHandler.js b/backend/common/errors/errorHandler.js index 98ce0e689..4b208acb5 100644 --- a/backend/common/errors/errorHandler.js +++ b/backend/common/errors/errorHandler.js @@ -1,6 +1,22 @@ const logger = require('../../misc/logger') +const logError = (error, request) => { + logger.error({ + message: error.message, + error: { + stack: error.stack, + complete: error, + }, + request: { + requestBody: request.body, + requestParams: request.params, + requestQuery: request.query, + }, + }) +} + const errorHandler = (error, request, response, next) => { + logError(error, request) switch (error.name) { case 'UnauthorizedError': { return response.status(401).json({ @@ -8,51 +24,59 @@ const errorHandler = (error, request, response, next) => { status: 401, }) } + case 'NotFoundError': { + return response.status(404).json({ + message: error.message || 'Not found', + status: 404, + }) + } case 'InsufficientPrivilegesError': { return response.status(401).json({ - message: 'Insufficient privileges', + message: error.message || 'Insufficient privileges', status: 401, }) } - case 'ForbiddenError': { + case 'EmailVerificationError': { return response.status(403).json({ - message: error.message || 'Forbidden', + message: error.message || 'Email verification required', status: 403, }) } - case 'NotFoundError': { - return response.status(404).json({ - message: error.message || 'Not found', - status: 404, - }) - } case 'ValidationError': { return response.status(400).json({ - message: error.message, + message: error.message || 'Validation error', errors: error.errors, status: 400, }) } + case 'ForbiddenError': { + return response.status(403).json({ + message: error.message || 'Forbidden', + status: 403, + }) + } + case 'AlreadyExistsError': { + return response.status(400).json({ + message: error.message || 'Already exists', + status: 400, + }) + } case 'MongoError': { if (error.code === 11000) { return response.status(400).json({ + message: 'Report to tech support', type: 'unique-violation', status: 400, }) } - break + return response.status(400).json({ + message: error.message || 'Validation error', + status: 400, + }) } default: - logger.error({ - message: 'Unhandled error', - error: { - message: error.message, - stack: error.stack, - }, - }) - return response.status(500).json({ - message: 'Unexpected error', + message: error.message || 'Unexpected error', status: 500, }) } diff --git a/backend/common/errors/errors.js b/backend/common/errors/errors.js index 8ffe6b640..7dec726c3 100644 --- a/backend/common/errors/errors.js +++ b/backend/common/errors/errors.js @@ -18,26 +18,26 @@ class CustomError extends Error { } class NotFoundError extends CustomError { - constructor(message) { + constructor(message, details) { const name = 'NotFoundError' const code = 1 - super(message, name, code) + super(message, name, code, details) } } class InsufficientPrivilegesError extends CustomError { - constructor(message) { + constructor(message, details) { const name = 'InsufficientPrivilegesError' const code = 2 - super(message, name, code) + super(message, name, code, details) } } class EmailVerificationError extends CustomError { - constructor(message) { + constructor(message, details) { const name = 'EmailVerificationError' const code = 3 - super(message, name, code) + super(message, name, code, details) } } @@ -50,26 +50,34 @@ class ValidationError extends CustomError { } class ForbiddenError extends CustomError { - constructor(message) { + constructor(message, details) { const name = 'ForbiddenError' const code = 5 - super(message, name, code) + super(message, name, code, details) } } class UnauthorizedError extends CustomError { - constructor(message) { + constructor(message, details) { const name = 'UnauthorizedError' const code = 6 - super(message, name, code) + super(message, name, code, details) } } class AlreadyExistsError extends CustomError { - constructor(message) { + constructor(message, details) { const name = 'AlreadyExistsError' const code = 7 - super(message, name, code) + super(message, name, code, details) + } +} + +class MongoError extends CustomError { + constructor(message, details) { + const name = 'MongoError' + const code = 8 + super(message, name, code, details) } } @@ -81,4 +89,5 @@ module.exports = { ForbiddenError, UnauthorizedError, AlreadyExistsError, + MongoError, } diff --git a/backend/modules/voting-token/controller.js b/backend/modules/voting-token/controller.js index 428c3deb6..962541b6f 100644 --- a/backend/modules/voting-token/controller.js +++ b/backend/modules/voting-token/controller.js @@ -35,8 +35,7 @@ controller.getVotesByProject = async event => { }) return result }, []) - const sorted = _.orderBy(results, ['votes'], 'desc') - return sorted + return results } controller.getToken = id => { diff --git a/backend/modules/winner-votes/controller.js b/backend/modules/winner-votes/controller.js index 3ab3712f3..15c23fc16 100644 --- a/backend/modules/winner-votes/controller.js +++ b/backend/modules/winner-votes/controller.js @@ -2,6 +2,7 @@ const _ = require('lodash') const WinnerVote = require('./model') const projectController = require('../project/controller') const tokenVotingController = require('../voting-token/controller') +const { ValidationError, MongoError } = require('../../common/errors/errors') const controller = {} @@ -17,8 +18,7 @@ controller.getFinalistProjectsWithAllVotes = async event => { return projectObject }) const userVotes = await controller.getVotesForEvent(event) - const tokenVotes = await tokenVotingController.getVotesByProject(event._id) - if (userVotes) { + if (userVotes && userVotes.length > 0) { userVotes.map(v => { const projectWithVotes = _.find( finalistProjectsWithVotes, @@ -29,7 +29,8 @@ controller.getFinalistProjectsWithAllVotes = async event => { } }) } - if (tokenVotes) { + const tokenVotes = await tokenVotingController.getVotesByProject(event._id) + if (tokenVotes && tokenVotes.length > 0) { tokenVotes.map(v => { const projectWithVotes = _.find( finalistProjectsWithVotes, @@ -65,4 +66,43 @@ controller.getVotesForEvent = async event => { return results } +controller.submitVote = async (eventId, userId, projectId) => { + if (!eventId || !projectId || !userId) { + throw new ValidationError( + 'Some data is missing, try again. If the problem persists, contact support.', + ) + } + + let vote = await WinnerVote.findOne({ + event: eventId, + user: userId, + }) + + if (vote) { + vote.project = projectId + } else { + vote = new WinnerVote({ + event: eventId, + user: userId, + project: projectId, + }) + } + + if (!vote) { + throw new Error( + 'Unknown error, please try again. If the problem persists, contact support.', + ) + } + + try { + const result = await vote.save() + return result + } catch (err) { + throw new MongoError( + 'Vote could not be saved, reload the page and try again.', + err, + ) + } +} + module.exports = controller diff --git a/backend/modules/winner-votes/model.js b/backend/modules/winner-votes/model.js index 17f23aae6..a24561925 100644 --- a/backend/modules/winner-votes/model.js +++ b/backend/modules/winner-votes/model.js @@ -8,6 +8,7 @@ const WinnerVoteSchema = new mongoose.Schema({ }, user: { type: String, + required: true, }, project: { type: mongoose.Schema.Types.ObjectId, @@ -24,7 +25,7 @@ WinnerVoteSchema.index( }, { unique: true, - } + }, ) const WinnerVote = mongoose.model('WinnerVote', WinnerVoteSchema) diff --git a/backend/modules/winner-votes/routes.js b/backend/modules/winner-votes/routes.js index f7483e10a..89075a358 100644 --- a/backend/modules/winner-votes/routes.js +++ b/backend/modules/winner-votes/routes.js @@ -13,11 +13,16 @@ const { } = require('../../common/middleware/events') const votingController = require('./controller') +const errorHandler = require('../../common/errors/errorHandler') const getProjectsWithVotesForEvent = asyncHandler(async (req, res) => { - const finalistProjectsWithAllVotes = - await votingController.getFinalistProjectsWithAllVotes(req.event) - return res.status(200).json(finalistProjectsWithAllVotes) + try { + const finalistProjectsWithAllVotes = + await votingController.getFinalistProjectsWithAllVotes(req.event) + return res.status(200).json(finalistProjectsWithAllVotes) + } catch (e) { + return errorHandler(e, req, res) + } }) const getVote = asyncHandler(async (req, res) => { @@ -30,79 +35,28 @@ const getVote = asyncHandler(async (req, res) => { }) const submitVote = asyncHandler(async (req, res) => { - const vote = await WinnerVote.findOne({ - event: req.event._id, - user: req.user.sub, - }) - if (vote) { - vote.project = req.body.projectId - const result = await vote.save() + const eventId = req.event._id + const userId = req.user.sub + const projectId = req.body.projectId + try { + const result = await votingController.submitVote( + eventId, + userId, + projectId, + ) return res.status(200).json(result) + } catch (e) { + return errorHandler(e, req, res) } - const newVote = new WinnerVote({ - event: req.event._id, - user: req.user.sub, - project: req.body.projectId, - }) - const result = await newVote.save() - return res.status(200).json(result) }) router .route('/:slug') - .get( - hasToken, - hasRegisteredToEvent, - getVote, - // asyncHandler(async (req, res) => { - // const vote = await WinnerVote.findOne({ - // event: req.event._id, - // user: req.user.sub, - // }) - - // return res.status(200).json(vote) - // }), - ) - .post( - hasToken, - hasRegisteredToEvent, - submitVote, - // asyncHandler(async (req, res) => { - // const vote = await WinnerVote.findOne({ - // event: req.event._id, - // user: req.user.sub, - // }) - // if (vote) { - // vote.project = req.body.projectId - // const result = await vote.save() - // return res.status(200).json(result) - // } - // const newVote = new WinnerVote({ - // event: req.event._id, - // user: req.user.sub, - // project: req.body.projectId, - // }) - // const result = await newVote.save() - // return res.status(200).json(result) - // }), - ) - -router.route('/:slug/results').get( - hasToken, - isEventOrganiser, - getProjectsWithVotesForEvent, - // asyncHandler(async (req, res) => { - // const votes = await WinnerVote.find({ - // event: req.event._id, - // }) - - // const grouped = _.groupBy(votes, 'project') - // console.log('Grouped votes:', grouped) - // getProjectsWithVotesForEvent(req, res) + .get(hasToken, hasRegisteredToEvent, getVote) + .post(hasToken, hasRegisteredToEvent, submitVote) - // return res.status(200).json(grouped) - // } -) -// ) +router + .route('/:slug/results') + .get(hasToken, isEventOrganiser, getProjectsWithVotesForEvent) module.exports = router diff --git a/frontend/src/pages/_dashboard/renderDashboard/organiser/projects/winners/index.js b/frontend/src/pages/_dashboard/renderDashboard/organiser/projects/winners/index.js index c8eb2d7d2..1debed269 100644 --- a/frontend/src/pages/_dashboard/renderDashboard/organiser/projects/winners/index.js +++ b/frontend/src/pages/_dashboard/renderDashboard/organiser/projects/winners/index.js @@ -1,7 +1,7 @@ import React, { useCallback, useEffect, useState } from 'react' import { Grid, Box, Dialog } from '@material-ui/core' -import { useSelector } from 'react-redux' +import { useDispatch, useSelector } from 'react-redux' import PageHeader from 'components/generic/PageHeader' import PageWrapper from 'components/layouts/PageWrapper' @@ -10,6 +10,7 @@ import ProjectDetail from 'components/projects/ProjectDetail' import * as OrganiserSelectors from 'redux/organiser/selectors' import * as AuthSelectors from 'redux/auth/selectors' +import * as SnackbarActions from 'redux/snackbar/actions' import WinnerVoteService from 'services/winnerVote' import _ from 'lodash' @@ -17,6 +18,7 @@ import _ from 'lodash' export default () => { const event = useSelector(OrganiserSelectors.event) const idToken = useSelector(AuthSelectors.getIdToken) + const dispatch = useDispatch() const [loading, setLoading] = useState(true) const [selected, setSelected] = useState(false) @@ -36,10 +38,17 @@ export default () => { } } catch (error) { console.error(error) + dispatch( + SnackbarActions.error( + `Error retrieving the voting data: ${ + error.response.data.message || error.message + }`, + ), + ) } finally { setLoading(false) } - }, [event, idToken]) + }, [event]) useEffect(() => { if (event?.slug) { diff --git a/frontend/src/pages/_dashboard/renderDashboard/participant/finalist-voting/index.js b/frontend/src/pages/_dashboard/renderDashboard/participant/finalist-voting/index.js index 107bfde2d..e8511c60b 100644 --- a/frontend/src/pages/_dashboard/renderDashboard/participant/finalist-voting/index.js +++ b/frontend/src/pages/_dashboard/renderDashboard/participant/finalist-voting/index.js @@ -29,12 +29,11 @@ export default () => { const [vote, setVote] = useState(null) const [hasVoted, setVoted] = useState(false) - const getCurrentVote = useCallback(() => { + const getCurrentVote = useCallback(async () => { return WinnerVoteService.getVote(idToken, event.slug) }, [idToken, event]) const getFinalists = useCallback(async () => { - setLoading(true) EventsService.getFinalists(idToken, event.slug) .then(finalistProjects => { setProjects(finalistProjects) @@ -42,42 +41,40 @@ export default () => { .catch(err => { dispatch( SnackbarActions.error( - 'Something went wrong... Please try again', + 'Something went wrong while fetching finalists, refresh the page to try again.', ), ) - console.error(err) - }) - .finally(() => { - setLoading(false) }) }, [idToken, event]) useEffect(() => { - getFinalists() - update() + setLoading(true) + try { + getFinalists() + update() + } catch (err) { + } finally { + setLoading(false) + } }, []) const update = useCallback(async () => { try { - setLoading(true) const vote = await getCurrentVote() - if (vote) { - setVote(vote.project) + if (vote && vote?.project) { + setVote(vote?.project) setVoted(true) } } catch (err) { dispatch( SnackbarActions.error( - 'Something went wrong... Please try again', + 'Something went wrong while fetching your vote, refresh the page to try again.', ), ) - console.error(err) - } finally { - setLoading(false) } }, [event, idToken]) - const handleSubmit = useCallback(async () => { + const handleSubmit = async () => { try { setLoading(true) const result = await WinnerVoteService.submitVote( @@ -91,13 +88,16 @@ export default () => { } catch (err) { dispatch( SnackbarActions.error( - 'Something went wrong... Please try again', + `Score could not be saved. Error: ${ + err.response.data.message || err.message + }`, ), ) } finally { setLoading(false) } - }, [idToken, event, vote]) + } + return ( Date: Wed, 5 Jun 2024 17:28:22 +0300 Subject: [PATCH 2/2] Improved session verification useEffect logic in App.js --- frontend/src/App.js | 24 +++++++++--------------- 1 file changed, 9 insertions(+), 15 deletions(-) diff --git a/frontend/src/App.js b/frontend/src/App.js index 20a9c5560..daf478ab1 100755 --- a/frontend/src/App.js +++ b/frontend/src/App.js @@ -34,24 +34,18 @@ export default ({ history, location }) => { }, [location, history]) useEffect(() => { - if (isAuthenticated) { - if (isSessionExpired) { - setLoading(true) - console.log('renewing session now') + setLoading(false) + if (isAuthenticated && isSessionExpired) { + setLoading(true) + console.log('renewing session now') + try { dispatch(AuthActions.renewSession()) - .then(() => { - setLoading(false) - }) - .catch(err => { - console.log(err) - dispatch(SnackbarActions.error('Please, log in again')) - }) - .finally(() => setLoading(false)) - } else { + } catch (err) { + console.log(err) + dispatch(SnackbarActions.error('Please, log in again')) + } finally { setLoading(false) } - } else { - setLoading(false) } }, [dispatch, isAuthenticated, isSessionExpired])