From ecf1a0e4ad8234537d631128c06dd404c33ef365 Mon Sep 17 00:00:00 2001 From: Panagiotis Papadopoulos Date: Sat, 8 Mar 2025 17:07:25 +0100 Subject: [PATCH] refactor(utils): add safeExtractMessageAndStackFromError util to remove code duplication --- src/routes/api/export.ts | 6 ++++-- src/routes/api/import.ts | 5 +++-- src/routes/api/script.ts | 4 +++- src/routes/api/sql.ts | 4 +++- src/routes/api/sync.ts | 5 +++-- src/routes/custom.ts | 7 +++---- src/routes/routes.ts | 7 ++----- src/services/utils.spec.ts | 17 +++++++++++++++++ src/services/utils.ts | 6 ++++++ 9 files changed, 44 insertions(+), 17 deletions(-) diff --git a/src/routes/api/export.ts b/src/routes/api/export.ts index 5aa072a75..7433cd552 100644 --- a/src/routes/api/export.ts +++ b/src/routes/api/export.ts @@ -9,6 +9,7 @@ import log from "../../services/log.js"; import NotFoundError from "../../errors/not_found_error.js"; import type { Request, Response } from "express"; import ValidationError from "../../errors/validation_error.js"; +import { safeExtractMessageAndStackFromError } from "../../services/utils.js"; function exportBranch(req: Request, res: Response) { const { branchId, type, format, version, taskId } = req.params; @@ -38,10 +39,11 @@ function exportBranch(req: Request, res: Response) { throw new NotFoundError(`Unrecognized export format '${format}'`); } } catch (e: unknown) { - const message = `Export failed with following error: '${(e instanceof Error) ? e.message : e}'. More details might be in the logs.`; + const [errMessage, errStack] = safeExtractMessageAndStackFromError(e); + const message = `Export failed with following error: '${errMessage}'. More details might be in the logs.`; taskContext.reportError(message); - log.error((e instanceof Error) ? message + e.stack : message); + log.error(errMessage + errStack); res.setHeader("Content-Type", "text/plain").status(500).send(message); } diff --git a/src/routes/api/import.ts b/src/routes/api/import.ts index 1efa780ae..6dfce5870 100644 --- a/src/routes/api/import.ts +++ b/src/routes/api/import.ts @@ -13,6 +13,7 @@ import TaskContext from "../../services/task_context.js"; import ValidationError from "../../errors/validation_error.js"; import type { Request } from "express"; import type BNote from "../../becca/entities/bnote.js"; +import { safeExtractMessageAndStackFromError } from "../../services/utils.js"; async function importNotesToBranch(req: Request) { const { parentNoteId } = req.params; @@ -69,7 +70,7 @@ async function importNotesToBranch(req: Request) { note = await singleImportService.importSingleFile(taskContext, file, parentNote); } } catch (e: unknown) { - const [errMessage, errStack] = e instanceof Error ? [e.message, e.stack] : ["Unknown Error", undefined]; + const [errMessage, errStack] = safeExtractMessageAndStackFromError(e); const message = `Import failed with following error: '${errMessage}'. More details might be in the logs.`; taskContext.reportError(message); @@ -122,7 +123,7 @@ async function importAttachmentsToNote(req: Request) { try { await singleImportService.importAttachment(taskContext, file, parentNote); } catch (e: unknown) { - const [errMessage, errStack] = e instanceof Error ? [e.message, e.stack] : ["Unknown Error", undefined]; + const [errMessage, errStack] = safeExtractMessageAndStackFromError(e); const message = `Import failed with following error: '${errMessage}'. More details might be in the logs.`; taskContext.reportError(message); diff --git a/src/routes/api/script.ts b/src/routes/api/script.ts index a289dc565..c702a82d8 100644 --- a/src/routes/api/script.ts +++ b/src/routes/api/script.ts @@ -6,6 +6,7 @@ import becca from "../../becca/becca.js"; import syncService from "../../services/sync.js"; import sql from "../../services/sql.js"; import type { Request } from "express"; +import { safeExtractMessageAndStackFromError } from "../../services/utils.js"; interface ScriptBody { script: string; @@ -34,9 +35,10 @@ async function exec(req: Request) { maxEntityChangeId: syncService.getMaxEntityChangeId() }; } catch (e: unknown) { + const [errMessage] = safeExtractMessageAndStackFromError(e); return { success: false, - error: (e instanceof Error) ? e.message : "Unknown Error" + error: errMessage }; } } diff --git a/src/routes/api/sql.ts b/src/routes/api/sql.ts index a9876b657..a78c3e2f4 100644 --- a/src/routes/api/sql.ts +++ b/src/routes/api/sql.ts @@ -4,6 +4,7 @@ import sql from "../../services/sql.js"; import becca from "../../becca/becca.js"; import type { Request } from "express"; import ValidationError from "../../errors/validation_error.js"; +import { safeExtractMessageAndStackFromError } from "../../services/utils.js"; function getSchema() { const tableNames = sql.getColumn(`SELECT name FROM sqlite_master WHERE type='table' AND name NOT LIKE 'sqlite_%' ORDER BY name`); @@ -57,9 +58,10 @@ function execute(req: Request) { results }; } catch (e: unknown) { + const [errMessage] = safeExtractMessageAndStackFromError(e); return { success: false, - error: (e instanceof Error) ? e.message : "Unknown Error" + error: errMessage }; } } diff --git a/src/routes/api/sync.ts b/src/routes/api/sync.ts index 8e186f92a..50088a097 100644 --- a/src/routes/api/sync.ts +++ b/src/routes/api/sync.ts @@ -9,7 +9,7 @@ import optionService from "../../services/options.js"; import contentHashService from "../../services/content_hash.js"; import log from "../../services/log.js"; import syncOptions from "../../services/sync_options.js"; -import utils from "../../services/utils.js"; +import utils, { safeExtractMessageAndStackFromError } from "../../services/utils.js"; import ws from "../../services/ws.js"; import type { Request } from "express"; import type { EntityChange } from "../../services/entity_changes_interface.js"; @@ -31,9 +31,10 @@ async function testSync() { return { success: true, message: t("test_sync.successful") }; } catch (e: unknown) { + const [errMessage] = safeExtractMessageAndStackFromError(e); return { success: false, - error: (e instanceof Error) ? e.message : "Unknown Error" + error: errMessage }; } } diff --git a/src/routes/custom.ts b/src/routes/custom.ts index 40188bcc1..093a0e6eb 100644 --- a/src/routes/custom.ts +++ b/src/routes/custom.ts @@ -5,6 +5,7 @@ import cls from "../services/cls.js"; import sql from "../services/sql.js"; import becca from "../becca/becca.js"; import type { Request, Response, Router } from "express"; +import { safeExtractMessageAndStackFromError } from "../services/utils.js"; function handleRequest(req: Request, res: Response) { // express puts content after first slash into 0 index element @@ -26,7 +27,7 @@ function handleRequest(req: Request, res: Response) { try { match = path.match(regex); } catch (e: unknown) { - const [errMessage, errStack] = e instanceof Error ? [e.message, e.stack] : ["Unknown Error", undefined]; + const [errMessage, errStack] = safeExtractMessageAndStackFromError(e); log.error(`Testing path for label '${attr.attributeId}', regex '${attr.value}' failed with error: ${errMessage}, stack: ${errStack}`); continue; } @@ -47,10 +48,8 @@ function handleRequest(req: Request, res: Response) { res }); } catch (e: unknown) { - const [errMessage, errStack] = e instanceof Error ? [e.message, e.stack] : ["Unknown Error", undefined]; - + const [errMessage, errStack] = safeExtractMessageAndStackFromError(e); log.error(`Custom handler '${note.noteId}' failed with: ${errMessage}, ${errStack}`); - res.setHeader("Content-Type", "text/plain").status(500).send(errMessage); } } else if (attr.name === "customResourceProvider") { diff --git a/src/routes/routes.ts b/src/routes/routes.ts index 227b10d3c..02b4e2333 100644 --- a/src/routes/routes.ts +++ b/src/routes/routes.ts @@ -1,6 +1,6 @@ "use strict"; -import { isElectron } from "../services/utils.js"; +import { isElectron, safeExtractMessageAndStackFromError } from "../services/utils.js"; import multer from "multer"; import log from "../services/log.js"; import express from "express"; @@ -494,10 +494,7 @@ function handleResponse(resultHandler: ApiResultHandler, req: express.Request, r } function handleException(e: unknown | Error, method: HttpMethod, path: string, res: express.Response) { - - const [errMessage, errStack]: [message: string, stack: string] = (e instanceof Error) - ? [e.message, e.stack || "No Stack Trace"] - : ["Unknown Error", "No Stack Trace"]; + const [errMessage, errStack] = safeExtractMessageAndStackFromError(e); log.error(`${method} ${path} threw exception: '${errMessage}', stack: ${errStack}`); diff --git a/src/services/utils.spec.ts b/src/services/utils.spec.ts index 52c173da4..afa1ba4e7 100644 --- a/src/services/utils.spec.ts +++ b/src/services/utils.spec.ts @@ -500,6 +500,23 @@ describe("#isDev", () => { }); }); +describe("#safeExtractMessageAndStackFromError", () => { + it("should correctly extract the message and stack property if it gets passed an instance of an Error", () => { + const testMessage = "Test Message"; + const testError = new Error(testMessage); + const actual = utils.safeExtractMessageAndStackFromError(testError); + expect(actual[0]).toBe(testMessage); + expect(actual[1]).not.toBeUndefined(); + }); + + it("should use the fallback 'Unknown Error' message, if it gets passed anything else than an instance of an Error", () => { + const testNonError = "this is not an instance of an Error, but JS technically allows us to throw this anyways"; + const actual = utils.safeExtractMessageAndStackFromError(testNonError); + expect(actual[0]).toBe("Unknown Error"); + expect(actual[1]).toBeUndefined(); + }); +}) + describe("#formatDownloadTitle", () => { //prettier-ignore const testCases: [fnValue: Parameters, expectedValue: ReturnType][] = [ diff --git a/src/services/utils.ts b/src/services/utils.ts index 6c539f9de..4dfa9fd17 100644 --- a/src/services/utils.ts +++ b/src/services/utils.ts @@ -363,6 +363,11 @@ export function processStringOrBuffer(data: string | Buffer | null) { } } +export function safeExtractMessageAndStackFromError(err: unknown) { + return (err instanceof Error) ? [err.message, err.stack] as const : ["Unknown Error", undefined] as const; +} + + export default { compareVersions, crash, @@ -393,6 +398,7 @@ export default { removeDiacritic, removeTextFileExtension, replaceAll, + safeExtractMessageAndStackFromError, sanitizeSqlIdentifier, stripTags, timeLimit,