From 6167566f1bbb424aa2a2772d1dd953aafe615a03 Mon Sep 17 00:00:00 2001 From: Jonas Jenwald Date: Mon, 9 Aug 2021 12:02:49 +0200 Subject: [PATCH] Re-factor the `BaseException.name` handling, and clean-up some code Once we're finally able to get rid of SystemJS, which is unfortunately still blocked on [bug 1247687](https://bugzilla.mozilla.org/show_bug.cgi?id=1247687), we might also want to clean-up (or even completely remove) the `BaseException` abstraction and simply extend `Error` directly instead. At that point we'd need to (explicitly) set the `name` on each class anyway, so this patch is essentially preparing for future clean-up. Furthermore, after the `BaseException` abstraction was added there's been *multiple* issues filed about third-party minification breaking our code since `this.constructor.name` is not guaranteed to always do what you intended. While hard-coding the strings indeed feels quite unfortunate, it's likely the "best" solution to avoid the problem described above. --- src/core/core_utils.js | 20 ++++++++++++++++---- src/core/jbig2.js | 2 +- src/core/jpg.js | 10 +++++++--- src/core/jpx.js | 2 +- src/display/api.js | 13 ++----------- src/display/display_utils.js | 2 +- src/shared/message_handler.js | 24 ++++++++++++++---------- src/shared/util.js | 34 +++++++++++++++++++++++++--------- 8 files changed, 67 insertions(+), 40 deletions(-) diff --git a/src/core/core_utils.js b/src/core/core_utils.js index 253a349bc..7282eb0e5 100644 --- a/src/core/core_utils.js +++ b/src/core/core_utils.js @@ -52,17 +52,29 @@ function getArrayLookupTableFactory(initializer) { class MissingDataException extends BaseException { constructor(begin, end) { - super(`Missing data [${begin}, ${end})`); + super(`Missing data [${begin}, ${end})`, "MissingDataException"); this.begin = begin; this.end = end; } } -class ParserEOFException extends BaseException {} +class ParserEOFException extends BaseException { + constructor(msg) { + super(msg, "ParserEOFException"); + } +} -class XRefEntryException extends BaseException {} +class XRefEntryException extends BaseException { + constructor(msg) { + super(msg, "XRefEntryException"); + } +} -class XRefParseException extends BaseException {} +class XRefParseException extends BaseException { + constructor(msg) { + super(msg, "XRefParseException"); + } +} /** * Get the value of an inheritable property. diff --git a/src/core/jbig2.js b/src/core/jbig2.js index dbc430858..72f9e8dad 100644 --- a/src/core/jbig2.js +++ b/src/core/jbig2.js @@ -20,7 +20,7 @@ import { CCITTFaxDecoder } from "./ccitt.js"; class Jbig2Error extends BaseException { constructor(msg) { - super(`JBIG2 error: ${msg}`); + super(`JBIG2 error: ${msg}`, "Jbig2Error"); } } diff --git a/src/core/jpg.js b/src/core/jpg.js index 22e2af9a5..5084181ca 100644 --- a/src/core/jpg.js +++ b/src/core/jpg.js @@ -18,18 +18,22 @@ import { readUint16 } from "./core_utils.js"; class JpegError extends BaseException { constructor(msg) { - super(`JPEG error: ${msg}`); + super(`JPEG error: ${msg}`, "JpegError"); } } class DNLMarkerError extends BaseException { constructor(message, scanLines) { - super(message); + super(message, "DNLMarkerError"); this.scanLines = scanLines; } } -class EOIMarkerError extends BaseException {} +class EOIMarkerError extends BaseException { + constructor(msg) { + super(msg, "EOIMarkerError"); + } +} /** * This code was forked from https://github.com/notmasteryet/jpgjs. diff --git a/src/core/jpx.js b/src/core/jpx.js index 63dc11e8e..99bcf3c45 100644 --- a/src/core/jpx.js +++ b/src/core/jpx.js @@ -19,7 +19,7 @@ import { ArithmeticDecoder } from "./arithmetic_decoder.js"; class JpxError extends BaseException { constructor(msg) { - super(`JPX error: ${msg}`); + super(`JPX error: ${msg}`, "JpxError"); } } diff --git a/src/display/api.js b/src/display/api.js index 978b05802..1fa6367e3 100644 --- a/src/display/api.js +++ b/src/display/api.js @@ -2565,17 +2565,8 @@ class WorkerTransport { case "UnknownErrorException": reason = new UnknownErrorException(ex.message, ex.details); break; - } - if (!(reason instanceof Error)) { - const msg = "DocException - expected a valid Error."; - if ( - typeof PDFJSDev === "undefined" || - PDFJSDev.test("!PRODUCTION || TESTING") - ) { - unreachable(msg); - } else { - warn(msg); - } + default: + unreachable("DocException - expected a valid Error."); } loadingTask._capability.reject(reason); }); diff --git a/src/display/display_utils.js b/src/display/display_utils.js index e4cc62085..c94834a74 100644 --- a/src/display/display_utils.js +++ b/src/display/display_utils.js @@ -300,7 +300,7 @@ class PageViewport { class RenderingCancelledException extends BaseException { constructor(msg, type) { - super(msg); + super(msg, "RenderingCancelledException"); this.type = type; } } diff --git a/src/shared/message_handler.js b/src/shared/message_handler.js index 9f65d0c0a..372427f8a 100644 --- a/src/shared/message_handler.js +++ b/src/shared/message_handler.js @@ -20,6 +20,7 @@ import { MissingPDFException, UnexpectedResponseException, UnknownErrorException, + warn, } from "./util.js"; const CallbackKind = { @@ -42,18 +43,21 @@ const StreamKind = { function wrapReason(reason) { if ( - typeof PDFJSDev === "undefined" || - PDFJSDev.test("!PRODUCTION || TESTING") - ) { - assert( + !( reason instanceof Error || - (typeof reason === "object" && reason !== null), - 'wrapReason: Expected "reason" to be a (possibly cloned) Error.' - ); - } else { - if (typeof reason !== "object" || reason === null) { - return reason; + (typeof reason === "object" && reason !== null) + ) + ) { + if ( + typeof PDFJSDev === "undefined" || + PDFJSDev.test("!PRODUCTION || TESTING") + ) { + throw new Error( + 'wrapReason: Expected "reason" to be a (possibly cloned) Error.' + ); } + warn('wrapReason: Expected "reason" to be a (possibly cloned) Error.'); + return reason; } switch (reason.name) { case "AbortException": diff --git a/src/shared/util.js b/src/shared/util.js index 92d0ae9a0..a696fe659 100644 --- a/src/shared/util.js +++ b/src/shared/util.js @@ -459,12 +459,12 @@ function shadow(obj, prop, value) { */ const BaseException = (function BaseExceptionClosure() { // eslint-disable-next-line no-shadow - function BaseException(message) { + function BaseException(message, name) { if (this.constructor === BaseException) { unreachable("Cannot initialize BaseException."); } this.message = message; - this.name = this.constructor.name; + this.name = name; } BaseException.prototype = new Error(); BaseException.constructor = BaseException; @@ -474,25 +474,33 @@ const BaseException = (function BaseExceptionClosure() { class PasswordException extends BaseException { constructor(msg, code) { - super(msg); + super(msg, "PasswordException"); this.code = code; } } class UnknownErrorException extends BaseException { constructor(msg, details) { - super(msg); + super(msg, "UnknownErrorException"); this.details = details; } } -class InvalidPDFException extends BaseException {} +class InvalidPDFException extends BaseException { + constructor(msg) { + super(msg, "InvalidPDFException"); + } +} -class MissingPDFException extends BaseException {} +class MissingPDFException extends BaseException { + constructor(msg) { + super(msg, "MissingPDFException"); + } +} class UnexpectedResponseException extends BaseException { constructor(msg, status) { - super(msg); + super(msg, "UnexpectedResponseException"); this.status = status; } } @@ -500,12 +508,20 @@ class UnexpectedResponseException extends BaseException { /** * Error caused during parsing PDF data. */ -class FormatError extends BaseException {} +class FormatError extends BaseException { + constructor(msg) { + super(msg, "FormatError"); + } +} /** * Error used to indicate task cancellation. */ -class AbortException extends BaseException {} +class AbortException extends BaseException { + constructor(msg) { + super(msg, "AbortException"); + } +} const NullCharactersRegExp = /\x00/g;