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

Ensure that ReadableStreams are cancelled with actual Errors #11034

Merged
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
7 changes: 3 additions & 4 deletions src/core/chunked_stream.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
}
}
Expand Down
8 changes: 4 additions & 4 deletions src/core/pdf_manager.js
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ class BasePdfManager {
this._password = password;
}

terminate() {
terminate(reason) {
unreachable('Abstract method `terminate` called');
}
}
Expand Down Expand Up @@ -134,7 +134,7 @@ class LocalPdfManager extends BasePdfManager {
return this._loadedStreamPromise;
}

terminate() {}
terminate(reason) {}
}

class NetworkPdfManager extends BasePdfManager {
Expand Down Expand Up @@ -188,8 +188,8 @@ class NetworkPdfManager extends BasePdfManager {
return this.streamManager.onLoadedStream();
}

terminate() {
this.streamManager.abort();
terminate(reason) {
this.streamManager.abort(reason);
}
}

Expand Down
18 changes: 9 additions & 9 deletions src/core/worker.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -274,8 +274,8 @@ var WorkerMessageHandler = {
cancelXHRs = null;
});

cancelXHRs = function () {
pdfStream.cancelAllRequests('abort');
cancelXHRs = function(reason) {
pdfStream.cancelAllRequests(reason);
};

return pdfManagerCapability.promise;
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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();

Expand Down
12 changes: 7 additions & 5 deletions src/display/api.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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) {
Expand Down
3 changes: 2 additions & 1 deletion test/unit/fetch_stream_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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() {
Expand Down Expand Up @@ -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;
});

Expand Down
4 changes: 2 additions & 2 deletions test/unit/message_handler_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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();
Expand Down
3 changes: 2 additions & 1 deletion test/unit/network_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
* limitations under the License.
*/

import { AbortException } from '../../src/shared/util';
import { PDFNetworkStream } from '../../src/display/network';

describe('network', function() {
Expand Down Expand Up @@ -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;
});

Expand Down
6 changes: 3 additions & 3 deletions test/unit/node_stream_spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';

Expand Down Expand Up @@ -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;
});

Expand Down