Skip to content
This repository has been archived by the owner on Jul 30, 2018. It is now read-only.

add support for signal request option #399

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 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
5 changes: 5 additions & 0 deletions src/request/interfaces.d.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import { AbortSignal } from '@dojo/shim/AbortController';
import { IterableIterator } from '@dojo/shim/iterator';
import Task from '../async/Task';
import UrlSearchParams, { ParamList } from '../UrlSearchParams';
Expand Down Expand Up @@ -57,6 +58,10 @@ export interface RequestOptions {
* Password for HTTP authentication
*/
password?: string;
/**
* Abort signal, allowing for cancelable requests
*/
signal?: AbortSignal;
/**
* Number of milliseconds before the request times out and is canceled
*/
Expand Down
9 changes: 9 additions & 0 deletions src/request/providers/node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -654,6 +654,15 @@ export default function node(url: string, options: NodeRequestOptions = {}): Upl
timeoutReject && timeoutReject(new TimeoutError('The request timed out'));
}, options.timeout);
}

if (options.signal) {
options.signal.addEventListener('abort', () => {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when the request is fulfilled, the event listener should be removed.

const abortError = new Error('Aborted');
abortError.name = 'AbortError';
reject(abortError);
});
}

},
() => {
request.abort();
Expand Down
13 changes: 13 additions & 0 deletions src/request/providers/xhr.ts
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,19 @@ export default function xhr(url: string, options: XhrRequestOptions = {}): Uploa
const task = <UploadObservableTask<XhrResponse>>new Task<XhrResponse>((resolve, reject) => {
timeoutReject = reject;

if (options.signal) {
options.signal.addEventListener('abort', () => {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

when the request is fulfilled, the event listener should be removed.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ycabon I'm not sure there's a good place in the code to remove this event listener. Also, I'm not sure it's necessary as subsequent abort events will have no effect on a fulfilled Task.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My concern was more for cases where the signal is reused for multiple calls, would the Task be kept in memory until the signal and controller are GCed.

let abortError: Error;
try {
abortError = new DOMException('Aborted', 'AbortError');
} catch {
abortError = new Error('Aborted');
abortError.name = 'AbortError';
}
reject(new DOMException('Aborted', 'AbortError'));
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like you should use abortError here.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice catch! Thanks!

});
}

request.onreadystatechange = function() {
if (isAborted) {
return;
Expand Down
15 changes: 15 additions & 0 deletions tests/unit/request/node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import * as zlib from 'zlib';
import { Response } from '../../../src/request/interfaces';
import { default as nodeRequest, NodeResponse } from '../../../src/request/providers/node';
import TimeoutError from '../../../src/request/TimeoutError';
import AbortController from '@dojo/shim/AbortController';

const serverPort = 8124;
const serverUrl = 'http://localhost:' + serverPort;
Expand Down Expand Up @@ -463,6 +464,20 @@ registerSuite('request/node', {
);
},

signal(this: any) {
const dfd = this.async();
const url = getRequestUrl('foo.json');
const controller = new AbortController();
const { signal } = controller;
const request = nodeRequest(url, { signal });
request.catch(
dfd.callback((error: Error) => {
assert.strictEqual(error.name, 'AbortError');
})
);
controller.abort();
},

'user and password': {
both(this: any): void {
const user = 'user name';
Expand Down
18 changes: 18 additions & 0 deletions tests/unit/request/xhr.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
const { registerSuite } = intern.getInterface('object');
const { assert } = intern.getPlugin('chai');

import AbortController from '@dojo/shim/AbortController';
import xhrRequest, { XhrResponse } from '../../../src/request/providers/xhr';
import { Response } from '../../../src/request/interfaces';
import UrlSearchParams from '../../../src/UrlSearchParams';
Expand Down Expand Up @@ -94,6 +95,23 @@ registerSuite('request/providers/xhr', {
);
},

'"signal"'(this: any) {
if (!echoServerAvailable) {
this.skip('No echo server available');
}
const dfd = this.async();
const controller = new AbortController();
const { signal } = controller;
const request = xhrRequest('/__echo/foo.json', { signal, timeout: 100 });
request.catch(
dfd.callback((error: Error) => {
assert.strictEqual(error.name, 'AbortError');
})
);

controller.abort();
},

'user and password'(this: any) {
if (!echoServerAvailable) {
this.skip('No echo server available');
Expand Down