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

Throw if request canceled and received incomplete response #767

Merged
merged 7 commits into from Apr 5, 2019
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions source/as-promise.ts
Expand Up @@ -40,6 +40,11 @@ export default function asPromise(options: Options) {
return;
}

if (response.canceled()) {
szmarczak marked this conversation as resolved.
Show resolved Hide resolved
// Canceled while downloading - will throw a CancelError or TimeoutError
szmarczak marked this conversation as resolved.
Show resolved Hide resolved
return;
}

const limitStatusCode = options.followRedirect ? 299 : 399;

response.body = data;
Expand Down
10 changes: 9 additions & 1 deletion source/as-stream.ts
Expand Up @@ -23,7 +23,15 @@ export default function asStream(options: MergedOptions) {
const emitter = requestAsEventEmitter(options, input);

// Cancels the request
proxy._destroy = emitter.abort;
proxy._destroy = (error, callback) => {
// Stop transmitting data,
// so the stream won't have an `end` event.
output.destroy();
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The more elegant way would be to unpipe the response.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We don't have to care about it. It will end if no error is provided, otherwise it will throw before ending.


// Abort safely
emitter.abort();
callback(error);
};

emitter.on('response', (response: Response) => {
const {statusCode} = response;
Expand Down
21 changes: 20 additions & 1 deletion source/request-as-event-emitter.ts
Expand Up @@ -26,10 +26,14 @@ export interface RequestAsEventEmitter extends EventEmitter {
abort: () => void;
}

export interface Abortable {
abort: () => void;
}

export default (options, input?: TransformStream) => {
const emitter = new EventEmitter() as RequestAsEventEmitter;
const redirects = [] as string[];
let currentRequest: http.ClientRequest;
let currentRequest: http.ClientRequest | Abortable;
let requestUrl: string;
let redirectString: string;
let uploadBodySize: number | undefined;
Expand Down Expand Up @@ -120,6 +124,21 @@ export default (options, input?: TransformStream) => {
gotOptions: options
};

// Cached requests don't have the request object (so we can't do `request.aborted`)
// so we need to create our own `canceled` property.
szmarczak marked this conversation as resolved.
Show resolved Hide resolved
if (response.fromCache) {
response.canceled = () => false;

currentRequest = {
abort: () => {
response.canceled = () => true;
response.destroy();
}
};
} else {
response.canceled = () => response.req.aborted;
}

const rawCookies = response.headers['set-cookie'];
if (options.cookieJar && rawCookies) {
await Promise.all(rawCookies.map(rawCookie => setCookie(rawCookie, response.url)));
Expand Down
32 changes: 32 additions & 0 deletions test/timeout.ts
Expand Up @@ -449,3 +449,35 @@ test('no memory leak when using socket timeout and keepalive agent', withServer,

t.is(socket.listenerCount('timeout'), 0);
});

test('throws on incomplete (canceled) response - promise', withServer, async (t, server, got) => {
server.get('/', downloadHandler);

await t.throwsAsync(got({
timeout: {request: 500}
}), got.TimeoutError);
});

test('throws on incomplete (canceled) response - promise #2', withServer, async (t, server, got) => {
server.get('/', downloadHandler);

const promise = got('').on('response', () => {
setTimeout(() => promise.cancel(), 500);
});

await t.throwsAsync(promise, got.CancelError);
});

test('throws on incomplete (canceled) response - stream', withServer, async (t, server, got) => {
server.get('/', downloadHandler);

const errorString = 'Foobar';

const stream = got.stream('').on('response', () => {
setTimeout(() => stream.destroy(new Error(errorString)), 500);
});

await t.throwsAsync(getStream(stream), errorString);
});

test.todo('throws on incomplete (canceled) response - cached request');