Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[api-major] Replace MissingPDFException and UnexpectedResponseException with one exception #19264

Merged
merged 1 commit into from
Jan 16, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions examples/mobile-viewer/viewer.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -91,10 +91,10 @@ const PDFViewerApplication = {
let key = "pdfjs-loading-error";
if (reason instanceof pdfjsLib.InvalidPDFException) {
key = "pdfjs-invalid-file-error";
} else if (reason instanceof pdfjsLib.MissingPDFException) {
key = "pdfjs-missing-file-error";
} else if (reason instanceof pdfjsLib.UnexpectedResponseException) {
key = "pdfjs-unexpected-response-error";
} else if (reason instanceof pdfjsLib.ResponseException) {
key = reason.missing
? "pdfjs-missing-file-error"
: "pdfjs-unexpected-response-error";
}
self.l10n.get(key).then(msg => {
self.error(msg, { message: reason?.message });
Expand Down
6 changes: 3 additions & 3 deletions src/display/fetch_stream.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
import { AbortException, assert, warn } from "../shared/util.js";
import {
createHeaders,
createResponseStatusError,
createResponseError,
extractFilenameFromHeader,
getResponseOrigin,
validateRangeRequestCapabilities,
Expand Down Expand Up @@ -127,7 +127,7 @@ class PDFFetchStreamReader {
stream._responseOrigin = getResponseOrigin(response.url);

if (!validateResponseStatus(response.status)) {
throw createResponseStatusError(response.status, url);
throw createResponseError(response.status, url);
}
this._reader = response.body.getReader();
this._headersCapability.resolve();
Expand Down Expand Up @@ -230,7 +230,7 @@ class PDFFetchStreamRangeReader {
);
}
if (!validateResponseStatus(response.status)) {
throw createResponseStatusError(response.status, url);
throw createResponseError(response.status, url);
}
this._readCapability.resolve();
this._reader = response.body.getReader();
Expand Down
6 changes: 3 additions & 3 deletions src/display/network.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
import { assert, stringToBytes, warn } from "../shared/util.js";
import {
createHeaders,
createResponseStatusError,
createResponseError,
extractFilenameFromHeader,
getResponseOrigin,
validateRangeRequestCapabilities,
Expand Down Expand Up @@ -329,7 +329,7 @@ class PDFNetworkStreamFullRequestReader {
}

_onError(status) {
this._storedError = createResponseStatusError(status, this._url);
this._storedError = createResponseError(status, this._url);
this._headersCapability.reject(this._storedError);
for (const requestCapability of this._requests) {
requestCapability.reject(this._storedError);
Expand Down Expand Up @@ -454,7 +454,7 @@ class PDFNetworkStreamRangeRequestReader {
}

_onError(status) {
this._storedError ??= createResponseStatusError(status, this._url);
this._storedError ??= createResponseError(status, this._url);
for (const requestCapability of this._requests) {
requestCapability.reject(this._storedError);
}
Expand Down
18 changes: 6 additions & 12 deletions src/display/network_utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,7 @@
* limitations under the License.
*/

import {
assert,
MissingPDFException,
UnexpectedResponseException,
} from "../shared/util.js";
import { assert, ResponseException } from "../shared/util.js";
import { getFilenameFromContentDispositionHeader } from "./content_disposition.js";
import { isPdfFile } from "./display_utils.js";

Expand Down Expand Up @@ -108,13 +104,11 @@ function extractFilenameFromHeader(responseHeaders) {
return null;
}

function createResponseStatusError(status, url) {
if (status === 404 || (status === 0 && url.startsWith("file:"))) {
return new MissingPDFException('Missing PDF "' + url + '".');
}
return new UnexpectedResponseException(
function createResponseError(status, url) {
return new ResponseException(
`Unexpected server response (${status}) while retrieving PDF "${url}".`,
status
status,
/* missing = */ status === 404 || (status === 0 && url.startsWith("file:"))
);
}

Expand All @@ -124,7 +118,7 @@ function validateResponseStatus(status) {

export {
createHeaders,
createResponseStatusError,
createResponseError,
extractFilenameFromHeader,
getResponseOrigin,
validateRangeRequestCapabilities,
Expand Down
5 changes: 3 additions & 2 deletions src/display/node_stream.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,8 @@
*/
/* globals process */

import { AbortException, assert, MissingPDFException } from "../shared/util.js";
import { AbortException, assert } from "../shared/util.js";
import { createResponseError } from "./network_utils.js";

if (typeof PDFJSDev !== "undefined" && PDFJSDev.test("MOZCENTRAL")) {
throw new Error(
Expand Down Expand Up @@ -111,7 +112,7 @@ class PDFNodeStreamFsFullReader {
},
error => {
if (error.code === "ENOENT") {
error = new MissingPDFException(`Missing PDF "${this._url}".`);
error = createResponseError(/* status = */ 0, this._url.href);
}
this._storedError = error;
this._headersCapability.reject(error);
Expand Down
6 changes: 2 additions & 4 deletions src/pdf.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,13 +31,12 @@ import {
FeatureTest,
ImageKind,
InvalidPDFException,
MissingPDFException,
normalizeUnicode,
OPS,
PasswordResponses,
PermissionFlag,
ResponseException,
shadow,
UnexpectedResponseException,
Util,
VerbosityLevel,
} from "./shared/util.js";
Expand Down Expand Up @@ -112,7 +111,6 @@ export {
InvalidPDFException,
isDataScheme,
isPdfFile,
MissingPDFException,
noContextMenu,
normalizeUnicode,
OPS,
Expand All @@ -124,12 +122,12 @@ export {
PermissionFlag,
PixelsPerInch,
RenderingCancelledException,
ResponseException,
setLayerDimensions,
shadow,
stopEvent,
TextLayer,
TouchManager,
UnexpectedResponseException,
Util,
VerbosityLevel,
version,
Expand Down
12 changes: 4 additions & 8 deletions src/shared/message_handler.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,8 @@ import {
AbortException,
assert,
InvalidPDFException,
MissingPDFException,
PasswordException,
UnexpectedResponseException,
ResponseException,
UnknownErrorException,
unreachable,
} from "./util.js";
Expand All @@ -46,9 +45,8 @@ function wrapReason(ex) {
if (
ex instanceof AbortException ||
ex instanceof InvalidPDFException ||
ex instanceof MissingPDFException ||
ex instanceof PasswordException ||
ex instanceof UnexpectedResponseException ||
ex instanceof ResponseException ||
ex instanceof UnknownErrorException
) {
// Avoid re-creating the exception when its type is already correct.
Expand All @@ -65,12 +63,10 @@ function wrapReason(ex) {
return new AbortException(ex.message);
case "InvalidPDFException":
return new InvalidPDFException(ex.message);
case "MissingPDFException":
return new MissingPDFException(ex.message);
case "PasswordException":
return new PasswordException(ex.message, ex.code);
case "UnexpectedResponseException":
return new UnexpectedResponseException(ex.message, ex.status);
case "ResponseException":
return new ResponseException(ex.message, ex.status, ex.missing);
case "UnknownErrorException":
return new UnknownErrorException(ex.message, ex.details);
}
Expand Down
16 changes: 5 additions & 11 deletions src/shared/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -501,16 +501,11 @@ class InvalidPDFException extends BaseException {
}
}

class MissingPDFException extends BaseException {
constructor(msg) {
super(msg, "MissingPDFException");
}
}

class UnexpectedResponseException extends BaseException {
constructor(msg, status) {
super(msg, "UnexpectedResponseException");
class ResponseException extends BaseException {
constructor(msg, status, missing) {
super(msg, "ResponseException");
this.status = status;
this.missing = missing;
}
}

Expand Down Expand Up @@ -1161,7 +1156,6 @@ export {
LINE_DESCENT_FACTOR,
LINE_FACTOR,
MAX_IMAGE_SIZE_TO_CACHE,
MissingPDFException,
normalizeUnicode,
objectFromMap,
objectSize,
Expand All @@ -1171,6 +1165,7 @@ export {
PasswordResponses,
PermissionFlag,
RenderingIntentFlag,
ResponseException,
setVerbosityLevel,
shadow,
string32,
Expand All @@ -1180,7 +1175,6 @@ export {
TextRenderingMode,
toBase64Util,
toHexUtil,
UnexpectedResponseException,
UnknownErrorException,
unreachable,
utf8StringToString,
Expand Down
6 changes: 4 additions & 2 deletions test/unit/api_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,12 +20,12 @@ import {
ImageKind,
InvalidPDFException,
isNodeJS,
MissingPDFException,
objectSize,
OPS,
PasswordException,
PasswordResponses,
PermissionFlag,
ResponseException,
UnknownErrorException,
} from "../../src/shared/util.js";
import {
Expand Down Expand Up @@ -297,7 +297,9 @@ describe("api", function () {
// Shouldn't get here.
expect(false).toEqual(true);
} catch (reason) {
expect(reason instanceof MissingPDFException).toEqual(true);
expect(reason instanceof ResponseException).toEqual(true);
expect(reason.status).toEqual(isNodeJS ? 0 : 404);
expect(reason.missing).toEqual(true);
}

await loadingTask.destroy();
Expand Down
8 changes: 3 additions & 5 deletions test/unit/network_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,10 +13,7 @@
* limitations under the License.
*/

import {
AbortException,
UnexpectedResponseException,
} from "../../src/shared/util.js";
import { AbortException, ResponseException } from "../../src/shared/util.js";
import { PDFNetworkStream } from "../../src/display/network.js";
import { testCrossOriginRedirects } from "./common_pdfstream_tests.js";
import { TestPdfsServer } from "./test_utils.js";
Expand Down Expand Up @@ -155,8 +152,9 @@ describe("network", function () {
// Shouldn't get here.
expect(false).toEqual(true);
} catch (ex) {
expect(ex instanceof UnexpectedResponseException).toEqual(true);
expect(ex instanceof ResponseException).toEqual(true);
Snuffleupagus marked this conversation as resolved.
Show resolved Hide resolved
expect(ex.status).toEqual(0);
expect(ex.missing).toEqual(false);
}
}

Expand Down
42 changes: 18 additions & 24 deletions test/unit/network_utils_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,12 @@

import {
createHeaders,
createResponseStatusError,
createResponseError,
extractFilenameFromHeader,
validateRangeRequestCapabilities,
validateResponseStatus,
} from "../../src/display/network_utils.js";
import {
MissingPDFException,
UnexpectedResponseException,
} from "../../src/shared/util.js";
import { ResponseException } from "../../src/shared/util.js";

describe("network_utils", function () {
describe("createHeaders", function () {
Expand Down Expand Up @@ -370,31 +367,28 @@ describe("network_utils", function () {
});
});

describe("createResponseStatusError", function () {
it("handles missing PDF file responses", function () {
expect(createResponseStatusError(404, "https://foo.com/bar.pdf")).toEqual(
new MissingPDFException('Missing PDF "https://foo.com/bar.pdf".')
);
describe("createResponseError", function () {
function testCreateResponseError(url, status, missing) {
const error = createResponseError(status, url);

expect(createResponseStatusError(0, "file://foo.pdf")).toEqual(
new MissingPDFException('Missing PDF "file://foo.pdf".')
expect(error instanceof ResponseException).toEqual(true);
expect(error.message).toEqual(
`Unexpected server response (${status}) while retrieving PDF "${url}".`
);
expect(error.status).toEqual(status);
expect(error.missing).toEqual(missing);
}

it("handles missing PDF file responses", function () {
testCreateResponseError("https://foo.com/bar.pdf", 404, true);

testCreateResponseError("file://foo.pdf", 0, true);
});

it("handles unexpected responses", function () {
expect(createResponseStatusError(302, "https://foo.com/bar.pdf")).toEqual(
new UnexpectedResponseException(
"Unexpected server response (302) while retrieving PDF " +
'"https://foo.com/bar.pdf".'
)
);
testCreateResponseError("https://foo.com/bar.pdf", 302, false);

expect(createResponseStatusError(0, "https://foo.com/bar.pdf")).toEqual(
new UnexpectedResponseException(
"Unexpected server response (0) while retrieving PDF " +
'"https://foo.com/bar.pdf".'
)
);
testCreateResponseError("https://foo.com/bar.pdf", 0, false);
});
});

Expand Down
6 changes: 2 additions & 4 deletions test/unit/pdf_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,12 @@ import {
ImageKind,
InvalidPDFException,
isNodeJS,
MissingPDFException,
normalizeUnicode,
OPS,
PasswordResponses,
PermissionFlag,
ResponseException,
shadow,
UnexpectedResponseException,
Util,
VerbosityLevel,
} from "../../src/shared/util.js";
Expand Down Expand Up @@ -90,7 +89,6 @@ const expectedAPI = Object.freeze({
InvalidPDFException,
isDataScheme,
isPdfFile,
MissingPDFException,
noContextMenu,
normalizeUnicode,
OPS,
Expand All @@ -102,12 +100,12 @@ const expectedAPI = Object.freeze({
PermissionFlag,
PixelsPerInch,
RenderingCancelledException,
ResponseException,
setLayerDimensions,
shadow,
stopEvent,
TextLayer,
TouchManager,
UnexpectedResponseException,
Util,
VerbosityLevel,
version,
Expand Down
Loading
Loading