From bdc829198cc8cd0c7cabcd3e4cd5b408e16243a4 Mon Sep 17 00:00:00 2001 From: Panagiotis Papadopoulos Date: Mon, 10 Mar 2025 07:15:43 +0100 Subject: [PATCH 1/8] chore(share): use safeExtractMessageAndStackFromError to get rid of "any" in try/catch blocks --- src/share/routes.spec.ts | 6 ++++-- src/share/routes.ts | 7 ++++--- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/src/share/routes.spec.ts b/src/share/routes.spec.ts index 86e1927bdd..5072bb2970 100644 --- a/src/share/routes.spec.ts +++ b/src/share/routes.spec.ts @@ -2,6 +2,7 @@ import { beforeAll, beforeEach, describe, expect, it } from "vitest"; import supertest from "supertest"; import { initializeTranslations } from "../services/i18n.js"; import type { Application, Request, Response, NextFunction } from "express"; +import { safeExtractMessageAndStackFromError } from "../services/utils.js"; let app: Application; @@ -11,8 +12,9 @@ describe("Share API test", () => { beforeAll(async () => { initializeTranslations(); app = (await import("../app.js")).default; - app.use((err: any, req: Request, res: Response, next: NextFunction) => { - if (err.message.includes("Cannot set headers after they are sent to the client")) { + app.use((err: unknown, req: Request, res: Response, next: NextFunction) => { + const [errMessage] = safeExtractMessageAndStackFromError(err) + if (errMessage.includes("Cannot set headers after they are sent to the client")) { cannotSetHeadersCount++; } diff --git a/src/share/routes.ts b/src/share/routes.ts index 3c6db87d54..db7e62f40b 100644 --- a/src/share/routes.ts +++ b/src/share/routes.ts @@ -15,7 +15,7 @@ import log from "../services/log.js"; import type SNote from "./shaca/entities/snote.js"; import type SBranch from "./shaca/entities/sbranch.js"; import type SAttachment from "./shaca/entities/sattachment.js"; -import utils from "../services/utils.js"; +import utils, { safeExtractMessageAndStackFromError } from "../services/utils.js"; import options from "../services/options.js"; function getSharedSubTreeRoot(note: SNote): { note?: SNote; branch?: SBranch } { @@ -183,8 +183,9 @@ function register(router: Router) { res.send(ejsResult); useDefaultView = false; // Rendering went okay, don't use default view } - } catch (e: any) { - log.error(`Rendering user provided share template (${templateId}) threw exception ${e.message} with stacktrace: ${e.stack}`); + } catch (e: unknown) { + const [errMessage, errStack] = safeExtractMessageAndStackFromError(e); + log.error(`Rendering user provided share template (${templateId}) threw exception ${errMessage} with stacktrace: ${errStack}`); } } } From c2b75a642186720906d1bae5154602416f458b41 Mon Sep 17 00:00:00 2001 From: Panagiotis Papadopoulos Date: Mon, 10 Mar 2025 07:33:20 +0100 Subject: [PATCH 2/8] chore(share): fix @typescript-eslint/no-unused-vars for "next" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit alternative solution, since they are unused and it is the last argument → remove it. We can still go that route later on though, if we agree upon it. --- src/share/routes.ts | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/share/routes.ts b/src/share/routes.ts index db7e62f40b..90018ddabe 100644 --- a/src/share/routes.ts +++ b/src/share/routes.ts @@ -195,7 +195,7 @@ function register(router: Router) { } } - router.get("/share/", (req, res, next) => { + router.get("/share/", (req, res, _next) => { if (req.path.substr(-1) !== "/") { res.redirect("../share/"); return; @@ -211,7 +211,7 @@ function register(router: Router) { renderNote(shaca.shareRootNote, req, res); }); - router.get("/share/:shareId", (req, res, next) => { + router.get("/share/:shareId", (req, res, _next) => { shacaLoader.ensureLoad(); const { shareId } = req.params; @@ -221,7 +221,7 @@ function register(router: Router) { renderNote(note, req, res); }); - router.get("/share/api/notes/:noteId", (req, res, next) => { + router.get("/share/api/notes/:noteId", (req, res, _next) => { shacaLoader.ensureLoad(); let note: SNote | boolean; @@ -234,7 +234,7 @@ function register(router: Router) { res.json(note.getPojo()); }); - router.get("/share/api/notes/:noteId/download", (req, res, next) => { + router.get("/share/api/notes/:noteId/download", (req, res, _next) => { shacaLoader.ensureLoad(); let note: SNote | boolean; @@ -256,7 +256,7 @@ function register(router: Router) { }); // :filename is not used by trilium, but instead used for "save as" to assign a human-readable filename - router.get("/share/api/images/:noteId/:filename", (req, res, next) => { + router.get("/share/api/images/:noteId/:filename", (req, res, _next) => { shacaLoader.ensureLoad(); let image: SNote | boolean; @@ -282,7 +282,7 @@ function register(router: Router) { }); // :filename is not used by trilium, but instead used for "save as" to assign a human-readable filename - router.get("/share/api/attachments/:attachmentId/image/:filename", (req, res, next) => { + router.get("/share/api/attachments/:attachmentId/image/:filename", (req, res, _next) => { shacaLoader.ensureLoad(); let attachment: SAttachment | boolean; @@ -300,7 +300,7 @@ function register(router: Router) { } }); - router.get("/share/api/attachments/:attachmentId/download", (req, res, next) => { + router.get("/share/api/attachments/:attachmentId/download", (req, res, _next) => { shacaLoader.ensureLoad(); let attachment: SAttachment | boolean; @@ -322,7 +322,7 @@ function register(router: Router) { }); // used for PDF viewing - router.get("/share/api/notes/:noteId/view", (req, res, next) => { + router.get("/share/api/notes/:noteId/view", (req, res, _next) => { shacaLoader.ensureLoad(); let note: SNote | boolean; @@ -340,7 +340,7 @@ function register(router: Router) { }); // Used for searching, require noteId so we know the subTreeRoot - router.get("/share/api/notes", (req, res, next) => { + router.get("/share/api/notes", (req, res, _next) => { shacaLoader.ensureLoad(); const ancestorNoteId = req.query.ancestorNoteId ?? "_share"; From cd9d90323ccedc7fbdd866eff50da23d34b8768d Mon Sep 17 00:00:00 2001 From: Panagiotis Papadopoulos Date: Mon, 10 Mar 2025 07:34:43 +0100 Subject: [PATCH 3/8] chore(share): fix @typescript-eslint/no-unused-vars for unused note variable there's no need to assign a variable, if we never use the value outside of the if check --- src/share/routes.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/share/routes.ts b/src/share/routes.ts index 90018ddabe..932adfaacd 100644 --- a/src/share/routes.ts +++ b/src/share/routes.ts @@ -344,7 +344,6 @@ function register(router: Router) { shacaLoader.ensureLoad(); const ancestorNoteId = req.query.ancestorNoteId ?? "_share"; - let note; if (typeof ancestorNoteId !== "string") { res.status(400).json({ message: "'ancestorNoteId' parameter is mandatory." }); @@ -352,7 +351,7 @@ function register(router: Router) { } // This will automatically return if no ancestorNoteId is provided and there is no shareIndex - if (!(note = checkNoteAccess(ancestorNoteId, req, res))) { + if (!checkNoteAccess(ancestorNoteId, req, res)) { return; } From c2aae454562a4b4f7313530bef14d9f93db2a7b3 Mon Sep 17 00:00:00 2001 From: Panagiotis Papadopoulos Date: Mon, 10 Mar 2025 08:10:44 +0100 Subject: [PATCH 4/8] chore(share): fix no-unused-vars and prefer-const lint issues --- src/share/shaca/entities/sattachment.ts | 2 +- src/share/shaca/entities/snote.ts | 4 ++-- src/share/shaca/shaca_loader.ts | 2 +- 3 files changed, 4 insertions(+), 4 deletions(-) diff --git a/src/share/shaca/entities/sattachment.ts b/src/share/shaca/entities/sattachment.ts index 612ba81f34..e8d1e50d99 100644 --- a/src/share/shaca/entities/sattachment.ts +++ b/src/share/shaca/entities/sattachment.ts @@ -47,7 +47,7 @@ class SAttachment extends AbstractShacaEntity { } } - let content = row.content; + const content = row.content; if (this.hasStringContent()) { return content === null ? "" : content.toString("utf-8"); diff --git a/src/share/shaca/entities/snote.ts b/src/share/shaca/entities/snote.ts index b9e1a8b234..e4ba17d0c1 100644 --- a/src/share/shaca/entities/snote.ts +++ b/src/share/shaca/entities/snote.ts @@ -105,7 +105,7 @@ class SNote extends AbstractShacaEntity { } } - let content = row.content; + const content = row.content; if (this.hasStringContent()) { return content === null ? "" : content.toString("utf-8"); @@ -212,7 +212,7 @@ class SNote extends AbstractShacaEntity { /** * @throws Error in case of invalid JSON */ - getJsonContent(): any | null { + getJsonContent(): unknown | null { const content = this.getContent(); if (typeof content !== "string" || !content || !content.trim()) { diff --git a/src/share/shaca/shaca_loader.ts b/src/share/shaca/shaca_loader.ts index 474f5be37a..93783e7a0c 100644 --- a/src/share/shaca/shaca_loader.ts +++ b/src/share/shaca/shaca_loader.ts @@ -93,7 +93,7 @@ function ensureLoad() { eventService.subscribe( [eventService.ENTITY_CREATED, eventService.ENTITY_CHANGED, eventService.ENTITY_DELETED, eventService.ENTITY_CHANGE_SYNCED, eventService.ENTITY_DELETE_SYNCED], - ({ entityName, entity }) => { + ({ _entityName, _entity }) => { shaca.reset(); } ); From 2a5ac80c05928971cc5e2facd727920c5133d7e5 Mon Sep 17 00:00:00 2001 From: Panagiotis Papadopoulos Date: Mon, 10 Mar 2025 08:12:07 +0100 Subject: [PATCH 5/8] chore(utils/safeExtractMessageAndStackFromError): add explicit return type to have it as a named tuple --- src/services/utils.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/services/utils.ts b/src/services/utils.ts index 3cb84d3a19..dfaffec98e 100644 --- a/src/services/utils.ts +++ b/src/services/utils.ts @@ -363,7 +363,7 @@ export function processStringOrBuffer(data: string | Buffer | null) { } } -export function safeExtractMessageAndStackFromError(err: unknown) { +export function safeExtractMessageAndStackFromError(err: unknown): [errMessage: string, errStack: string | undefined] { return (err instanceof Error) ? [err.message, err.stack] as const : ["Unknown Error", undefined] as const; } From a5a66a12e2ce7d6efe8d1908e3631849c29ccba7 Mon Sep 17 00:00:00 2001 From: Panagiotis Papadopoulos Date: Mon, 10 Mar 2025 19:30:32 +0100 Subject: [PATCH 6/8] chore(share): fix tsc nagging about svg not existing on unknown JSON and TS without using a validation library like zod, is really a bit of a pain in the backside... --- src/share/routes.ts | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/src/share/routes.ts b/src/share/routes.ts index 932adfaacd..a2dbb6e0dd 100644 --- a/src/share/routes.ts +++ b/src/share/routes.ts @@ -115,7 +115,14 @@ function renderImageAttachment(image: SNote, res: Response, attachmentName: stri svgString = content; } else { // backwards compatibility, before attachments, the SVG was stored in the main note content as a separate key - const contentSvg = image.getJsonContentSafely()?.svg; + const possibleSvgContent = image.getJsonContentSafely(); + + const contentSvg = (typeof possibleSvgContent === "object" + && possibleSvgContent !== null + && "svg" in possibleSvgContent + && typeof possibleSvgContent.svg === "string") + ? possibleSvgContent.svg + : null; if (contentSvg) { svgString = contentSvg; From ae1ef555224663f964dc8ee1f32502aa88471b46 Mon Sep 17 00:00:00 2001 From: Panagiotis Papadopoulos Date: Sat, 15 Mar 2025 12:27:02 +0100 Subject: [PATCH 7/8] chore(share): remove unused "_next" addresses https://github.com/TriliumNext/Notes/pull/1384#discussion_r1989044764 --- src/share/routes.ts | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/src/share/routes.ts b/src/share/routes.ts index a2dbb6e0dd..03de585a8f 100644 --- a/src/share/routes.ts +++ b/src/share/routes.ts @@ -202,7 +202,7 @@ function register(router: Router) { } } - router.get("/share/", (req, res, _next) => { + router.get("/share/", (req, res) => { if (req.path.substr(-1) !== "/") { res.redirect("../share/"); return; @@ -218,7 +218,7 @@ function register(router: Router) { renderNote(shaca.shareRootNote, req, res); }); - router.get("/share/:shareId", (req, res, _next) => { + router.get("/share/:shareId", (req, res) => { shacaLoader.ensureLoad(); const { shareId } = req.params; @@ -228,7 +228,7 @@ function register(router: Router) { renderNote(note, req, res); }); - router.get("/share/api/notes/:noteId", (req, res, _next) => { + router.get("/share/api/notes/:noteId", (req, res) => { shacaLoader.ensureLoad(); let note: SNote | boolean; @@ -241,7 +241,7 @@ function register(router: Router) { res.json(note.getPojo()); }); - router.get("/share/api/notes/:noteId/download", (req, res, _next) => { + router.get("/share/api/notes/:noteId/download", (req, res) => { shacaLoader.ensureLoad(); let note: SNote | boolean; @@ -263,7 +263,7 @@ function register(router: Router) { }); // :filename is not used by trilium, but instead used for "save as" to assign a human-readable filename - router.get("/share/api/images/:noteId/:filename", (req, res, _next) => { + router.get("/share/api/images/:noteId/:filename", (req, res) => { shacaLoader.ensureLoad(); let image: SNote | boolean; @@ -289,7 +289,7 @@ function register(router: Router) { }); // :filename is not used by trilium, but instead used for "save as" to assign a human-readable filename - router.get("/share/api/attachments/:attachmentId/image/:filename", (req, res, _next) => { + router.get("/share/api/attachments/:attachmentId/image/:filename", (req, res) => { shacaLoader.ensureLoad(); let attachment: SAttachment | boolean; @@ -307,7 +307,7 @@ function register(router: Router) { } }); - router.get("/share/api/attachments/:attachmentId/download", (req, res, _next) => { + router.get("/share/api/attachments/:attachmentId/download", (req, res) => { shacaLoader.ensureLoad(); let attachment: SAttachment | boolean; @@ -329,7 +329,7 @@ function register(router: Router) { }); // used for PDF viewing - router.get("/share/api/notes/:noteId/view", (req, res, _next) => { + router.get("/share/api/notes/:noteId/view", (req, res) => { shacaLoader.ensureLoad(); let note: SNote | boolean; @@ -347,7 +347,7 @@ function register(router: Router) { }); // Used for searching, require noteId so we know the subTreeRoot - router.get("/share/api/notes", (req, res, _next) => { + router.get("/share/api/notes", (req, res) => { shacaLoader.ensureLoad(); const ancestorNoteId = req.query.ancestorNoteId ?? "_share"; From 73305a5327071cfe85eaad9f6aa9b4828b0729da Mon Sep 17 00:00:00 2001 From: Panagiotis Papadopoulos Date: Sat, 15 Mar 2025 12:31:33 +0100 Subject: [PATCH 8/8] chore(share): remove unused args addresses https://github.com/TriliumNext/Notes/pull/1384#discussion_r1989045491 --- src/share/shaca/shaca_loader.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/share/shaca/shaca_loader.ts b/src/share/shaca/shaca_loader.ts index 93783e7a0c..c0834cb8b1 100644 --- a/src/share/shaca/shaca_loader.ts +++ b/src/share/shaca/shaca_loader.ts @@ -93,7 +93,7 @@ function ensureLoad() { eventService.subscribe( [eventService.ENTITY_CREATED, eventService.ENTITY_CHANGED, eventService.ENTITY_DELETED, eventService.ENTITY_CHANGE_SYNCED, eventService.ENTITY_DELETE_SYNCED], - ({ _entityName, _entity }) => { + () => { shaca.reset(); } );