Skip to content

Commit

Permalink
Expose options.timeout in hooks
Browse files Browse the repository at this point in the history
  • Loading branch information
szmarczak committed Nov 4, 2019
1 parent 10d30c6 commit d520a3a
Show file tree
Hide file tree
Showing 4 changed files with 29 additions and 11 deletions.
6 changes: 1 addition & 5 deletions source/normalize-arguments.ts
Expand Up @@ -73,13 +73,9 @@ export const preNormalizeArguments = (options: Options, defaults?: NormalizedOpt
}

if (is.number(options.timeout)) {
options.gotTimeout = {request: options.timeout};
} else if (is.object(options.timeout)) {
options.gotTimeout = options.timeout;
options.timeout = {request: options.timeout};
}

delete options.timeout;

const {retry} = options;
options.retry = {
calculateDelay: retryObject => retryObject.computedValue,
Expand Down
14 changes: 10 additions & 4 deletions source/request-as-event-emitter.ts
Expand Up @@ -99,6 +99,8 @@ export default (options: NormalizedOptions, input?: TransformStream) => {

let timings: Timings;
const handleResponse = async (response: http.ServerResponse | ResponseObject): Promise<void> => {
options.timeout = timeout;

This comment has been minimized.

Copy link
@szmarczak

szmarczak Nov 5, 2019

Author Collaborator

This needs to be applied only if the response is cached. Needs a test.

This comment has been minimized.

Copy link
@szmarczak

szmarczak Nov 5, 2019

Author Collaborator

Actually it doesn't matter as there is no hook called between handleRequest and handleResponse.


try {
/* istanbul ignore next: fixes https://github.com/electron/electron/blob/cbb460d47628a7a146adf4419ed48550a98b2923/lib/browser/api/net.js#L59-L65 */
if (options.useElectronNet) {
Expand Down Expand Up @@ -194,6 +196,7 @@ export default (options: NormalizedOptions, input?: TransformStream) => {
}

currentRequest = request;
options.timeout = timeout;

const onError = (error: Error): void => {
const isTimedOutError = error instanceof TimedOutTimeoutError;
Expand Down Expand Up @@ -235,8 +238,8 @@ export default (options: NormalizedOptions, input?: TransformStream) => {

uploadProgress(request, emitter, uploadBodySize);

if (options.gotTimeout) {
timedOut(request, options.gotTimeout, options);
if (options.timeout) {
timedOut(request, options.timeout, options);
}

emitter.emit('request', request);
Expand Down Expand Up @@ -269,9 +272,12 @@ export default (options: NormalizedOptions, input?: TransformStream) => {
}
};

const {timeout} = options;
delete options.timeout;

if (options.cache) {
const cacheableRequest = new CacheableRequest(requestFn, options.cache);
const cacheRequest = cacheableRequest(options as https.RequestOptions, handleResponse);
const cacheRequest = cacheableRequest(options as unknown as https.RequestOptions, handleResponse);

cacheRequest.once('error', error => {
if (error instanceof CacheableRequest.RequestError) {
Expand All @@ -286,7 +292,7 @@ export default (options: NormalizedOptions, input?: TransformStream) => {
// Catches errors thrown by calling requestFn(...)
try {
// @ts-ignore TS complains that URLSearchParams is not the same as URLSearchParams
handleRequest(requestFn(options as any as URL, handleResponse));
handleRequest(requestFn(options as unknown as URL, handleResponse));
} catch (error) {
emitError(new RequestError(error, options));
}
Expand Down
4 changes: 2 additions & 2 deletions source/utils/types.ts
Expand Up @@ -164,9 +164,9 @@ export interface Options extends Except<https.RequestOptions, 'agent' | 'timeout
maxRedirects?: number;
}

export interface NormalizedOptions extends Except<Required<Options>, 'timeout' | 'dnsCache' | 'retry' | 'auth' | 'body' | 'port'> {
export interface NormalizedOptions extends Except<Required<Options>, 'dnsCache' | 'retry' | 'auth' | 'body' | 'port'> {
hooks: Hooks;
gotTimeout: Required<Delays>;
timeout: Required<Delays>;
retry: NormalizedRetryOptions;
lookup?: CacheableLookup['lookup'];
readonly prefixUrl: string;
Expand Down
16 changes: 16 additions & 0 deletions test/hooks.ts
Expand Up @@ -538,3 +538,19 @@ test('catches HTTPErrors', withServer, async (t, _server, got) => {
}
}));
});

test('timeout can be modified using a hook', withServer, async (t, server, got) => {
server.get('/', () => {});

await t.throwsAsync(got({
timeout: 1000,
hooks: {
init: [
options => {
options.timeout.request = 500;
}
]
},
retry: 0
}), 'Timeout awaiting \'request\' for 500ms');
});

0 comments on commit d520a3a

Please sign in to comment.