From 6bde49a60667160d8e8fd9d8ccb05caedeb034f5 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Thu, 26 Dec 2024 12:15:06 +0100 Subject: [PATCH] Reduce duplication when handling "DocException" and "PasswordRequest" messages Rather than having to manually implement the exception-handling for the "DocException" message, we can instead re-use (and slightly extend) the existing `wrapReason` function since that one already does what we need. Furthermore, we can also simplify handling of the "PasswordRequest" message a little bit and again re-use the `wrapReason` function. Finally, the patch makes the following smaller changes: - Avoid needlessly re-creating exceptions in the `wrapReason` function. - Use a slightly shorter parameter name in the `wrapReason` function. - Remove the unused entries in the `CallbackKind`/`StreamKind` enumerations. --- src/core/worker.js | 19 +++---------- src/display/api.js | 51 +++++++++-------------------------- src/shared/message_handler.js | 39 ++++++++++++++++----------- 3 files changed, 38 insertions(+), 71 deletions(-) diff --git a/src/core/worker.js b/src/core/worker.js index 7ae22ee45afdb..f1ab5b57c6d24 100644 --- a/src/core/worker.js +++ b/src/core/worker.js @@ -18,14 +18,10 @@ import { assert, getVerbosityLevel, info, - InvalidPDFException, isNodeJS, - MissingPDFException, PasswordException, setVerbosityLevel, stringToPDFString, - UnexpectedResponseException, - UnknownErrorException, VerbosityLevel, warn, } from "../shared/util.js"; @@ -36,10 +32,10 @@ import { } from "./core_utils.js"; import { Dict, isDict, Ref, RefSetCache } from "./primitives.js"; import { LocalPdfManager, NetworkPdfManager } from "./pdf_manager.js"; +import { MessageHandler, wrapReason } from "../shared/message_handler.js"; import { AnnotationFactory } from "./annotation.js"; import { clearGlobalCaches } from "./cleanup_helper.js"; import { incrementalUpdate } from "./writer.js"; -import { MessageHandler } from "../shared/message_handler.js"; import { PDFWorkerStream } from "./worker_stream.js"; import { StructTreeRoot } from "./struct_tree.js"; @@ -347,18 +343,9 @@ class WorkerMessageHandler { finishWorkerTask(task); handler.send("DocException", ex); }); - } else if ( - ex instanceof InvalidPDFException || - ex instanceof MissingPDFException || - ex instanceof UnexpectedResponseException || - ex instanceof UnknownErrorException - ) { - handler.send("DocException", ex); } else { - handler.send( - "DocException", - new UnknownErrorException(ex.message, ex.toString()) - ); + // Ensure that we always fallback to `UnknownErrorException`. + handler.send("DocException", wrapReason(ex)); } } diff --git a/src/display/api.js b/src/display/api.js index 1594d7cb2bfd1..04d0d8f88dc2b 100644 --- a/src/display/api.js +++ b/src/display/api.js @@ -24,17 +24,12 @@ import { FeatureTest, getVerbosityLevel, info, - InvalidPDFException, isNodeJS, MAX_IMAGE_SIZE_TO_CACHE, - MissingPDFException, - PasswordException, RenderingIntentFlag, setVerbosityLevel, shadow, stringToBytes, - UnexpectedResponseException, - UnknownErrorException, unreachable, warn, } from "../shared/util.js"; @@ -51,6 +46,7 @@ import { RenderingCancelledException, StatTimer, } from "./display_utils.js"; +import { MessageHandler, wrapReason } from "../shared/message_handler.js"; import { NodeCanvasFactory, NodeCMapReaderFactory, @@ -63,7 +59,6 @@ import { DOMCMapReaderFactory } from "display-cmap_reader_factory"; import { DOMFilterFactory } from "./filter_factory.js"; import { DOMStandardFontDataFactory } from "display-standard_fontdata_factory"; import { GlobalWorkerOptions } from "./worker_options.js"; -import { MessageHandler } from "../shared/message_handler.js"; import { Metadata } from "./metadata.js"; import { OptionalContentConfig } from "./optional_content_config.js"; import { PDFDataTransportStream } from "./transport_stream.js"; @@ -2731,34 +2726,18 @@ class WorkerTransport { loadingTask._capability.resolve(new PDFDocumentProxy(pdfInfo, this)); }); - messageHandler.on("DocException", function (ex) { - let reason; - switch (ex.name) { - case "PasswordException": - reason = new PasswordException(ex.message, ex.code); - break; - case "InvalidPDFException": - reason = new InvalidPDFException(ex.message); - break; - case "MissingPDFException": - reason = new MissingPDFException(ex.message); - break; - case "UnexpectedResponseException": - reason = new UnexpectedResponseException(ex.message, ex.status); - break; - case "UnknownErrorException": - reason = new UnknownErrorException(ex.message, ex.details); - break; - default: - unreachable("DocException - expected a valid Error."); - } - loadingTask._capability.reject(reason); + messageHandler.on("DocException", ex => { + loadingTask._capability.reject(wrapReason(ex)); }); - messageHandler.on("PasswordRequest", exception => { + messageHandler.on("PasswordRequest", ex => { this.#passwordCapability = Promise.withResolvers(); - if (loadingTask.onPassword) { + try { + if (!loadingTask.onPassword) { + throw wrapReason(ex); + } + const updatePassword = password => { if (password instanceof Error) { this.#passwordCapability.reject(password); @@ -2766,15 +2745,9 @@ class WorkerTransport { this.#passwordCapability.resolve({ password }); } }; - try { - loadingTask.onPassword(updatePassword, exception.code); - } catch (ex) { - this.#passwordCapability.reject(ex); - } - } else { - this.#passwordCapability.reject( - new PasswordException(exception.message, exception.code) - ); + loadingTask.onPassword(updatePassword, ex.code); + } catch (err) { + this.#passwordCapability.reject(err); } return this.#passwordCapability.promise; }); diff --git a/src/shared/message_handler.js b/src/shared/message_handler.js index 0ed90ee5e8435..9ea0e5f79e030 100644 --- a/src/shared/message_handler.js +++ b/src/shared/message_handler.js @@ -16,6 +16,7 @@ import { AbortException, assert, + InvalidPDFException, MissingPDFException, PasswordException, UnexpectedResponseException, @@ -24,13 +25,11 @@ import { } from "./util.js"; const CallbackKind = { - UNKNOWN: 0, DATA: 1, ERROR: 2, }; const StreamKind = { - UNKNOWN: 0, CANCEL: 1, CANCEL_COMPLETE: 2, CLOSE: 3, @@ -43,31 +42,39 @@ const StreamKind = { function onFn() {} -function wrapReason(reason) { +function wrapReason(ex) { if ( - !( - reason instanceof Error || - (typeof reason === "object" && reason !== null) - ) + ex instanceof AbortException || + ex instanceof InvalidPDFException || + ex instanceof MissingPDFException || + ex instanceof PasswordException || + ex instanceof UnexpectedResponseException || + ex instanceof UnknownErrorException ) { + // Avoid re-creating the exception when its type is already correct. + return ex; + } + + if (!(ex instanceof Error || (typeof ex === "object" && ex !== null))) { unreachable( 'wrapReason: Expected "reason" to be a (possibly cloned) Error.' ); } - switch (reason.name) { + switch (ex.name) { case "AbortException": - return new AbortException(reason.message); + return new AbortException(ex.message); + case "InvalidPDFException": + return new InvalidPDFException(ex.message); case "MissingPDFException": - return new MissingPDFException(reason.message); + return new MissingPDFException(ex.message); case "PasswordException": - return new PasswordException(reason.message, reason.code); + return new PasswordException(ex.message, ex.code); case "UnexpectedResponseException": - return new UnexpectedResponseException(reason.message, reason.status); + return new UnexpectedResponseException(ex.message, ex.status); case "UnknownErrorException": - return new UnknownErrorException(reason.message, reason.details); - default: - return new UnknownErrorException(reason.message, reason.toString()); + return new UnknownErrorException(ex.message, ex.details); } + return new UnknownErrorException(ex.message, ex.toString()); } class MessageHandler { @@ -532,4 +539,4 @@ class MessageHandler { } } -export { MessageHandler }; +export { MessageHandler, wrapReason };