From 0f0e55deb24feb7a454d6c62bc663e544f09953e Mon Sep 17 00:00:00 2001 From: Panagiotis Papadopoulos Date: Wed, 5 Mar 2025 18:09:44 +0100 Subject: [PATCH 01/22] chore(lint): fix lint issues in src/routes --- src/routes/api/clipper.ts | 4 ++-- src/routes/api/revisions.ts | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/routes/api/clipper.ts b/src/routes/api/clipper.ts index 4d821e480..0b7b315aa 100644 --- a/src/routes/api/clipper.ts +++ b/src/routes/api/clipper.ts @@ -219,9 +219,9 @@ function handshake() { } function findNotesByUrl(req: Request) { - let pageUrl = req.params.noteUrl; + const pageUrl = req.params.noteUrl; const clipperInbox = getClipperInboxNote(); - let foundPage = findClippingNote(clipperInbox, pageUrl, null); + const foundPage = findClippingNote(clipperInbox, pageUrl, null); return { noteId: foundPage ? foundPage.noteId : null }; diff --git a/src/routes/api/revisions.ts b/src/routes/api/revisions.ts index e65d20113..0e4cf1136 100644 --- a/src/routes/api/revisions.ts +++ b/src/routes/api/revisions.ts @@ -111,7 +111,7 @@ function eraseRevision(req: Request) { } function eraseAllExcessRevisions() { - let allNoteIds = sql.getRows("SELECT noteId FROM notes WHERE SUBSTRING(noteId, 1, 1) != '_'") as { noteId: string }[]; + const allNoteIds = sql.getRows("SELECT noteId FROM notes WHERE SUBSTRING(noteId, 1, 1) != '_'") as { noteId: string }[]; allNoteIds.forEach((row) => { becca.getNote(row.noteId)?.eraseExcessRevisionSnapshots(); }); From dfb8982a997502d0c2374e706d2d18112121f7ed Mon Sep 17 00:00:00 2001 From: Panagiotis Papadopoulos Date: Thu, 6 Mar 2025 22:44:54 +0100 Subject: [PATCH 02/22] chore(lint): improve type and get rid of "any" --- src/becca/entities/bnote.ts | 2 +- src/routes/api/attachments.ts | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/src/becca/entities/bnote.ts b/src/becca/entities/bnote.ts index b0edd8c61..971055755 100644 --- a/src/becca/entities/bnote.ts +++ b/src/becca/entities/bnote.ts @@ -1618,7 +1618,7 @@ class BNote extends AbstractBeccaEntity { * @param matchBy - choose by which property we detect if to update an existing attachment. * Supported values are either 'attachmentId' (default) or 'title' */ - saveAttachment({ attachmentId, role, mime, title, content, position }: AttachmentRow, matchBy = "attachmentId") { + saveAttachment({ attachmentId, role, mime, title, content, position }: AttachmentRow, matchBy: "attachmentId" | "title" | undefined = "attachmentId") { if (!["attachmentId", "title"].includes(matchBy)) { throw new Error(`Unsupported value '${matchBy}' for matchBy param, has to be either 'attachmentId' or 'title'.`); } diff --git a/src/routes/api/attachments.ts b/src/routes/api/attachments.ts index a609b93dc..b0b46db1f 100644 --- a/src/routes/api/attachments.ts +++ b/src/routes/api/attachments.ts @@ -33,7 +33,9 @@ function getAllAttachments(req: Request) { function saveAttachment(req: Request) { const { noteId } = req.params; const { attachmentId, role, mime, title, content } = req.body; - const { matchBy } = req.query as any; + const matchByQuery = req.query.matchBy + const isValidMatchBy = (typeof matchByQuery === "string") && (matchByQuery === "attachmentId" || matchByQuery === "title"); + const matchBy = isValidMatchBy ? matchByQuery : undefined; const note = becca.getNoteOrThrow(noteId); note.saveAttachment({ attachmentId, role, mime, title, content }, matchBy); From c8e36942fc8e31ea02f7d44695c6f84471fec0b5 Mon Sep 17 00:00:00 2001 From: Panagiotis Papadopoulos Date: Thu, 6 Mar 2025 22:59:31 +0100 Subject: [PATCH 03/22] chore(lint): get rid of "any" in attachments req.file is of type "Express.Multer.File | undefined". Returning with an "uploaded: false" type object -> same handling as in image.ts --- src/routes/api/attachments.ts | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/routes/api/attachments.ts b/src/routes/api/attachments.ts index b0b46db1f..718c99914 100644 --- a/src/routes/api/attachments.ts +++ b/src/routes/api/attachments.ts @@ -43,7 +43,14 @@ function saveAttachment(req: Request) { function uploadAttachment(req: Request) { const { noteId } = req.params; - const { file } = req as any; + const { file } = req; + + if (!file) { + return { + uploaded: false, + message: `Missing attachment data.` + }; + } const note = becca.getNoteOrThrow(noteId); let url; From 7feb38ffa1bcc1881d22e33cdd7bde8ef8d8e0bc Mon Sep 17 00:00:00 2001 From: Panagiotis Papadopoulos Date: Thu, 6 Mar 2025 23:21:47 +0100 Subject: [PATCH 04/22] chore(lint): fix no-unused-vars errors --- src/routes/api/image.ts | 2 +- src/routes/api/recent_changes.ts | 1 - src/routes/api/relation-map.ts | 2 +- src/routes/api/revisions.ts | 1 - src/routes/api/similar_notes.ts | 2 +- src/routes/api_docs.ts | 2 +- src/routes/custom.ts | 2 +- src/routes/error_handlers.ts | 2 +- 8 files changed, 6 insertions(+), 8 deletions(-) diff --git a/src/routes/api/image.ts b/src/routes/api/image.ts index d2d943486..d2f2b5632 100644 --- a/src/routes/api/image.ts +++ b/src/routes/api/image.ts @@ -82,7 +82,7 @@ function updateImage(req: Request) { const { noteId } = req.params; const { file } = req; - const note = becca.getNoteOrThrow(noteId); + const _note = becca.getNoteOrThrow(noteId); if (!file) { return { diff --git a/src/routes/api/recent_changes.ts b/src/routes/api/recent_changes.ts index 55a5ee9da..67b7436a0 100644 --- a/src/routes/api/recent_changes.ts +++ b/src/routes/api/recent_changes.ts @@ -5,7 +5,6 @@ import protectedSessionService from "../../services/protected_session.js"; import noteService from "../../services/notes.js"; import becca from "../../becca/becca.js"; import type { Request } from "express"; -import type { RevisionRow } from "../../becca/entities/rows.js"; interface RecentChangeRow { noteId: string; diff --git a/src/routes/api/relation-map.ts b/src/routes/api/relation-map.ts index 503bb36ef..2cf591b50 100644 --- a/src/routes/api/relation-map.ts +++ b/src/routes/api/relation-map.ts @@ -30,7 +30,7 @@ function getRelationMap(req: Request) { return resp; } - const questionMarks = noteIds.map((noteId) => "?").join(","); + const questionMarks = noteIds.map((_noteId) => "?").join(","); const relationMapNote = becca.getNoteOrThrow(relationMapNoteId); diff --git a/src/routes/api/revisions.ts b/src/routes/api/revisions.ts index 0e4cf1136..aeb6f32d2 100644 --- a/src/routes/api/revisions.ts +++ b/src/routes/api/revisions.ts @@ -1,7 +1,6 @@ "use strict"; import beccaService from "../../becca/becca_service.js"; -import revisionService from "../../services/revisions.js"; import utils from "../../services/utils.js"; import sql from "../../services/sql.js"; import cls from "../../services/cls.js"; diff --git a/src/routes/api/similar_notes.ts b/src/routes/api/similar_notes.ts index 930f53282..8cd82dc72 100644 --- a/src/routes/api/similar_notes.ts +++ b/src/routes/api/similar_notes.ts @@ -8,7 +8,7 @@ import becca from "../../becca/becca.js"; async function getSimilarNotes(req: Request) { const noteId = req.params.noteId; - const note = becca.getNoteOrThrow(noteId); + const _note = becca.getNoteOrThrow(noteId); return await similarityService.findSimilarNotes(noteId); } diff --git a/src/routes/api_docs.ts b/src/routes/api_docs.ts index 10d894056..df069a24f 100644 --- a/src/routes/api_docs.ts +++ b/src/routes/api_docs.ts @@ -1,4 +1,4 @@ -import type { Application, Router } from "express"; +import type { Application } from "express"; import swaggerUi from "swagger-ui-express"; import { readFile } from "fs/promises"; import { fileURLToPath } from "url"; diff --git a/src/routes/custom.ts b/src/routes/custom.ts index f2b961f80..d8b90de5c 100644 --- a/src/routes/custom.ts +++ b/src/routes/custom.ts @@ -68,7 +68,7 @@ function handleRequest(req: Request, res: Response) { function register(router: Router) { // explicitly no CSRF middleware since it's meant to allow integration from external services - router.all("/custom/:path*", (req: Request, res: Response, next) => { + router.all("/custom/:path*", (req: Request, res: Response, _next) => { cls.namespace.bindEmitter(req); cls.namespace.bindEmitter(res); diff --git a/src/routes/error_handlers.ts b/src/routes/error_handlers.ts index a73599e38..e773fcb22 100644 --- a/src/routes/error_handlers.ts +++ b/src/routes/error_handlers.ts @@ -22,7 +22,7 @@ function register(app: Application) { }); // error handler - app.use((err: any, req: Request, res: Response, next: NextFunction) => { + app.use((err: any, req: Request, res: Response, _next: NextFunction) => { if (err.status !== 404) { log.info(err); } else { From 04f3b637f94b3d5775c01e9e603134cc3f1e302c Mon Sep 17 00:00:00 2001 From: Panagiotis Papadopoulos Date: Thu, 6 Mar 2025 23:32:05 +0100 Subject: [PATCH 05/22] chore(lint): fix no-explicit-any in export.ts --- src/routes/api/export.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/routes/api/export.ts b/src/routes/api/export.ts index 9023125f9..5aa072a75 100644 --- a/src/routes/api/export.ts +++ b/src/routes/api/export.ts @@ -37,11 +37,11 @@ function exportBranch(req: Request, res: Response) { } else { throw new NotFoundError(`Unrecognized export format '${format}'`); } - } catch (e: any) { - const message = `Export failed with following error: '${e.message}'. More details might be in the logs.`; + } catch (e: unknown) { + const message = `Export failed with following error: '${(e instanceof Error) ? e.message : e}'. More details might be in the logs.`; taskContext.reportError(message); - log.error(message + e.stack); + log.error((e instanceof Error) ? message + e.stack : message); res.setHeader("Content-Type", "text/plain").status(500).send(message); } From ba5152de4094400f2f8a0aba9c34dd359b79b64c Mon Sep 17 00:00:00 2001 From: Panagiotis Papadopoulos Date: Fri, 7 Mar 2025 22:22:44 +0100 Subject: [PATCH 06/22] refactor(errors): extend errors from Error and add/assign statusCode this is in preparation for updating the routes/handleException method, to get rid of "any" (and improve in general) --- src/errors/not_found_error.ts | 9 +++++---- src/errors/validation_error.ts | 10 ++++++---- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/src/errors/not_found_error.ts b/src/errors/not_found_error.ts index 6d8fbe4d8..ae97a6ac5 100644 --- a/src/errors/not_found_error.ts +++ b/src/errors/not_found_error.ts @@ -1,8 +1,9 @@ -class NotFoundError { - message: string; - +class NotFoundError extends Error { + statusCode: number; constructor(message: string) { - this.message = message; + super(message); + this.name = "NotFoundError"; + this.statusCode = 404; } } diff --git a/src/errors/validation_error.ts b/src/errors/validation_error.ts index f9c0ba6fc..35eb5897d 100644 --- a/src/errors/validation_error.ts +++ b/src/errors/validation_error.ts @@ -1,9 +1,11 @@ -class ValidationError { - message: string; - +class ValidationError extends Error { + statusCode: number; constructor(message: string) { - this.message = message; + super(message) + this.name = "ValidationError"; + this.statusCode = 400; } + } export default ValidationError; From d8ce38513400db8da60c504641166bd9e62ebf5a Mon Sep 17 00:00:00 2001 From: Panagiotis Papadopoulos Date: Fri, 7 Mar 2025 22:27:13 +0100 Subject: [PATCH 07/22] refactor(routes): refactor handleException and get rid of "any" type --- src/routes/routes.ts | 28 +++++++++++++--------------- 1 file changed, 13 insertions(+), 15 deletions(-) diff --git a/src/routes/routes.ts b/src/routes/routes.ts index 7d5fea44b..ad1850aca 100644 --- a/src/routes/routes.ts +++ b/src/routes/routes.ts @@ -493,22 +493,20 @@ function handleResponse(resultHandler: ApiResultHandler, req: express.Request, r log.request(req, res, Date.now() - start, responseLength); } -function handleException(e: any, method: HttpMethod, path: string, res: express.Response) { - log.error(`${method} ${path} threw exception: '${e.message}', stack: ${e.stack}`); +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"]; + + log.error(`${method} ${path} threw exception: '${errMessage}', stack: ${errStack}`); + + const resStatusCode = (e instanceof ValidationError || e instanceof NotFoundError) ? e.statusCode : 500; + + res.status(resStatusCode).json({ + message: errMessage + }); - if (e instanceof ValidationError) { - res.status(400).json({ - message: e.message - }); - } else if (e instanceof NotFoundError) { - res.status(404).json({ - message: e.message - }); - } else { - res.status(500).json({ - message: e.message - }); - } } function createUploadMiddleware() { From 39d45dc11b2388c65018f793fddf4936e6b68fdb Mon Sep 17 00:00:00 2001 From: Panagiotis Papadopoulos Date: Fri, 7 Mar 2025 22:31:55 +0100 Subject: [PATCH 08/22] refactor(error_handlers): use existing NotFoundError class also gets rid of "any" type :-) --- src/routes/error_handlers.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/routes/error_handlers.ts b/src/routes/error_handlers.ts index e773fcb22..c570a9dbd 100644 --- a/src/routes/error_handlers.ts +++ b/src/routes/error_handlers.ts @@ -1,5 +1,6 @@ import type { Application, NextFunction, Request, Response } from "express"; import log from "../services/log.js"; +import NotFoundError from "../errors/not_found_error.js"; function register(app: Application) { app.use((err: any, req: Request, res: Response, next: NextFunction) => { @@ -16,8 +17,7 @@ function register(app: Application) { // catch 404 and forward to error handler app.use((req, res, next) => { - const err = new Error(`Router not found for request ${req.method} ${req.url}`); - (err as any).status = 404; + const err = new NotFoundError(`Router not found for request ${req.method} ${req.url}`); next(err); }); From 2c91f6e7bc207a1b83a4291c60e54f4a177b3fb2 Mon Sep 17 00:00:00 2001 From: Panagiotis Papadopoulos Date: Fri, 7 Mar 2025 22:47:03 +0100 Subject: [PATCH 09/22] refactor(errors): add HttpError class and extend existing errors from it --- src/errors/http_error.ts | 13 +++++++++++++ src/errors/not_found_error.ts | 10 ++++++---- src/errors/validation_error.ts | 9 +++++---- 3 files changed, 24 insertions(+), 8 deletions(-) create mode 100644 src/errors/http_error.ts diff --git a/src/errors/http_error.ts b/src/errors/http_error.ts new file mode 100644 index 000000000..2ab806d8b --- /dev/null +++ b/src/errors/http_error.ts @@ -0,0 +1,13 @@ +class HttpError extends Error { + + statusCode: number; + + constructor(message: string, statusCode: number) { + super(message); + this.name = "HttpError"; + this.statusCode = statusCode; + } + +} + +export default HttpError; \ No newline at end of file diff --git a/src/errors/not_found_error.ts b/src/errors/not_found_error.ts index ae97a6ac5..44f718a2c 100644 --- a/src/errors/not_found_error.ts +++ b/src/errors/not_found_error.ts @@ -1,10 +1,12 @@ -class NotFoundError extends Error { - statusCode: number; +import HttpError from "./http_error.js"; + +class NotFoundError extends HttpError { + constructor(message: string) { - super(message); + super(message, 404); this.name = "NotFoundError"; - this.statusCode = 404; } + } export default NotFoundError; diff --git a/src/errors/validation_error.ts b/src/errors/validation_error.ts index 35eb5897d..25cdd509e 100644 --- a/src/errors/validation_error.ts +++ b/src/errors/validation_error.ts @@ -1,9 +1,10 @@ -class ValidationError extends Error { - statusCode: number; +import HttpError from "./http_error.js"; + +class ValidationError extends HttpError { + constructor(message: string) { - super(message) + super(message, 400) this.name = "ValidationError"; - this.statusCode = 400; } } From 0c8df7f885dcfa0e2b9a8df188422ce64bdf07ab Mon Sep 17 00:00:00 2001 From: Panagiotis Papadopoulos Date: Fri, 7 Mar 2025 23:29:35 +0100 Subject: [PATCH 10/22] refactor(error_handlers): use newly added ForbiddenError class --- src/errors/forbidden_error.ts | 12 ++++++++++++ src/routes/error_handlers.ts | 6 ++---- 2 files changed, 14 insertions(+), 4 deletions(-) create mode 100644 src/errors/forbidden_error.ts diff --git a/src/errors/forbidden_error.ts b/src/errors/forbidden_error.ts new file mode 100644 index 000000000..3e62665b0 --- /dev/null +++ b/src/errors/forbidden_error.ts @@ -0,0 +1,12 @@ +import HttpError from "./http_error.js"; + +class ForbiddenError extends HttpError { + + constructor(message: string) { + super(message, 403); + this.name = "ForbiddenError"; + } + +} + +export default ForbiddenError; \ No newline at end of file diff --git a/src/routes/error_handlers.ts b/src/routes/error_handlers.ts index c570a9dbd..074bf0053 100644 --- a/src/routes/error_handlers.ts +++ b/src/routes/error_handlers.ts @@ -1,6 +1,7 @@ import type { Application, NextFunction, Request, Response } from "express"; import log from "../services/log.js"; import NotFoundError from "../errors/not_found_error.js"; +import ForbiddenError from "../errors/forbidden_error.js"; function register(app: Application) { app.use((err: any, req: Request, res: Response, next: NextFunction) => { @@ -9,10 +10,7 @@ function register(app: Application) { } log.error(`Invalid CSRF token: ${req.headers["x-csrf-token"]}, secret: ${req.cookies["_csrf"]}`); - - err = new Error("Invalid CSRF token"); - err.status = 403; - next(err); + next(new ForbiddenError("Invalid CSRF token")); }); // catch 404 and forward to error handler From 76574f0938c965cc2f54a0ca0ca182b3b68c1a6c Mon Sep 17 00:00:00 2001 From: Panagiotis Papadopoulos Date: Fri, 7 Mar 2025 23:35:19 +0100 Subject: [PATCH 11/22] refactor(error_handlers): use HttpError classes in errorHandler also gets rid of "any" type :-) --- src/routes/error_handlers.ts | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/src/routes/error_handlers.ts b/src/routes/error_handlers.ts index 074bf0053..6a4ee714b 100644 --- a/src/routes/error_handlers.ts +++ b/src/routes/error_handlers.ts @@ -2,6 +2,7 @@ import type { Application, NextFunction, Request, Response } from "express"; import log from "../services/log.js"; import NotFoundError from "../errors/not_found_error.js"; import ForbiddenError from "../errors/forbidden_error.js"; +import HttpError from "../errors/http_error.js"; function register(app: Application) { app.use((err: any, req: Request, res: Response, next: NextFunction) => { @@ -20,17 +21,19 @@ function register(app: Application) { }); // error handler - app.use((err: any, req: Request, res: Response, _next: NextFunction) => { - if (err.status !== 404) { - log.info(err); - } else { - log.info(`${err.status} ${req.method} ${req.url}`); - } + app.use((err: unknown | Error, req: Request, res: Response, _next: NextFunction) => { - res.status(err.status || 500); - res.send({ - message: err.message + const statusCode = (err instanceof HttpError) ? err.statusCode : 500; + const errMessage = (err instanceof Error && statusCode !== 404) + ? err + : `${statusCode} ${req.method} ${req.url}`; + + log.info(errMessage); + + res.status(statusCode).send({ + message: err instanceof Error ? err.message : "Unknown Error" }); + }); } From 4b6972fb2181db41e6f4e75869b83dddebbaa1d2 Mon Sep 17 00:00:00 2001 From: Panagiotis Papadopoulos Date: Sat, 8 Mar 2025 00:15:46 +0100 Subject: [PATCH 12/22] refactor(error_handlers): get rid of "any" type in csrf error handler --- src/routes/error_handlers.ts | 17 ++++++++++++----- 1 file changed, 12 insertions(+), 5 deletions(-) diff --git a/src/routes/error_handlers.ts b/src/routes/error_handlers.ts index 6a4ee714b..05b05f6a4 100644 --- a/src/routes/error_handlers.ts +++ b/src/routes/error_handlers.ts @@ -5,13 +5,20 @@ import ForbiddenError from "../errors/forbidden_error.js"; import HttpError from "../errors/http_error.js"; function register(app: Application) { - app.use((err: any, req: Request, res: Response, next: NextFunction) => { - if (err.code !== "EBADCSRFTOKEN") { - return next(err); + + app.use((err: unknown | Error, req: Request, res: Response, next: NextFunction) => { + + const isCsrfTokenError = typeof err === "object" + && err + && "code" in err + && err.code === "EBADCSRFTOKEN"; + + if (isCsrfTokenError) { + log.error(`Invalid CSRF token: ${req.headers["x-csrf-token"]}, secret: ${req.cookies["_csrf"]}`); + return next(new ForbiddenError("Invalid CSRF token")); } - log.error(`Invalid CSRF token: ${req.headers["x-csrf-token"]}, secret: ${req.cookies["_csrf"]}`); - next(new ForbiddenError("Invalid CSRF token")); + return next(err); }); // catch 404 and forward to error handler From 07fd5327b1180f7729fd0b8b843b96388fbbd734 Mon Sep 17 00:00:00 2001 From: Panagiotis Papadopoulos Date: Sat, 8 Mar 2025 00:22:12 +0100 Subject: [PATCH 13/22] refactor(routes/custom): get rid of "any" type catch blocks --- src/routes/custom.ts | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/src/routes/custom.ts b/src/routes/custom.ts index d8b90de5c..40188bcc1 100644 --- a/src/routes/custom.ts +++ b/src/routes/custom.ts @@ -25,8 +25,9 @@ function handleRequest(req: Request, res: Response) { try { match = path.match(regex); - } catch (e: any) { - log.error(`Testing path for label '${attr.attributeId}', regex '${attr.value}' failed with error: ${e.message}, stack: ${e.stack}`); + } catch (e: unknown) { + const [errMessage, errStack] = e instanceof Error ? [e.message, e.stack] : ["Unknown Error", undefined]; + log.error(`Testing path for label '${attr.attributeId}', regex '${attr.value}' failed with error: ${errMessage}, stack: ${errStack}`); continue; } @@ -45,10 +46,12 @@ function handleRequest(req: Request, res: Response) { req, res }); - } catch (e: any) { - log.error(`Custom handler '${note.noteId}' failed with: ${e.message}, ${e.stack}`); + } catch (e: unknown) { + const [errMessage, errStack] = e instanceof Error ? [e.message, e.stack] : ["Unknown Error", undefined]; - res.setHeader("Content-Type", "text/plain").status(500).send(e.message); + 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") { fileService.downloadNoteInt(attr.noteId, res); From b56ff558a48b8d384030a18b98a7cb22caedabd4 Mon Sep 17 00:00:00 2001 From: Panagiotis Papadopoulos Date: Sat, 8 Mar 2025 00:39:01 +0100 Subject: [PATCH 14/22] refactor(routes/api/import): get rid of "any" type in catch blocks --- src/routes/api/import.ts | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/src/routes/api/import.ts b/src/routes/api/import.ts index bb46f8990..1efa780ae 100644 --- a/src/routes/api/import.ts +++ b/src/routes/api/import.ts @@ -68,11 +68,12 @@ async function importNotesToBranch(req: Request) { } else { note = await singleImportService.importSingleFile(taskContext, file, parentNote); } - } catch (e: any) { - const message = `Import failed with following error: '${e.message}'. More details might be in the logs.`; + } catch (e: unknown) { + const [errMessage, errStack] = e instanceof Error ? [e.message, e.stack] : ["Unknown Error", undefined]; + const message = `Import failed with following error: '${errMessage}'. More details might be in the logs.`; taskContext.reportError(message); - log.error(message + e.stack); + log.error(message + errStack); return [500, message]; } @@ -120,11 +121,13 @@ async function importAttachmentsToNote(req: Request) { try { await singleImportService.importAttachment(taskContext, file, parentNote); - } catch (e: any) { - const message = `Import failed with following error: '${e.message}'. More details might be in the logs.`; + } catch (e: unknown) { + const [errMessage, errStack] = e instanceof Error ? [e.message, e.stack] : ["Unknown Error", undefined]; + + const message = `Import failed with following error: '${errMessage}'. More details might be in the logs.`; taskContext.reportError(message); - log.error(message + e.stack); + log.error(message + errStack); return [500, message]; } From 91c37fa235904a009c95d54fd16544d406821c4f Mon Sep 17 00:00:00 2001 From: Panagiotis Papadopoulos Date: Sat, 8 Mar 2025 00:54:29 +0100 Subject: [PATCH 15/22] chore(routes/electron): disable lint rule for specific line in this case using "{}" allows all primitive values, which seems to be what is required here. so let's disable the rule "@typescript-eslint/no-empty-object-type" for this line --- src/routes/electron.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/routes/electron.ts b/src/routes/electron.ts index 13147a803..05e21e77b 100644 --- a/src/routes/electron.ts +++ b/src/routes/electron.ts @@ -7,7 +7,7 @@ interface Response { setHeader: (name: string, value: string) => Response; header: (name: string, value: string) => Response; status: (statusCode: number) => Response; - send: (obj: {}) => void; + send: (obj: {}) => void; // eslint-disable-line @typescript-eslint/no-empty-object-type } function init(app: Application) { From 08a6053c385a935a229899e7dd392f8e0c47e8e1 Mon Sep 17 00:00:00 2001 From: Panagiotis Papadopoulos Date: Sat, 8 Mar 2025 15:27:04 +0100 Subject: [PATCH 16/22] refactor(routes/api/clipper): get rid of second htmlSanitizer call for pageUrl -> the value is already sanitized in line 112, there's no need to call htmlSanitizer a second time here --- src/routes/api/clipper.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/routes/api/clipper.ts b/src/routes/api/clipper.ts index 0b7b315aa..56f907c2c 100644 --- a/src/routes/api/clipper.ts +++ b/src/routes/api/clipper.ts @@ -123,8 +123,6 @@ function createNote(req: Request) { note.setLabel("clipType", clipType); if (pageUrl) { - pageUrl = htmlSanitizer.sanitizeUrl(pageUrl); - note.setLabel("pageUrl", pageUrl); note.setLabel("iconClass", "bx bx-globe"); } From dd9e1e69d7ec80c8ff081b4029fdd132e19670f2 Mon Sep 17 00:00:00 2001 From: Panagiotis Papadopoulos Date: Sat, 8 Mar 2025 15:27:39 +0100 Subject: [PATCH 17/22] fix(routes/api/clipper): fix typo in error message --- src/routes/api/clipper.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/routes/api/clipper.ts b/src/routes/api/clipper.ts index 56f907c2c..f08ef5c57 100644 --- a/src/routes/api/clipper.ts +++ b/src/routes/api/clipper.ts @@ -137,7 +137,7 @@ function createNote(req: Request) { const existingContent = note.getContent(); if (typeof existingContent !== "string") { - throw new ValidationError("Invalid note content tpye."); + throw new ValidationError("Invalid note content type."); } const rewrittenContent = processContent(images, note, content); const newContent = `${existingContent}${existingContent.trim() ? "
" : ""}${rewrittenContent}`; From 272d7cd652c6b6b4124be56cca7e6dba5a6f7073 Mon Sep 17 00:00:00 2001 From: Panagiotis Papadopoulos Date: Sat, 8 Mar 2025 15:31:12 +0100 Subject: [PATCH 18/22] chore(routes/api/clipper): fix prefer-const lint errors --- src/routes/api/clipper.ts | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/src/routes/api/clipper.ts b/src/routes/api/clipper.ts index f08ef5c57..9f1510fcb 100644 --- a/src/routes/api/clipper.ts +++ b/src/routes/api/clipper.ts @@ -30,12 +30,12 @@ function addClipping(req: Request) { // if a note under the clipperInbox has the same 'pageUrl' attribute, // add the content to that note and clone it under today's inbox // otherwise just create a new note under today's inbox - let { title, content, pageUrl, images } = req.body; + const { title, content, images } = req.body; const clipType = "clippings"; const clipperInbox = getClipperInboxNote(); - pageUrl = htmlSanitizer.sanitizeUrl(pageUrl); + const pageUrl = htmlSanitizer.sanitizeUrl(req.body.pageUrl); let clippingNote = findClippingNote(clipperInbox, pageUrl, clipType); if (!clippingNote) { @@ -100,16 +100,15 @@ function getClipperInboxNote() { } function createNote(req: Request) { - let { title, content, pageUrl, images, clipType, labels } = req.body; + const { content, images, labels } = req.body; - if (!title || !title.trim()) { - title = `Clipped note from ${pageUrl}`; - } + const clipType = htmlSanitizer.sanitize(req.body.clipType); + const pageUrl = htmlSanitizer.sanitizeUrl(req.body.pageUrl); - clipType = htmlSanitizer.sanitize(clipType); + const trimmedTitle = (typeof req.body.title === "string") ? req.body.title.trim() : ""; + const title = trimmedTitle || `Clipped note from ${pageUrl}`; const clipperInbox = getClipperInboxNote(); - pageUrl = htmlSanitizer.sanitizeUrl(pageUrl); let note = findClippingNote(clipperInbox, pageUrl, clipType); if (!note) { From e3d0c53d039a41123be90c1f0ee5b24796fef7a8 Mon Sep 17 00:00:00 2001 From: Panagiotis Papadopoulos Date: Sat, 8 Mar 2025 16:01:53 +0100 Subject: [PATCH 19/22] chore(routes): fix no-explicit-any lint/ts error for catch blocks --- src/routes/api/script.ts | 7 +++++-- src/routes/api/sql.ts | 4 ++-- src/routes/api/sync.ts | 4 ++-- src/routes/assets.ts | 2 +- src/routes/routes.ts | 2 +- 5 files changed, 11 insertions(+), 8 deletions(-) diff --git a/src/routes/api/script.ts b/src/routes/api/script.ts index 6907a5d9c..a289dc565 100644 --- a/src/routes/api/script.ts +++ b/src/routes/api/script.ts @@ -33,8 +33,11 @@ async function exec(req: Request) { executionResult: result, maxEntityChangeId: syncService.getMaxEntityChangeId() }; - } catch (e: any) { - return { success: false, error: e.message }; + } catch (e: unknown) { + return { + success: false, + error: (e instanceof Error) ? e.message : "Unknown Error" + }; } } diff --git a/src/routes/api/sql.ts b/src/routes/api/sql.ts index ed440f42d..a9876b657 100644 --- a/src/routes/api/sql.ts +++ b/src/routes/api/sql.ts @@ -56,10 +56,10 @@ function execute(req: Request) { success: true, results }; - } catch (e: any) { + } catch (e: unknown) { return { success: false, - error: e.message + error: (e instanceof Error) ? e.message : "Unknown Error" }; } } diff --git a/src/routes/api/sync.ts b/src/routes/api/sync.ts index 736e1e97b..8e186f92a 100644 --- a/src/routes/api/sync.ts +++ b/src/routes/api/sync.ts @@ -30,10 +30,10 @@ async function testSync() { syncService.sync(); return { success: true, message: t("test_sync.successful") }; - } catch (e: any) { + } catch (e: unknown) { return { success: false, - message: e.message + error: (e instanceof Error) ? e.message : "Unknown Error" }; } } diff --git a/src/routes/assets.ts b/src/routes/assets.ts index 37585d8d7..a26c22685 100644 --- a/src/routes/assets.ts +++ b/src/routes/assets.ts @@ -5,7 +5,7 @@ import express from "express"; import { isDev, isElectron } from "../services/utils.js"; import type serveStatic from "serve-static"; -const persistentCacheStatic = (root: string, options?: serveStatic.ServeStaticOptions>>) => { +const persistentCacheStatic = (root: string, options?: serveStatic.ServeStaticOptions>>) => { if (!isDev) { options = { maxAge: "1y", diff --git a/src/routes/routes.ts b/src/routes/routes.ts index ad1850aca..227b10d3c 100644 --- a/src/routes/routes.ts +++ b/src/routes/routes.ts @@ -477,7 +477,7 @@ function route(method: HttpMethod, path: string, middleware: express.Handler[], if (result?.then) { // promise - result.then((promiseResult: unknown) => handleResponse(resultHandler, req, res, promiseResult, start)).catch((e: any) => handleException(e, method, path, res)); + result.then((promiseResult: unknown) => handleResponse(resultHandler, req, res, promiseResult, start)).catch((e: unknown) => handleException(e, method, path, res)); } else { handleResponse(resultHandler, req, res, result, start); } From 7bd9be7b299d8298c7d014d6cf77fe559aa35e22 Mon Sep 17 00:00:00 2001 From: Panagiotis Papadopoulos Date: Sat, 8 Mar 2025 16:11:58 +0100 Subject: [PATCH 20/22] chore: use more narrow NoteType for RevisionRow --- src/becca/entities/brevision.ts | 4 ++-- src/becca/entities/rows.ts | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/becca/entities/brevision.ts b/src/becca/entities/brevision.ts index 6167ad4b2..bc9ce360c 100644 --- a/src/becca/entities/brevision.ts +++ b/src/becca/entities/brevision.ts @@ -7,7 +7,7 @@ import becca from "../becca.js"; import AbstractBeccaEntity from "./abstract_becca_entity.js"; import sql from "../../services/sql.js"; import BAttachment from "./battachment.js"; -import type { AttachmentRow, RevisionRow } from "./rows.js"; +import type { AttachmentRow, NoteType, RevisionRow } from "./rows.js"; import eraseService from "../../services/erase.js"; interface ContentOpts { @@ -36,7 +36,7 @@ class BRevision extends AbstractBeccaEntity { revisionId?: string; noteId!: string; - type!: string; + type!: NoteType; mime!: string; title!: string; dateLastEdited?: string; diff --git a/src/becca/entities/rows.ts b/src/becca/entities/rows.ts index b57801e1e..2228092ba 100644 --- a/src/becca/entities/rows.ts +++ b/src/becca/entities/rows.ts @@ -22,7 +22,7 @@ export interface AttachmentRow { export interface RevisionRow { revisionId?: string; noteId: string; - type: string; + type: NoteType; mime: string; isProtected?: boolean; title: string; From e20b662ea7af167d8dafa30f41c5d53a1c701f31 Mon Sep 17 00:00:00 2001 From: Panagiotis Papadopoulos Date: Sat, 8 Mar 2025 16:12:37 +0100 Subject: [PATCH 21/22] chore(routes): fix no-explicit-any lint/ts error for restoreRevision --- src/routes/api/revisions.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/routes/api/revisions.ts b/src/routes/api/revisions.ts index aeb6f32d2..18fbb7c39 100644 --- a/src/routes/api/revisions.ts +++ b/src/routes/api/revisions.ts @@ -144,7 +144,7 @@ function restoreRevision(req: Request) { note.title = revision.title; note.mime = revision.mime; - note.type = revision.type as any; + note.type = revision.type; note.setContent(revisionContent, { forceSave: true }); }); } From ecf1a0e4ad8234537d631128c06dd404c33ef365 Mon Sep 17 00:00:00 2001 From: Panagiotis Papadopoulos Date: Sat, 8 Mar 2025 17:07:25 +0100 Subject: [PATCH 22/22] 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,