Issue1666 - Include specific parameters to success/error messages on all routes

package.json

@@ -71,6 +71,7 @@
 		"dotenv": "~16.4.5",
 		"escape-html": "~1.0.3",
 		"express": "~4.19.2",
+		"express-async-errors": "~3.1.1",
 		"express-rate-limit": "~7.2.0",
 		"history": "~5.3.0",
 		"ini": "~4.1.3",

src/client/app/components/admin/users/CreateUserModalComponent.tsx

@@ -130,7 +130,7 @@ export default function CreateUserModal() {
 				showErrorNotification(
 					translate('users.failed.to.create.user') +
 					translate('users.successfully.edit.user.username') + userDetails.username + ') ' +
-					error.data.message);
+					error.data);
 				resetPasswordFields();
 			});
 	};

src/client/app/components/admin/users/EditUserModalComponent.tsx

@@ -209,7 +209,7 @@ export default function EditUserModalComponent(props: EditUserModalComponentProp
 				showErrorNotification(
 					translate('users.failed.to.edit.user') +
 					translate('users.successfully.edit.user.username') + userDetails.username + ') ' +
-					error.data.message
+					error.data
 				);
 			});
 		resetPasswordFields();
@@ -228,7 +228,7 @@ export default function EditUserModalComponent(props: EditUserModalComponentProp
 				showErrorNotification(
 					translate('users.failed.to.delete.user') +
 					translate('users.successfully.edit.user.username') + props.user.username + ') ' +
-					error.data.message
+					error.data
 				);
 			});
 	};

src/client/app/components/conversion/EditConversionModalComponent.tsx

@@ -420,7 +420,7 @@ export default function EditConversionModalComponent(props: EditConversionModalC
 					translate('conversion.delete.failure') +
 					' (' + translate('conversion.source') + ' "' + unitDataById[payload.sourceId]?.identifier + '"' +
 					', ' + translate('conversion.destination') + ' "' + unitDataById[payload.destinationId]?.identifier + '") ' +
-					error.data.message
+					error.data
 				);
 			});
 	};

src/client/app/components/groups/EditGroupModalComponent.tsx

@@ -205,9 +205,11 @@ export default function EditGroupModalComponent(props: EditGroupModalComponentPr
 		deleteGroup(groupState.id)
 			.unwrap()
 			.then(() => {
-				showSuccessNotification(translate('group.delete.success') + ' ' + groupState.name);
+				showSuccessNotification(
+					translate('group.delete.success') + ' (' + translate('name') + ' "' + groupState.name + '")'
+				);
 			}).catch(error => {
-				showErrorNotification(translate('group.delete.failure') + error.data.message);
+				showErrorNotification(translate('group.delete.failure') + ' ' + error.data);
 			});
 	};
 	/* End Confirm Delete Modal */

src/client/app/components/meters/EditMeterModalComponent.tsx

@@ -239,6 +239,7 @@ export default function EditMeterModalComponent(props: EditMeterModalComponentPr
 						(unitDataById[props.meter.unitId].unitRepresent != UnitRepresentType.quantity
 							&& unitDataById[localMeterEdits.unitId].unitRepresent == UnitRepresentType.quantity));
 
+
 				// Submit new meter if checks where ok.
 				editMeter({ meterData: submitState, shouldRefreshViews: shouldRefreshReadingViews })
 					.unwrap()

src/server/app.js

@@ -4,6 +4,11 @@
 
 const fs = require('fs');
 const express = require('express');
+// TODO Temporary patch for Express 4, which does not forward rejected async route handler
+// promises to the global error handler on its own (Express 5 does this natively). Remove this
+// import (and the express-async-errors dependency in package.json) once Express 5 is adopted —
+// see issue #1676 for that migration.
+require('express-async-errors');
 const rateLimit = require('express-rate-limit');
 const path = require('path');
 const favicon = require('serve-favicon');
@@ -182,7 +187,9 @@ app.use((err, req, res, next) => {
 		return res.status(HTTP_CODES.BAD_REQUEST).send('Bad Request');
 	}
 
-	log.error('Unhandled request error caught by global error handler; logging forwarded err object.', err);
+	// Include the method/URL so the admin-visible log entry can be traced back to a specific
+	// request instead of just showing a generic message with no route context.
+	log.error(`Unhandled request error caught by global error handler for ${req.method} ${req.originalUrl}; logging forwarded err object.`, err);
 	// If response headers are already sent, Express cannot safely change the response
 	// Forward to the default Express handler to finish error
 	if (res.headersSent) {

src/server/log.js

@@ -60,6 +60,13 @@ class Logger {
 		if (error !== null) {
 			if (error.stack) {
 				messageToLog += `Stacktrace: \n${error.stack}\n`;
+				// If this error wraps another one (e.g. new Error(msg, { cause: originalErr })),
+				// also print the original's stack so console/file output isn't limited to just
+				// where the wrapper was created — this only affects console/file output, not the
+				// message stored in the DB, which is unaffected either way.
+				if (error.cause && error.cause.stack) {
+					messageToLog += `Caused by: \n${error.cause.stack}\n`;
+				}
 			} else {
 				// It's possible someone passed in an error that isn't actually an Error object
 				// because javascript lets you throw anything. In that case, the error won't have

src/server/routes/baseline.js

@@ -6,20 +6,20 @@
 const { getConnection } = require('../db');
 const express = require('express');
 const Baseline = require('../models/Baseline');
-const log = require('../log');
 const validate = require('jsonschema').validate;
 const { adminAuthMiddleware } = require('./authenticator');
 const { STRING_GENERAL_MAX_LENGTH } = require('../util/validationConstants');
 const { HTTP_CODES } = require('../util/httpCodes');
 const { isValidIsoDateTime } = require('../util/timeValidation');
+const { success, failure } = require('./response');
 const router = express.Router();
 router.get('/', async (req, res) => {
 	const conn = getConnection();
 	try {
 		const rawBaselines = await Baseline.getAllBaselines(conn);
-		res.json(rawBaselines);
+		success(res, rawBaselines);
 	} catch (err) {
-		log(`Error while getting all baselines: ${err}`, 'error');
+		failure(res, HTTP_CODES.INTERNAL_SERVER_ERROR, new Error(`Error while getting all baselines: ${err.message}`, { cause: err }));
 	}
 });
 router.post('/new', adminAuthMiddleware('create baselines'), async (req, res) => {
@@ -59,14 +59,14 @@ router.post('/new', adminAuthMiddleware('create baselines'), async (req, res) =>
 	};
 
 	if (!validate(req.body, validParams).valid) {
-		res.sendStatus(HTTP_CODES.BAD_REQUEST);
+		failure(res, HTTP_CODES.BAD_REQUEST);
 		return;
 	}
 	// baseline.js does not use moment; validate date strings directly
 	// TODO This might not stay and is not used in OED now but need to see if it has a timezone for the check.
 	if (!isValidIsoDateTime(req.body.applyStart) || !isValidIsoDateTime(req.body.applyEnd) ||
 		!isValidIsoDateTime(req.body.calcStart) || !isValidIsoDateTime(req.body.calcEnd)) {
-		res.sendStatus(HTTP_CODES.BAD_REQUEST);
+		failure(res, HTTP_CODES.BAD_REQUEST);
 		return;
 	}
 
@@ -80,10 +80,9 @@ router.post('/new', adminAuthMiddleware('create baselines'), async (req, res) =>
 			req.body.calcEnd,
 			req.body.note);
 		await baseline.insert(conn);
-		res.sendStatus(HTTP_CODES.OK);
+		success(res);
 	} catch (err) {
-		res.sendStatus(HTTP_CODES.INTERNAL_SERVER_ERROR);
-		log(`Error while adding baseline: ${err}`, 'error');
+		failure(res, HTTP_CODES.INTERNAL_SERVER_ERROR, new Error(`Error while adding baseline: ${err.message}`, { cause: err }));
 	}
 });
 module.exports = router;

src/server/routes/ciks.js

@@ -3,9 +3,10 @@
  * file, You can obtain one at http://mozilla.org/MPL/2.0/. */
 
 const express = require('express');
-const { log } = require('../log');
 const { getConnection } = require('../db');
 const Cik = require('../models/Cik');
+const { success, failure } = require('./response');
+const { HTTP_CODES } = require('../util/httpCodes');
 
 const router = express.Router();
 
@@ -27,8 +28,8 @@ router.get('/', async (req, res) => {
   const conn = getConnection();
   try {
     const rows = await Cik.getAll(conn);
-    res.json(rows.map(formatCikForResponse));
+    success(res, rows.map(formatCikForResponse));
   } catch (err) {
-    log.error(`Error while performing GET ciks details query: ${err}`);
+    failure(res, HTTP_CODES.INTERNAL_SERVER_ERROR, new Error(`Error while performing GET ciks details query: ${err.message}`, { cause: err }));
   }
 });

src/server/routes/compareReadings.js

@@ -12,6 +12,7 @@ const Reading = require('../models/Reading');
 const { STRING_GENERAL_MAX_LENGTH, NUMERIC_ID_MAX_LENGTH } = require('../util/validationConstants');
 const { HTTP_CODES } = require('../util/httpCodes');
 const { isValidIsoDuration } = require('../util/timeValidation');
+const { success, failure } = require('./response');
 
 const DATE_TIME_WITH_TIME_REGEX = /^\d{4}-\d{2}-\d{2}(?:T| )\d{2}:\d{2}:\d{2}(?:\.\d+)?(?:Z|[+-]\d{2}:?\d{2})?$/;
 
@@ -118,7 +119,7 @@ function createRouter() {
 
 	router.get('/meters/:meter_ids', async (req, res) => {
 		if (!(validateMeterCompareReadingsParams(req.params) && validateQueryParams(req.query))) {
-			res.sendStatus(HTTP_CODES.BAD_REQUEST);
+			failure(res, HTTP_CODES.BAD_REQUEST);
 			return;
 		}
 		const meterIDs = req.params.meter_ids.split(',').map(id => parseInt(id));
@@ -128,20 +129,24 @@ function createRouter() {
 		const shiftRaw = req.query.shift;
 
 		if (!isValidCompareDateTime(currStartRaw) || !isValidCompareDateTime(currEndRaw) || !isValidIsoDuration(shiftRaw)) {
-			res.sendStatus(HTTP_CODES.BAD_REQUEST);
+			failure(res, HTTP_CODES.BAD_REQUEST);
 			return;
 		}
 
-		// The string sent should set the timezone to UTC so honor that as OED uses UTC.
-		const currStart = moment.parseZone(currStartRaw, moment.ISO_8601, true);
-		const currEnd = moment.parseZone(currEndRaw, moment.ISO_8601, true);
-		const shift = moment.duration(shiftRaw);
-		res.json(await meterCompareReadings(meterIDs, graphicUnitID, currStart, currEnd, shift));
+		try {
+			// The string sent should set the timezone to UTC so honor that as OED uses UTC.
+			const currStart = moment.parseZone(currStartRaw, moment.ISO_8601, true);
+			const currEnd = moment.parseZone(currEndRaw, moment.ISO_8601, true);
+			const shift = moment.duration(shiftRaw);
+			success(res, await meterCompareReadings(meterIDs, graphicUnitID, currStart, currEnd, shift));
+		} catch (err) {
+			failure(res, HTTP_CODES.INTERNAL_SERVER_ERROR, new Error(`Error while performing GET meter compare readings: ${err.message}`, { cause: err }));
+		}
 	});
 
 	router.get('/groups/:group_ids', async (req, res) => {
 		if (!(validateGroupCompareReadingsParams(req.params) && validateQueryParams(req.query))) {
-			res.sendStatus(HTTP_CODES.BAD_REQUEST);
+			failure(res, HTTP_CODES.BAD_REQUEST);
 			return;
 		}
 		const groupIDs = req.params.group_ids.split(',').map(id => parseInt(id));
@@ -151,15 +156,19 @@ function createRouter() {
 		const shiftRaw = req.query.shift;
 
 		if (!isValidCompareDateTime(currStartRaw) || !isValidCompareDateTime(currEndRaw) || !isValidIsoDuration(shiftRaw)) {
-			res.sendStatus(HTTP_CODES.BAD_REQUEST);
+			failure(res, HTTP_CODES.BAD_REQUEST);
 			return;
 		}
 
-		// The string sent should set the timezone to UTC so honor that as OED uses UTC.
-		const currStart = moment.parseZone(currStartRaw, moment.ISO_8601, true);
-		const currEnd = moment.parseZone(currEndRaw, moment.ISO_8601, true);
-		const shift = moment.duration(shiftRaw);
-		res.json(await groupCompareReadings(groupIDs, graphicUnitID, currStart, currEnd, shift));
+		try {
+			// The string sent should set the timezone to UTC so honor that as OED uses UTC.
+			const currStart = moment.parseZone(currStartRaw, moment.ISO_8601, true);
+			const currEnd = moment.parseZone(currEndRaw, moment.ISO_8601, true);
+			const shift = moment.duration(shiftRaw);
+			success(res, await groupCompareReadings(groupIDs, graphicUnitID, currStart, currEnd, shift));
+		} catch (err) {
+			failure(res, HTTP_CODES.INTERNAL_SERVER_ERROR, new Error(`Error while performing GET group compare readings: ${err.message}`, { cause: err }));
+		}
 	});
 
 	return router;

src/server/routes/conversionArray.js

@@ -9,6 +9,7 @@ const { refreshAllReadingViews } = require('../services/refreshAllReadingViews')
 const validate = require('jsonschema').validate;
 const { adminAuthMiddleware } = require('./authenticator');
 const { HTTP_CODES } = require('../util/httpCodes');
+const { success, failure } = require('./response');
 
 const router = express.Router();
 
@@ -30,20 +31,22 @@ router.post('/refresh', adminAuthMiddleware('conversion refresh system data'), a
 	};
 
 	if (!validate(req.body, validParams).valid) {
-		res.sendStatus(HTTP_CODES.BAD_REQUEST);
+		failure(res, HTTP_CODES.BAD_REQUEST);
 		return;
 	}
 
-	// TODO: Add try/catch error handling to properly handle failures during Cik refresh
-	// or reading view refresh operations and return appropriate error responses.
-	if (req.body.redoCik) {
-		const conn = getConnection();
-		await redoCik(conn);
-	}
-	if (req.body.refreshReadingViews) {
-		await refreshAllReadingViews();
+	try {
+		if (req.body.redoCik) {
+			const conn = getConnection();
+			await redoCik(conn);
+		}
+		if (req.body.refreshReadingViews) {
+			await refreshAllReadingViews();
+		}
+		success(res);
+	} catch (err) {
+		failure(res, HTTP_CODES.INTERNAL_SERVER_ERROR, new Error(`Error while performing conversion array refresh: ${err.message}`, { cause: err }));
 	}
-	res.sendStatus(HTTP_CODES.OK);
 });
 
 module.exports = router;