AsyncWrap: HTTP has no handle context

[AndreasMadsen]

When creating a simple HTTPserver and connecting to it one looses the handle context. Meaning there is no way to know what handle or callback created the new handle.

'use strict';
const http = require('http');
const server = http.createServer(function (req, res) {
 res.end('hallo world');
});

server.listen(0, 'localhost', function () {
 const addr = server.address();
 const req = http.get(`http://${addr.address}:${addr.port}`, function (res) {
 res.resume();
 res.once('end', server.close.bind(server));
 });
});

Note that this issues exists even after applying #3216 (adds parent to init hook)

complete test case: https://gist.github.com/AndreasMadsen/f56bbdbcd18a2c6358f3
dprof dump: http://bl.ocks.org/AndreasMadsen/raw/6c460eb0e7d6eeadb31a/

/cc @trevnorris

[trevnorris]

Unfortunately getting around this requires knowing implementation details, but here's an example:

'use strict';
const http = require('http');
const async_wrap = process.binding('async_wrap');
const print = process._rawDebug;
let client;

function init(p, parent) {
 if (parent) client = this;
}

function noop() { }

async_wrap.setupHooks(init, noop, noop);
async_wrap.enable();


const server = http.createServer(function(req, res) {
 // Print if this client matches the init w/ parent.
 print(req.client._handle === client);
 res.end('hello world');
});


server.listen(0, 'localhost', function() {
 // Disable listening to simply test.
 async_wrap.disable();
 http.get('http://localhost:' + server.address().port + '/', (res) => {
 res.resume();
 res.once('end', () => server.close());
 });
});

This demonstrates that the client handle for a given http request can still be retrieved. Though this is not ideal. I'll look into propagating this information during the HTTPParser phase.

One issue with your test is that it will exit on cases like SHUTDOWNWRAP. May want to focus your test using .disable() like I have above.

[trevnorris]

It should work the same as TCP if you listened for the http server's 'connection' event. Instead of the default 'request' event.

[AndreasMadsen]

One issue with your test is that it will exit on cases like SHUTDOWNWRAP. May want to focus your test using .disable() like I have above.

Sorry I don't follow you.

1. I don't understand your SHUTDOWNWRAP reference, could you provide an example.

2. I find that the first part of the test (https://gist.github.com/AndreasMadsen/f56bbdbcd18a2c6358f3#file-test_http-js-L7L52) i fairly generic. It just checks that there always is a callback or handle context for the new handle. I think that should always be the case, otherwise making a long-stack-trace tool would not be possible.

It should work the same as TCP if you listened for the http server's 'connection' event. Instead of the default 'request' event.

I think we are on the same page here, but just to be sure. One of my goals is to write a long-stack-trace tool, that doesn't require any extra interaction from the user except adding -r trace. Changing the the eventlistener from request to connection and feeding back the socket handle to the server handle, definitely qualifies as extra interaction.

[AndreasMadsen]

Closed by #5419

[trevnorris]

may need to reopen unless I can fix a regression the patch introduced...