Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

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

Closed
pyrostrex opened this Issue · 9 comments

5 participants

@pyrostrex

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

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

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

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

@SaltwaterC

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

@SaltwaterC: Thanks for the reproduction of bug :)

@SaltwaterC

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

@pyrostrex

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

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

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.

@SaltwaterC SaltwaterC referenced this issue from a commit
@SaltwaterC SaltwaterC Fixes #1085. The agent end event may call detachSocket() after the so…
…cket is detached and destroyed by abort(). This patch avoids that behavior.
e901998
@koichik koichik closed this issue from a commit
@SaltwaterC SaltwaterC Fixes #1085. The agent end event may call detachSocket() after the so…
…cket is detached and destroyed by abort(). This patch avoids that behavior.
5b02d56
@koichik koichik closed this in 5b02d56
@koichik koichik referenced this issue from a commit
@koichik koichik Add tests for #1085 and #1304
Fixes #1327.
8caf7fd
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.