nodules / asker

http.request wrapper with gzip, request retries and http.Agent tuning
MIT License
93 stars 11 forks source link

Should asker stop buffering response on request abort? #122

Closed doochik closed 8 years ago

doochik commented 8 years ago

In some cases data event can be fired after request abort. Asker continues buffering response (https://github.com/nodules/asker/blob/acc28e620c5cf1ef50b31bb470f8e94984a43bea/lib/asker.js#L539) until error or end event will be fired.

Should we fix it? I can change event handler like this

res.on('data', function(chunk) {
    if (httpRequest.rejected) {
        // clear buffer
        // body = [];
        return;
    }

    body.push(chunk);
    bodyLength += chunk.length;
});
narqo commented 8 years ago

Looks like this'll make #92 impossible to implement.

kaero commented 8 years ago

What you mean under "request abort"? Request hasn't a public method to abort a request.

narqo commented 8 years ago

@kaero I believe it's about this peace: https://github.com/nodules/asker/blob/acc28e620c5cf1ef50b31bb470f8e94984a43bea/lib/asker.js#L592-L604

doochik commented 8 years ago

yep

kaero commented 8 years ago

@doochik proposed changes looks acceptable for me, feel free to pr :)

doochik commented 8 years ago

Here it is :) https://github.com/nodules/asker/pull/123

kaero commented 8 years ago

+ asker@1.0.5