Merge pull request #1347 from TriliumNext/chore_eslint-fixes_src-routes

chore(lint): fix eslint issues in `src/routes`

src/becca/entities/bnote.ts

@@ -1618,7 +1618,7 @@ class BNote extends AbstractBeccaEntity<BNote> {
      * @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'.`);
         }

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<BRevision> {
 
     revisionId?: string;
     noteId!: string;
-    type!: string;
+    type!: NoteType;
     mime!: string;
     title!: string;
     dateLastEdited?: string;

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;

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;

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;

src/errors/not_found_error.ts

@@ -1,9 +1,12 @@
-class NotFoundError {
+import HttpError from "./http_error.js";
-    message: string;
+
+class NotFoundError extends HttpError {
 
     constructor(message: string) {
-        this.message = message;
+        super(message, 404);
+        this.name = "NotFoundError";
     }
+
 }
 
 export default NotFoundError;

src/errors/validation_error.ts

@@ -1,9 +1,12 @@
-class ValidationError {
+import HttpError from "./http_error.js";
-    message: string;
+
+class ValidationError extends HttpError {
 
     constructor(message: string) {
-        this.message = message;
+        super(message, 400)
+        this.name = "ValidationError";
     }
+
 }
 
 export default ValidationError;

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);
@@ -41,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;

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()) {
+    const clipType = htmlSanitizer.sanitize(req.body.clipType);
-        title = `Clipped note from ${pageUrl}`;
+    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) {
@@ -123,8 +122,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");
         }
@@ -139,7 +136,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() ? "<br/>" : ""}${rewrittenContent}`;
@@ -219,9 +216,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
     };

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;
@@ -37,11 +38,12 @@ function exportBranch(req: Request, res: Response) {
         } else {
             throw new NotFoundError(`Unrecognized export format '${format}'`);
         }
-    } catch (e: any) {
+    } catch (e: unknown) {
-        const message = `Export failed with following error: '${e.message}'. 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(message + e.stack);
+        log.error(errMessage + errStack);
 
         res.setHeader("Content-Type", "text/plain").status(500).send(message);
     }

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 {

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;
@@ -68,11 +69,12 @@ async function importNotesToBranch(req: Request) {
         } else {
             note = await singleImportService.importSingleFile(taskContext, file, parentNote);
         }
-    } catch (e: any) {
+    } catch (e: unknown) {
-        const message = `Import failed with following error: '${e.message}'. More details might be in the logs.`;
+        const [errMessage, errStack] = safeExtractMessageAndStackFromError(e);
+        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 +122,13 @@ async function importAttachmentsToNote(req: Request) {
 
     try {
         await singleImportService.importAttachment(taskContext, file, parentNote);
-    } catch (e: any) {
+    } catch (e: unknown) {
-        const message = `Import failed with following error: '${e.message}'. More details might be in the logs.`;
+        const [errMessage, errStack] = safeExtractMessageAndStackFromError(e);
+
+        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];
     }

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;

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);
 

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";
@@ -111,7 +110,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();
     });
@@ -145,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 });
         });
     }

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;
@@ -33,8 +34,12 @@ async function exec(req: Request) {
             executionResult: result,
             maxEntityChangeId: syncService.getMaxEntityChangeId()
         };
-    } catch (e: any) {
+    } catch (e: unknown) {
-        return { success: false, error: e.message };
+        const [errMessage] = safeExtractMessageAndStackFromError(e);
+        return {
+            success: false,
+            error: errMessage
+        };
     }
 }
 

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);
 }

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`);
@@ -56,10 +57,11 @@ function execute(req: Request) {
             success: true,
             results
         };
-    } catch (e: any) {
+    } catch (e: unknown) {
+        const [errMessage] = safeExtractMessageAndStackFromError(e);
         return {
             success: false,
-            error: e.message
+            error: errMessage
         };
     }
 }

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";
@@ -30,10 +30,11 @@ async function testSync() {
         syncService.sync();
 
         return { success: true, message: t("test_sync.successful") };
-    } catch (e: any) {
+    } catch (e: unknown) {
+        const [errMessage] = safeExtractMessageAndStackFromError(e);
         return {
             success: false,
-            message: e.message
+            error: errMessage
         };
     }
 }

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";

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<express.Response<any, Record<string, any>>>) => {
+const persistentCacheStatic = (root: string, options?: serveStatic.ServeStaticOptions<express.Response<unknown, Record<string, unknown>>>) => {
     if (!isDev) {
         options = {
             maxAge: "1y",

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
@@ -25,8 +26,9 @@ function handleRequest(req: Request, res: Response) {
 
         try {
             match = path.match(regex);
-        } catch (e: any) {
+        } catch (e: unknown) {
-            log.error(`Testing path for label '${attr.attributeId}', regex '${attr.value}' failed with error: ${e.message}, stack: ${e.stack}`);
+            const [errMessage, errStack] = safeExtractMessageAndStackFromError(e);
+            log.error(`Testing path for label '${attr.attributeId}', regex '${attr.value}' failed with error: ${errMessage}, stack: ${errStack}`);
             continue;
         }
 
@@ -45,10 +47,10 @@ function handleRequest(req: Request, res: Response) {
                     req,
                     res
                 });
-            } catch (e: any) {
+            } catch (e: unknown) {
-                log.error(`Custom handler '${note.noteId}' failed with: ${e.message}, ${e.stack}`);
+                const [errMessage, errStack] = safeExtractMessageAndStackFromError(e);
-
+                log.error(`Custom handler '${note.noteId}' failed with: ${errMessage}, ${errStack}`);
-                res.setHeader("Content-Type", "text/plain").status(500).send(e.message);
+                res.setHeader("Content-Type", "text/plain").status(500).send(errMessage);
             }
         } else if (attr.name === "customResourceProvider") {
             fileService.downloadNoteInt(attr.noteId, res);
@@ -68,7 +70,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);
 

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) {

src/routes/error_handlers.ts

@@ -1,38 +1,46 @@
 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) => {
+
-        if (err.code !== "EBADCSRFTOKEN") {
+    app.use((err: unknown | Error, req: Request, res: Response, next: NextFunction) => {
-            return next(err);
+
+        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"]}`);
+        return next(err);
-
-        err = new Error("Invalid CSRF token");
-        err.status = 403;
-        next(err);
     });
 
     // 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}`);
+        const err = new NotFoundError(`Router not found for request ${req.method} ${req.url}`);
-        (err as any).status = 404;
         next(err);
     });
 
     // error handler
-    app.use((err: any, req: Request, res: Response, next: NextFunction) => {
+    app.use((err: unknown | Error, req: Request, res: Response, _next: NextFunction) => {
-        if (err.status !== 404) {
-            log.info(err);
-        } else {
-            log.info(`${err.status} ${req.method} ${req.url}`);
-        }
 
-        res.status(err.status || 500);
+        const statusCode = (err instanceof HttpError) ? err.statusCode : 500;
-        res.send({
+        const errMessage = (err instanceof Error && statusCode !== 404)
-            message: err.message
+            ? err
+            : `${statusCode} ${req.method} ${req.url}`;
+
+        log.info(errMessage);
+
+        res.status(statusCode).send({
+            message: err instanceof Error ? err.message : "Unknown Error"
         });
+
     });
 }
 

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";
@@ -471,7 +471,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);
             }
@@ -487,22 +487,17 @@ 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) {
+function handleException(e: unknown | Error, method: HttpMethod, path: string, res: express.Response) {
-    log.error(`${method} ${path} threw exception: '${e.message}', stack: ${e.stack}`);
+    const [errMessage, errStack] = safeExtractMessageAndStackFromError(e);
+
+    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() {

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<typeof utils.formatDownloadTitle>, expectedValue: ReturnType<typeof utils.formatDownloadTitle>][] = [

src/services/utils.ts

@@ -362,6 +362,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,
@@ -392,6 +397,7 @@ export default {
     removeDiacritic,
     removeTextFileExtension,
     replaceAll,
+    safeExtractMessageAndStackFromError,
     sanitizeSqlIdentifier,
     stripTags,
     timeLimit,