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.

Sign up for GitHub

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

Implement disabling timeouts #127

Merged
merged 5 commits into from
Apr 22, 2019
Merged

Implement disabling timeouts #127

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
b311f73
limited maximum timeout, added timeout: false options, fixes #117
stroncium Apr 20, 2019
a1ae24c
added handling of delay promise rejection
stroncium Apr 20, 2019
f98ea97
range error
stroncium Apr 21, 2019
f851814
error text
stroncium Apr 22, 2019
39c576c
Update index.js
szmarczak Apr 22, 2019
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: 3 additions & 2 deletions index.d.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,11 +61,12 @@ export interface Options extends RequestInit {
retry?: number;

/**
Timeout in milliseconds for getting a response.
Timeout in milliseconds for getting a response. Can not be greater than 2147483647.
If set to `false`, there will be no timeout.

@default 10000
*/
timeout?: number;
timeout?: number | false;
Copy link
Collaborator

Choose a reason for hiding this comment

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

I tried this in a different TS project yesterday and it didn't seem to work. Are you sure this is correct?

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm pretty sure this works. Just tested quickly on a TS project and seemed like it did.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is a new feature, but not experimental. Available since TypeScript 2.0.


/**
Hooks allow modifications during the request lifecycle. Hook functions may be async and are run serially.
Expand Down
14 changes: 12 additions & 2 deletions index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,7 +111,13 @@ class TimeoutError extends Error {
}
}

const delay = ms => new Promise(resolve => setTimeout(resolve, ms));
const delay = ms => new Promise((resolve, reject) => {
if (ms > 2147483647) { // The maximum value of a 32bit int (see #117)
reject(new RangeError('The `timeout` option cannot be greater than 2147483647'));
} else {
setTimeout(resolve, ms);
}
});

// `Promise.race()` workaround (#91)
const timeout = (promise, ms, abortController) => new Promise((resolve, reject) => {
Expand All @@ -123,7 +129,7 @@ const timeout = (promise, ms, abortController) => new Promise((resolve, reject)
}

reject(new TimeoutError());
});
}).catch(reject);
/* eslint-enable promise/prefer-await-to-then */
});

Expand Down Expand Up @@ -293,6 +299,10 @@ class Ky {
await hook(this._options);
}

if (this._timeout === false) {
return fetch(this._input, this._options);
}

return timeout(fetch(this._input, this._options), this._timeout, this.abortController);
}
}
Expand Down
5 changes: 3 additions & 2 deletions readme.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -190,10 +190,11 @@ It adheres to the [`Retry-After`](https://developer.mozilla.org/en-US/docs/Web/H

##### timeout

Type: `number`<br>
Type: `number` `false`<br>
Default: `10000`

Timeout in milliseconds for getting a response.
Timeout in milliseconds for getting a response. Can not be greater than 2147483647.
If set to `false`, there will be no timeout.

##### hooks

Expand Down
32 changes: 32 additions & 0 deletions test/main.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -169,6 +169,38 @@ test('timeout option', async t => {
await server.close();
});

test('timeout:false option', async t => {
let requestCount = 0;

const server = await createTestServer();
server.get('/', async (request, response) => {
requestCount++;
await delay(1000);
response.end(fixture);
});

await t.notThrowsAsync(ky(server.url, {timeout: false}).text(), TimeoutError);
t.is(requestCount, 1);

await server.close();
});

test('invalid timeout option', async t => { // #117
let requestCount = 0;

const server = await createTestServer();
server.get('/', async (request, response) => {
requestCount++;
await delay(1000);
response.end(fixture);
});

await t.throwsAsync(ky(server.url, {timeout: 21474836470}).text(), RangeError, 'The `timeout` option cannot be greater than 2147483647');
t.is(requestCount, 1);

await server.close();
});

test('searchParams option', async t => {
const server = await createTestServer();

Expand Down