Improve error handling in server (#812)

* add functions to convert all errors to `ErrorResponse` type

* replace all `{ message: error.toString() }` with `toErrorResponse(error)`

* refactor: all controllers to to use getErrorMessage and add types to the Response

* 404 for nonexistent api routes

apps/client/src/features/app-settings/panel/sources-panel/SourcesPanel.tsx

@@ -2,7 +2,7 @@ import { ChangeEvent, useRef, useState } from 'react';
 import { Button, Input } from '@chakra-ui/react';
 import { IoCloudOutline } from '@react-icons/all-files/io5/IoCloudOutline';
 import { IoDownloadOutline } from '@react-icons/all-files/io5/IoDownloadOutline';
-import { ImportMap, unpackError } from 'ontime-utils';
+import { getErrorMessage, ImportMap } from 'ontime-utils';
 
 import {
   getWorksheetNames as getWorksheetNamesExcel,
@@ -59,7 +59,7 @@ export default function SourcesPanel() {
       setImportFlow('excel');
       setHasFile('done');
     } catch (error) {
-      const errorMessage = unpackError(error);
+      const errorMessage = getErrorMessage(error);
       setError(`Error uploading file: ${errorMessage}`);
       setWorksheets(null);
       setHasFile('none');

apps/server/src/api-data/custom-fields/customFields.controller.ts

@@ -1,7 +1,8 @@
-import { CustomField, CustomFields } from 'ontime-types';
+import { CustomField, CustomFields, ErrorResponse } from 'ontime-types';
 
 import type { Request, Response } from 'express';
 
+import { getErrorMessage } from 'ontime-utils';
 import {
   createCustomField,
   editCustomField,
@@ -14,36 +15,37 @@ export async function getCustomFields(_req: Request, res: Response<CustomFields>
   res.json(customFields);
 }
 
-// Expects { label: <label> type: 'string | ..' }
+export async function postCustomField(req: Request, res: Response<CustomFields | ErrorResponse>) {
-export async function postCustomField(req: Request, res: Response) {
   try {
     const newField = req.body as CustomField;
     const allFields = await createCustomField(newField);
     res.status(201).send(allFields);
   } catch (error) {
-    res.status(400).send({ message: error.toString() });
+    const message = getErrorMessage(error);
+    res.status(400).send({ message });
   }
 }
 
-// Expects { label: <oldLabel>, field: { label: <newLabel> type: 'string | ..' } }
+export async function putCustomField(req: Request, res: Response<CustomFields | ErrorResponse>) {
-export async function putCustomField(req: Request, res: Response) {
   try {
     const oldLabel = req.params.label;
     const { colour, type, label } = req.body;
     const newFields = await editCustomField(oldLabel, { label, colour, type });
     res.status(200).send(newFields);
   } catch (error) {
-    res.status(400).send({ message: error.toString() });
+    const message = getErrorMessage(error);
+    res.status(400).send({ message });
   }
 }
 
 // Expects { label: <label> }
-export async function deleteCustomField(req: Request, res: Response) {
+export async function deleteCustomField(req: Request, res: Response<CustomFields | ErrorResponse>) {
   try {
     const fieldToDelete = req.params.label;
     await removeCustomField(fieldToDelete);
     res.sendStatus(204);
   } catch (error) {
-    res.status(400).send({ message: error.toString() });
+    const message = getErrorMessage(error);
+    res.status(400).send({ message });
   }
 }

apps/server/src/api-data/db/db.controller.ts

@@ -16,6 +16,7 @@ import * as projectService from '../../services/project-service/ProjectService.j
 import { ensureJsonExtension } from '../../utils/fileManagement.js';
 import { generateUniqueFileName } from '../../utils/generateUniqueFilename.js';
 import { appStateService } from '../../services/app-state-service/AppStateService.js';
+import { getErrorMessage } from 'ontime-utils';
 
 export async function patchPartialProjectFile(req: Request, res: Response<DatabaseModel | ErrorResponse>) {
   // all fields are optional in validation
@@ -31,7 +32,8 @@ export async function patchPartialProjectFile(req: Request, res: Response<Databa
     const newData = await projectService.applyDataModel(patchDb);
     res.status(200).send(newData);
   } catch (error) {
-    res.status(400).send({ message: String(error) });
+    const message = getErrorMessage(error);
+    res.status(400).send({ message });
   }
 }
 
@@ -69,17 +71,17 @@ export async function createProjectFile(req: Request, res: Response<{ filename:
       filename,
     });
   } catch (error) {
-    res.status(500).send({ message: String(error) });
+    const message = getErrorMessage(error);
+    res.status(500).send({ message });
   }
 }
 
 export async function projectDownload(_req: Request, res: Response) {
   const fileTitle = projectService.getProjectTitle();
-  res.download(resolveDbPath, `${fileTitle}.json`, (err) => {
+  res.download(resolveDbPath, `${fileTitle}.json`, (error) => {
-    if (err) {
+    if (error) {
-      res.status(500).send({
+      const message = getErrorMessage(error);
-        message: `Could not download the file: ${err}`,
+      res.status(500).send({ message });
-      });
     }
   });
 }
@@ -104,7 +106,8 @@ export async function postProjectFile(req: Request, res: Response<MessageRespons
       message: `Loaded project ${filename}`,
     });
   } catch (error) {
-    res.status(400).send({ message: `Failed parsing ${error}` });
+    const message = getErrorMessage(error);
+    res.status(400).send({ message });
   }
 }
 
@@ -116,7 +119,8 @@ export async function listProjects(_req: Request, res: Response<ProjectFileListR
     const data = await projectService.getProjectList();
     res.status(200).send(data);
   } catch (error) {
-    res.status(500).send({ message: String(error) });
+    const message = getErrorMessage(error);
+    res.status(500).send({ message });
   }
 }
 
@@ -136,7 +140,8 @@ export async function loadProject(req: Request, res: Response<MessageResponse |
       message: `Loaded project ${name}`,
     });
   } catch (error) {
-    res.status(500).send({ message: String(error) });
+    const message = getErrorMessage(error);
+    res.status(500).send({ message });
   }
 }
 
@@ -167,7 +172,8 @@ export async function duplicateProjectFile(req: Request, res: Response<MessageRe
       message: `Duplicated project ${filename} to ${newFilename}`,
     });
   } catch (error) {
-    res.status(500).send({ message: String(error) });
+    const message = getErrorMessage(error);
+    res.status(500).send({ message });
   }
 }
 
@@ -199,7 +205,8 @@ export async function renameProjectFile(req: Request, res: Response<MessageRespo
       message: `Renamed project ${filename} to ${newFilename}`,
     });
   } catch (error) {
-    res.status(500).send({ message: String(error) });
+    const message = getErrorMessage(error);
+    res.status(500).send({ message });
   }
 }
 
@@ -235,7 +242,8 @@ export async function deleteProjectFile(req: Request, res: Response<MessageRespo
       message: `Deleted project ${filename}`,
     });
   } catch (error) {
-    res.status(500).send({ message: String(error) });
+    const message = getErrorMessage(error);
+    res.status(500).send({ message });
   }
 }
 

apps/server/src/api-data/http/http.controller.ts

@@ -5,6 +5,7 @@ import { Request, Response } from 'express';
 import { DataProvider } from '../../classes/data-provider/DataProvider.js';
 import { failEmptyObjects } from '../../utils/routerUtils.js';
 import { httpIntegration } from '../../services/integration-service/HttpIntegration.js';
+import { getErrorMessage } from 'ontime-utils';
 
 export async function getHTTP(_req: Request, res: Response<HttpSettings>) {
   const http = DataProvider.getHttp();
@@ -24,6 +25,7 @@ export async function postHTTP(req: Request, res: Response<HttpSettings | ErrorR
     const result = await DataProvider.setHttp(httpSettings);
     res.send(result).status(200);
   } catch (error) {
-    res.status(400).send({ message: String(error) });
+    const message = getErrorMessage(error);
+    res.status(400).send({ message });
   }
 }

apps/server/src/api-data/index.ts

@@ -25,3 +25,8 @@ appRouter.use('/sheets', sheetsRouter);
 appRouter.use('/excel', excelRouter);
 appRouter.use('/url-presets', urlPresetsRouter);
 appRouter.use('/view-settings', viewSettingsRouter);
+
+//we don't want to redirect to react index when using api routes
+appRouter.all('/*', (_req, res) => {
+  res.status(404).send();
+});

apps/server/src/api-data/osc/osc.controller.ts

@@ -5,6 +5,7 @@ import { Request, Response } from 'express';
 import { DataProvider } from '../../classes/data-provider/DataProvider.js';
 import { failEmptyObjects } from '../../utils/routerUtils.js';
 import { oscIntegration } from '../../services/integration-service/OscIntegration.js';
+import { getErrorMessage } from 'ontime-utils';
 
 export async function getOSC(_req: Request, res: Response<OSCSettings>) {
   const osc = DataProvider.getOsc();
@@ -24,6 +25,7 @@ export async function postOSC(req: Request, res: Response<OSCSettings | ErrorRes
     const result = await DataProvider.setOsc(oscSettings);
     res.send(result).status(200);
   } catch (error) {
-    res.status(400).send({ message: String(error) });
+    const message = getErrorMessage(error);
+    res.status(400).send({ message });
   }
 }

apps/server/src/api-data/project/project.controller.ts

@@ -4,6 +4,7 @@ import type { Request, Response } from 'express';
 import { DataProvider } from '../../classes/data-provider/DataProvider.js';
 import { removeUndefined } from '../../utils/parserUtils.js';
 import { failEmptyObjects } from '../../utils/routerUtils.js';
+import { getErrorMessage } from 'ontime-utils';
 
 export async function getProjectData(_req: Request, res: Response<ProjectData>) {
   res.json(DataProvider.getProjectData());
@@ -27,6 +28,7 @@ export async function postProjectData(req: Request, res: Response<ProjectData |
     const newData = await DataProvider.setProjectData(newEvent);
     res.status(200).send(newData);
   } catch (error) {
-    res.status(400).send({ message: error.toString() });
+    const message = getErrorMessage(error);
+    res.status(400).send({ message });
   }
 }

apps/server/src/api-data/rundown/rundown.controller.ts

@@ -14,6 +14,7 @@ import {
   swapEvents,
 } from '../../services/rundown-service/RundownService.js';
 import { getNormalisedRundown, getRundown } from '../../services/rundown-service/rundownUtils.js';
+import { getErrorMessage } from 'ontime-utils';
 
 export async function rundownGetAll(_req: Request, res: Response<OntimeRundown>) {
   const rundown = getRundown();
@@ -34,7 +35,8 @@ export async function rundownPost(req: Request, res: Response<OntimeRundownEntry
     const newEvent = await addEvent(req.body);
     res.status(201).send(newEvent);
   } catch (error) {
-    res.status(400).send({ message: error.toString() });
+    const message = getErrorMessage(error);
+    res.status(400).send({ message });
   }
 }
 
@@ -47,7 +49,8 @@ export async function rundownPut(req: Request, res: Response<OntimeRundownEntry
     const event = await editEvent(req.body);
     res.status(200).send(event);
   } catch (error) {
-    res.status(400).send({ message: error.toString() });
+    const message = getErrorMessage(error);
+    res.status(400).send({ message });
   }
 }
 
@@ -61,7 +64,8 @@ export async function rundownBatchPut(req: Request, res: Response<MessageRespons
     await batchEditEvents(ids, data);
     res.status(200).send({ message: 'Batch edit successful' });
   } catch (error) {
-    res.status(400).send(error);
+    const message = getErrorMessage(error);
+    res.status(400).send({ message });
   }
 }
 
@@ -75,7 +79,8 @@ export async function rundownReorder(req: Request, res: Response<OntimeRundownEn
     const event = await reorderEvent(eventId, from, to);
     res.status(200).send(event.newEvent);
   } catch (error) {
-    res.status(400).send({ message: error.toString() });
+    const message = getErrorMessage(error);
+    res.status(400).send({ message });
   }
 }
 
@@ -89,7 +94,8 @@ export async function rundownSwap(req: Request, res: Response<MessageResponse |
     await swapEvents(from, to);
     res.status(200).send({ message: 'Swap successful' });
   } catch (error) {
-    res.status(400).send({ message: error.toString() });
+    const message = getErrorMessage(error);
+    res.status(400).send({ message });
   }
 }
 
@@ -98,7 +104,8 @@ export async function rundownApplyDelay(req: Request, res: Response<MessageRespo
     await applyDelay(req.params.eventId);
     res.status(200).send({ message: 'Delay applied' });
   } catch (error) {
-    res.status(400).send({ message: error.toString() });
+    const message = getErrorMessage(error);
+    res.status(400).send({ message });
   }
 }
 
@@ -107,7 +114,8 @@ export async function rundownDelete(_req: Request, res: Response<MessageResponse
     await deleteAllEvents();
     res.status(204).send({ message: 'All events deleted' });
   } catch (error) {
-    res.status(400).send({ message: error.toString() });
+    const message = getErrorMessage(error);
+    res.status(400).send({ message });
   }
 }
 
@@ -116,6 +124,7 @@ export async function deleteEventById(req: Request, res: Response<MessageRespons
     await deleteEvent(req.params.eventId);
     res.status(204).send({ message: 'Event deleted' });
   } catch (error) {
-    res.status(400).send({ message: error.toString() });
+    const message = getErrorMessage(error);
+    res.status(400).send({ message });
   }
 }

apps/server/src/api-data/settings/settings.controller.ts

@@ -6,6 +6,7 @@ import { DataProvider } from '../../classes/data-provider/DataProvider.js';
 import { failEmptyObjects } from '../../utils/routerUtils.js';
 import { extractPin } from '../../services/project-service/ProjectService.js';
 import { isDocker } from '../../setup/index.js';
+import { getErrorMessage } from 'ontime-utils';
 import { obfuscate } from 'ontime-utils';
 
 export async function getSettings(_req: Request, res: Response<Settings>) {
@@ -31,6 +32,7 @@ export async function postSettings(req: Request, res: Response<Settings | ErrorR
     const editorKey = extractPin(req.body?.editorKey, settings.editorKey);
     const operatorKey = extractPin(req.body?.operatorKey, settings.operatorKey);
     const serverPort = Number(req.body?.serverPort);
+    //TODO: should this not be part of the validator?
     if (isNaN(serverPort)) {
       return res.status(400).send({ message: `Invalid value found for server port: ${req.body?.serverPort}` });
     }
@@ -59,6 +61,7 @@ export async function postSettings(req: Request, res: Response<Settings | ErrorR
     await DataProvider.setSettings(newData);
     res.status(200).send(newData);
   } catch (error) {
-    res.status(400).send({ message: String(error) });
+    const message = getErrorMessage(error);
+    res.status(400).send({ message });
   }
 }

apps/server/src/api-data/sheets/sheets.controller.ts

@@ -6,6 +6,8 @@
 import { Request, Response } from 'express';
 import { readFileSync } from 'fs';
 
+import type { AuthenticationStatus, CustomFields, ErrorResponse, OntimeRundown } from 'ontime-types';
+
 import { deleteFile } from '../../utils/parserUtils.js';
 import {
   revoke,
@@ -16,8 +18,12 @@ import {
   upload,
   getWorksheetOptions,
 } from '../../services/sheet-service/SheetService.js';
+import { getErrorMessage } from 'ontime-utils';
 
-export async function requestConnection(req: Request, res: Response) {
+export async function requestConnection(
+  req: Request,
+  res: Response<{ verification_url: string; user_code: string } | ErrorResponse>,
+) {
   const { sheetId } = req.params;
   const file = req.file.path;
 
@@ -28,7 +34,8 @@ export async function requestConnection(req: Request, res: Response) {
 
     res.status(200).send({ verification_url, user_code });
   } catch (error) {
-    res.status(500).send({ message: String(error) });
+    const message = getErrorMessage(error);
+    res.status(500).send({ message });
   }
 
   // delete uploaded file after parsing
@@ -39,52 +46,72 @@ export async function requestConnection(req: Request, res: Response) {
   }
 }
 
-export async function verifyAuthentication(_req: Request, res: Response) {
+export async function verifyAuthentication(
+  _req: Request,
+  res: Response<{ authenticated: AuthenticationStatus } | ErrorResponse>,
+) {
   try {
     const authenticated = hasAuth();
     res.status(200).send(authenticated);
   } catch (error) {
-    res.status(500).send({ message: String(error) });
+    const message = getErrorMessage(error);
+    res.status(500).send({ message });
   }
 }
 
-export async function revokeAuthentication(_req: Request, res: Response) {
+export async function revokeAuthentication(
+  _req: Request,
+  res: Response<{ authenticated: AuthenticationStatus } | ErrorResponse>,
+) {
   try {
     const authenticated = revoke();
     res.status(200).send(authenticated);
   } catch (error) {
-    res.status(500).send({ message: String(error) });
+    const message = getErrorMessage(error);
+    res.status(500).send({ message });
   }
 }
 
-export async function getWorksheetNamesFromSheet(req: Request, res: Response) {
+export async function getWorksheetNamesFromSheet(req: Request, res: Response<string[] | ErrorResponse>) {
   try {
     const { sheetId } = req.params;
     const { worksheetOptions } = await getWorksheetOptions(sheetId);
     res.status(200).send(worksheetOptions);
   } catch (error) {
-    res.status(500).send({ message: String(error) });
+    const message = getErrorMessage(error);
+    res.status(500).send({ message });
   }
 }
 
-export async function readFromSheet(req: Request, res: Response) {
+export async function readFromSheet(
+  req: Request,
+  res: Response<
+    | {
+        rundown: OntimeRundown;
+        customFields: CustomFields;
+      }
+    | ErrorResponse
+  >,
+) {
   try {
     const { sheetId } = req.params;
     const { options } = req.body;
     const data = await download(sheetId, options);
     res.status(200).send(data);
   } catch (error) {
-    res.status(500).send({ message: String(error) });
+    const message = getErrorMessage(error);
+    res.status(500).send({ message });
   }
 }
 
-export async function writeToSheet(req: Request, res: Response) {
+export async function writeToSheet(req: Request, res: Response<void | ErrorResponse>) {
   try {
     const { sheetId } = req.params;
     const { options } = req.body;
     await upload(sheetId, options);
     res.status(200).send();
   } catch (error) {
-    res.status(500).send({ message: String(error) });
+    const message = getErrorMessage(error);
+    res.status(500).send({ message });
   }
 }

apps/server/src/api-data/url-presets/urlPresets.controller.ts

@@ -4,6 +4,7 @@ import { Request, Response } from 'express';
 
 import { DataProvider } from '../../classes/data-provider/DataProvider.js';
 import { failIsNotArray } from '../../utils/routerUtils.js';
+import { getErrorMessage } from 'ontime-utils';
 
 export async function getUrlPresets(_req: Request, res: Response<URLPreset[]>) {
   const presets = DataProvider.getUrlPresets();
@@ -23,6 +24,7 @@ export async function postUrlPresets(req: Request, res: Response<URLPreset[] | E
     await DataProvider.setUrlPresets(newPresets);
     res.status(200).send(newPresets);
   } catch (error) {
-    res.status(400).send({ message: String(error) });
+    const message = getErrorMessage(error);
+    res.status(400).send({ message });
   }
 }

apps/server/src/api-data/view-settings/viewSettings.controller.ts

@@ -4,6 +4,7 @@ import { Request, Response } from 'express';
 
 import { DataProvider } from '../../classes/data-provider/DataProvider.js';
 import { failEmptyObjects } from '../../utils/routerUtils.js';
+import { getErrorMessage } from 'ontime-utils';
 
 export async function getViewSettings(_req: Request, res: Response<ViewSettings>) {
   const views = DataProvider.getViewSettings();
@@ -27,6 +28,7 @@ export async function postViewSettings(req: Request, res: Response<ViewSettings
     await DataProvider.setViewSettings(newData);
     res.status(200).send(newData);
   } catch (error) {
-    res.status(400).send({ message: String(error) });
+    const message = getErrorMessage(error);
+    res.status(400).send({ message });
   }
 }

apps/server/src/api-integration/integration.router.ts

@@ -13,7 +13,7 @@ import { logger } from '../classes/Logger.js';
 import { objectFromPath } from '../adapters/utils/parse.js';
 
 import { dispatchFromAdapter } from './integration.controller.js';
-import { unpackError } from 'ontime-utils';
+import { getErrorMessage } from 'ontime-utils';
 import { eventStore } from '../stores/EventStore.js';
 
 export const integrationRouter = express.Router();
@@ -46,7 +46,7 @@ integrationRouter.get('/*', (req: Request, res: Response) => {
     const reply = dispatchFromAdapter(action, params, 'http');
     res.status(202).json(reply);
   } catch (error) {
-    const errorMessage = unpackError(error);
+    const errorMessage = getErrorMessage(error);
     logger.error(LogOrigin.Rx, `HTTP IN: ${errorMessage}`);
     res.status(500).send({ message: errorMessage });
   }
@@ -57,9 +57,7 @@ integrationRouter.get('/poll', (_req: Request, res: Response<Partial<RuntimeStor
     const state = eventStore.poll();
     res.status(200).send(state);
   } catch (error) {
-    const message = unpackError(error);
+    const message = getErrorMessage(error);
-    res.status(500).send({
+    res.status(500).send({ message: `Could not get sync data: ${message}` });
-      message: `Could not get sync data: ${message}`,
-    });
   }
 });

packages/utils/index.ts

@@ -55,7 +55,7 @@ export { deepmerge } from './src/externals/deepmerge.js';
 export { deleteAtIndex, insertAtIndex, reorderArray, sortArrayByProperty } from './src/array-utils/arrayUtils.js';
 
 // generic utilities
-export { unpackError } from './src/generic/generic.js';
+export { getErrorMessage } from './src/generic/generic.js';
 export { obfuscate, unobfuscate } from './src/generic/generic.js';
 export { isNumeric } from './src/types/types.js';
 

packages/utils/src/generic/generic.ts

@@ -1,4 +1,4 @@
-export function unpackError(error: unknown): string {
+export function getErrorMessage(error: unknown): string {
   if (error instanceof Error) {
     return error.message;
   }