Skip to content
This repository has been archived by the owner on Apr 22, 2023. It is now read-only.

The http.abort() is too sensitive? #1085

Closed
pyrostrex opened this issue May 22, 2011 · 9 comments
Closed

The http.abort() is too sensitive? #1085

pyrostrex opened this issue May 22, 2011 · 9 comments
Labels

Comments

@pyrostrex
Copy link

assert.js:81
throw new assert.AssertionError({
^
AssertionError: true == false
at ClientRequest.detachSocket (http.js:346:3)
at IncomingMessage. (http.js:1341:11)
at IncomingMessage.emit (events.js:61:17)
at HTTPParser.onMessageComplete (http.js:133:23)
at Socket.ondata (http.js:1226:22)
at Socket._onReadable (net.js:683:27)
at IOWatcher.onReadable as callback

This error was simply because I requested http and get response code other than 200 then I aborted the connection.

@pkulak
Copy link

pkulak commented May 24, 2011

I'm not sure if this is the same thing, but if I upgrade to 1.4.8 from 1.4.7 I just get a steady stream of this:

AssertionError: true == false
at ClientRequest.detachSocket (http.js:346:3)
at IncomingMessage. (http.js:1341:11)
at IncomingMessage.emit (events.js:61:17)
at HTTPParser.onMessageComplete (http.js:133:23)
at Socket.ondata (http.js:1226:22)
at Socket._onReadable (net.js:683:27)
at IOWatcher.onReadable as callback

@JulesAU
Copy link

JulesAU commented May 26, 2011

I'm seeing this quite regularly after the upgrade from .4.7 to .4.8. Not sure exactly how to reproduce yet.
Running HTTPS + express here.

@pyrostrex
Copy link
Author

Weird, why is this problem never been fixed. Is it really hard? :P.

@SaltwaterC
Copy link

I know how to reproduce it. It happens for both HTTP and HTTPS. The assertion error happens only when there's a single data event. Initially I tough it was a variation of #1304, but after testing with more payloads, I found out that it isn't.

Reproducing method:

var http = require('http');
var options = {
    host: '127.0.0.1',
    path: '/1k' // small payload, is fetched into a single chunk returned by the data event
};
var request = http.get(options, function (response) {
    response.on('data', function (chunk) {
        request.abort();
    });
});
var http = require('http');
var options = {
    host: '127.0.0.1',
    path: '/100k' // large payload, is fetched into multiple chunks returned by the data event
};
var request = http.get(options, function (response) {
    response.on('data', function (chunk) {
        request.abort();
    });
});

I made the couple of files by using dd:

dd if=/dev/urandom of=/var/www/1k bs=1024 count=1
dd if=/dev/urandom of=/var/www/100k bs=1024 count=100

@pyrostrex: a bug must be reproduced and understood in order to be solved. I think the above comment answers your question.

@pyrostrex
Copy link
Author

@SaltwaterC: Thanks for the reproduction of bug :)

@SaltwaterC
Copy link

@pyrostrex: it affects me as well. I had to :)

@pyrostrex
Copy link
Author

Well, for the time being I just remove the assertion line to fix this since I think assertion is meant for testing purpose only right? :P

@SaltwaterC
Copy link

I removed the comment as it doesn't completely describe the situation. I managed to understand most of the situation. The auto detachSocket() happens when the agent completes the request, into the end listener, on line 1328: req.detachSocket(socket);

@SaltwaterC
Copy link

That detachSocket() of the end event of the agent is called after the socked is detached and destroyed by abort(). A three line patch saves the day. I'll make a pull request.

koichik added a commit that referenced this issue Jul 14, 2011
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

5 participants