From a3150166ec80165a85763adabef3a62dbdee5258 Mon Sep 17 00:00:00 2001
From: Jonas Jenwald <jonas.jenwald@gmail.com>
Date: Thu, 1 Aug 2019 16:31:32 +0200
Subject: [PATCH] Ensure that `ReadableStream`s are cancelled with actual
Errors
There's a number of spots in the current code, and tests, where `cancel` methods are not called with appropriate arguments (leading to Promises not being rejected with Errors as intended).
In some cases the cancel `reason` is implicitly set to `undefined`, and in others the cancel `reason` is just a plain String. To address this inconsistency, the patch changes things such that cancelling is done with `AbortException`s everywhere instead.
---
src/core/chunked_stream.js | 7 +++----
src/core/pdf_manager.js | 8 ++++----
src/core/worker.js | 18 +++++++++---------
src/display/api.js | 12 +++++++-----
test/unit/fetch_stream_spec.js | 3 ++-
test/unit/message_handler_spec.js | 4 ++--
test/unit/network_spec.js | 3 ++-
test/unit/node_stream_spec.js | 6 +++---
8 files changed, 32 insertions(+), 29 deletions(-)
diff --git a/src/core/chunked_stream.js b/src/core/chunked_stream.js
index a0f29ec74..cd2dadff7 100644
--- a/src/core/chunked_stream.js
+++ b/src/core/chunked_stream.js
@@ -567,14 +567,13 @@ class ChunkedStreamManager {
return Math.floor((end - 1) / this.chunkSize) + 1;
}
- abort() {
+ abort(reason) {
this.aborted = true;
if (this.pdfNetworkStream) {
- this.pdfNetworkStream.cancelAllRequests('abort');
+ this.pdfNetworkStream.cancelAllRequests(reason);
}
for (const requestId in this.promisesByRequest) {
- this.promisesByRequest[requestId].reject(
- new Error('Request was aborted'));
+ this.promisesByRequest[requestId].reject(reason);
}
}
}
diff --git a/src/core/pdf_manager.js b/src/core/pdf_manager.js
index 36a1729df..6f1c5dfff 100644
--- a/src/core/pdf_manager.js
+++ b/src/core/pdf_manager.js
@@ -97,7 +97,7 @@ class BasePdfManager {
this._password = password;
}
- terminate() {
+ terminate(reason) {
unreachable('Abstract method `terminate` called');
}
}
@@ -134,7 +134,7 @@ class LocalPdfManager extends BasePdfManager {
return this._loadedStreamPromise;
}
- terminate() {}
+ terminate(reason) {}
}
class NetworkPdfManager extends BasePdfManager {
@@ -188,8 +188,8 @@ class NetworkPdfManager extends BasePdfManager {
return this.streamManager.onLoadedStream();
}
- terminate() {
- this.streamManager.abort();
+ terminate(reason) {
+ this.streamManager.abort(reason);
}
}
diff --git a/src/core/worker.js b/src/core/worker.js
index 4312c3856..5ce47b43a 100644
--- a/src/core/worker.js
+++ b/src/core/worker.js
@@ -14,10 +14,10 @@
*/
import {
- arrayByteLength, arraysToBytes, createPromiseCapability, getVerbosityLevel,
- info, InvalidPDFException, MissingPDFException, PasswordException,
- setVerbosityLevel, UnexpectedResponseException, UnknownErrorException,
- UNSUPPORTED_FEATURES, VerbosityLevel, warn
+ AbortException, arrayByteLength, arraysToBytes, createPromiseCapability,
+ getVerbosityLevel, info, InvalidPDFException, MissingPDFException,
+ PasswordException, setVerbosityLevel, UnexpectedResponseException,
+ UnknownErrorException, UNSUPPORTED_FEATURES, VerbosityLevel, warn
} from '../shared/util';
import { clearPrimitiveCaches, Ref } from './primitives';
import { LocalPdfManager, NetworkPdfManager } from './pdf_manager';
@@ -274,8 +274,8 @@ var WorkerMessageHandler = {
cancelXHRs = null;
});
- cancelXHRs = function () {
- pdfStream.cancelAllRequests('abort');
+ cancelXHRs = function(reason) {
+ pdfStream.cancelAllRequests(reason);
};
return pdfManagerCapability.promise;
@@ -349,7 +349,7 @@ var WorkerMessageHandler = {
if (terminated) {
// We were in a process of setting up the manager, but it got
// terminated in the middle.
- newPdfManager.terminate();
+ newPdfManager.terminate(new AbortException('Worker was terminated.'));
throw new Error('Worker was terminated');
}
pdfManager = newPdfManager;
@@ -579,11 +579,11 @@ var WorkerMessageHandler = {
handler.on('Terminate', function wphTerminate(data) {
terminated = true;
if (pdfManager) {
- pdfManager.terminate();
+ pdfManager.terminate(new AbortException('Worker was terminated.'));
pdfManager = null;
}
if (cancelXHRs) {
- cancelXHRs();
+ cancelXHRs(new AbortException('Worker was terminated.'));
}
clearPrimitiveCaches();
diff --git a/src/display/api.js b/src/display/api.js
index 1b637000e..bea055001 100644
--- a/src/display/api.js
+++ b/src/display/api.js
@@ -16,10 +16,11 @@
/* eslint no-var: error */
import {
- assert, createPromiseCapability, getVerbosityLevel, info, InvalidPDFException,
- isArrayBuffer, isSameOrigin, MissingPDFException, NativeImageDecoding,
- PasswordException, setVerbosityLevel, shadow, stringToBytes,
- UnexpectedResponseException, UnknownErrorException, unreachable, URL, warn
+ AbortException, assert, createPromiseCapability, getVerbosityLevel, info,
+ InvalidPDFException, isArrayBuffer, isSameOrigin, MissingPDFException,
+ NativeImageDecoding, PasswordException, setVerbosityLevel, shadow,
+ stringToBytes, UnexpectedResponseException, UnknownErrorException,
+ unreachable, URL, warn
} from '../shared/util';
import {
deprecated, DOMCanvasFactory, DOMCMapReaderFactory, DummyStatTimer,
@@ -1768,7 +1769,8 @@ class WorkerTransport {
Promise.all(waitOn).then(() => {
this.fontLoader.clear();
if (this._networkStream) {
- this._networkStream.cancelAllRequests();
+ this._networkStream.cancelAllRequests(
+ new AbortException('Worker was terminated.'));
}
if (this.messageHandler) {
diff --git a/test/unit/fetch_stream_spec.js b/test/unit/fetch_stream_spec.js
index 0e0a56926..da33ff685 100644
--- a/test/unit/fetch_stream_spec.js
+++ b/test/unit/fetch_stream_spec.js
@@ -14,6 +14,7 @@
*/
/* eslint no-var: error */
+import { AbortException } from '../../src/shared/util';
import { PDFFetchStream } from '../../src/display/fetch_stream';
describe('fetch_stream', function() {
@@ -72,7 +73,7 @@ describe('fetch_stream', function() {
isStreamingSupported = fullReader.isStreamingSupported;
isRangeSupported = fullReader.isRangeSupported;
// We shall be able to close full reader without any issue.
- fullReader.cancel(new Error('Don\'t need full reader'));
+ fullReader.cancel(new AbortException('Don\'t need fullReader.'));
fullReaderCancelled = true;
});
diff --git a/test/unit/message_handler_spec.js b/test/unit/message_handler_spec.js
index 72735300b..f9465791f 100644
--- a/test/unit/message_handler_spec.js
+++ b/test/unit/message_handler_spec.js
@@ -13,7 +13,7 @@
* limitations under the License.
*/
-import { createPromiseCapability } from '../../src/shared/util';
+import { AbortException, createPromiseCapability } from '../../src/shared/util';
import { LoopbackPort } from '../../src/display/api';
import { MessageHandler } from '../../src/shared/message_handler';
@@ -124,7 +124,7 @@ describe('message_handler', function () {
return sleep(10);
}).then(() => {
expect(log).toEqual('01p2');
- return reader.cancel();
+ return reader.cancel(new AbortException('reader cancelled.'));
}).then(() => {
expect(log).toEqual('01p2c4');
done();
diff --git a/test/unit/network_spec.js b/test/unit/network_spec.js
index fef547e66..5402e66d5 100644
--- a/test/unit/network_spec.js
+++ b/test/unit/network_spec.js
@@ -13,6 +13,7 @@
* limitations under the License.
*/
+import { AbortException } from '../../src/shared/util';
import { PDFNetworkStream } from '../../src/display/network';
describe('network', function() {
@@ -79,7 +80,7 @@ describe('network', function() {
isStreamingSupported = fullReader.isStreamingSupported;
isRangeSupported = fullReader.isRangeSupported;
// we shall be able to close the full reader without issues
- fullReader.cancel('Don\'t need full reader');
+ fullReader.cancel(new AbortException('Don\'t need fullReader.'));
fullReaderCancelled = true;
});
diff --git a/test/unit/node_stream_spec.js b/test/unit/node_stream_spec.js
index 81781caf2..3023b6190 100644
--- a/test/unit/node_stream_spec.js
+++ b/test/unit/node_stream_spec.js
@@ -14,7 +14,7 @@
*/
/* globals __non_webpack_require__ */
-import { assert } from '../../src/shared/util';
+import { AbortException, assert } from '../../src/shared/util';
import isNodeJS from '../../src/shared/is_node';
import { PDFNodeStream } from '../../src/display/node_stream';
@@ -167,14 +167,14 @@ describe('node_stream', function() {
isStreamingSupported1 = fullReader1.isStreamingSupported;
isRangeSupported1 = fullReader1.isRangeSupported;
// we shall be able to close the full reader without issues
- fullReader1.cancel('Don\'t need full reader');
+ fullReader1.cancel(new AbortException('Don\'t need fullReader1.'));
fullReaderCancelled1 = true;
});
let promise2 = fullReader2.headersReady.then(function () {
isStreamingSupported2 = fullReader2.isStreamingSupported;
isRangeSupported2 = fullReader2.isRangeSupported;
- fullReader2.cancel('Don\'t need full reader');
+ fullReader2.cancel(new AbortException('Don\'t need fullReader2.'));
fullReaderCancelled2 = true;
});