Skip to content

Commit

Permalink
Fix hanging promise on timeout on HTTP error
Browse files Browse the repository at this point in the history
Fixes #1341
  • Loading branch information
szmarczak committed Jul 3, 2020
1 parent 7582d91 commit 934211f
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 5 deletions.
14 changes: 10 additions & 4 deletions source/as-promise/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -148,9 +148,7 @@ export default function asPromise<T>(options: NormalizedOptions): CancelableRequ
resolve(options.resolveBodyOnly ? response.body as T : response as unknown as T);
};

request.once('response', onResponse);

request.once('error', async (error: RequestError) => {
const onError = async (error: RequestError) => {
if (promise.isCanceled) {
return;
}
Expand Down Expand Up @@ -219,14 +217,22 @@ export default function asPromise<T>(options: NormalizedOptions): CancelableRequ
if (error instanceof HTTPError) {
// The error will be handled by the `response` event
onResponse(request._response as Response);

// Reattach the error handler, because there may be a timeout later.
process.nextTick(() => {
request.once('error', onError);
});
return;
}

// Don't emit the `response` event
request.destroy();

reject(error);
});
};

request.once('response', onResponse);
request.once('error', onError);

proxyEvents(request, emitter, proxiedRequestEvents);
};
Expand Down
15 changes: 14 additions & 1 deletion test/error.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import {promisify} from 'util';
import http = require('http');
import stream = require('stream');
import test from 'ava';
import got, {RequestError, HTTPError} from '../source';
import got, {RequestError, HTTPError, TimeoutError} from '../source';
import withServer from './helpers/with-server';

const pStreamPipeline = promisify(stream.pipeline);
Expand Down Expand Up @@ -204,6 +204,19 @@ test('errors can have request property', withServer, async (t, server, got) => {
t.truthy(error.request.downloadProgress);
});

test('promise does not hang on timeout on HTTP error', withServer, async (t, server, got) => {
server.get('/', (_request, response) => {
response.statusCode = 404;
response.write('asdf');
});

await t.throwsAsync(got({
timeout: 100
}), {
instanceOf: TimeoutError
});
});

// Fails randomly on Node 10:
// Blocked by https://github.com/istanbuljs/nyc/issues/619
// eslint-disable-next-line ava/no-skip-test
Expand Down

0 comments on commit 934211f

Please sign in to comment.