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

responses corresponding to requests queued by http agent are attached to incorrect domain #25460

Open
misterdjules opened this issue Jan 12, 2019 · 0 comments
Labels
domain Issues and PRs related to the domain subsystem. help wanted Issues that need assistance from volunteers or PRs that need help to proceed. http Issues or PRs related to the http subsystem.

Comments

@misterdjules
Copy link

  • Version: Current tip of master (cf9bcde), but possibly most versions.
  • Platform: All platforms.
  • Subsystem: domain, http.

Running the following code:

'use strict';

const common = require('../common');
const domain = require('domain');
const http = require('http');

const server = http.createServer((req, res) => {
    res.end();
});

function performHttpRequestWithDomain(domainId, agent, cb) {
    const d = domain.create();
    d._id = domainId;
    d.run(() => {
        const req = http.get({
            host: '127.0.0.1', port: server.address().port, agent
        }, res => {
            if (req.domain._id !== domainId) {
                console.log('req.domain._id !== domainId');
                console.log('req.domain._id:', req.domain._id);
                console.log('domainId:', domainId);
                process.exit(1);
            }

            if (req.domain._id !== res.domain._id) {
		        console.log('req.domain._id !== res.domain._id');
                console.log('req.domain._id:', req.domain._id);
                console.log('res.domain._id:', res.domain._id);
                process.exit(1);
            }

            res.on('data', () => {});
            res.on('end', cb);
        });

        req.end();
    });

    d.on('error', (domainErr) => {
        console.log('got domain error:', domainErr);
        process.exit(1);
    });
}

server.listen(0, '127.0.0.1', () => {
    const agent = new http.Agent({maxSockets: 1});
    let nbReqComplete = 0;

    function maybeCloseServer() {
        if (nbReqComplete === 2) {
            server.close();
        }
    }

    performHttpRequestWithDomain(1, agent, common.mustCall(() => {
        ++nbReqComplete;
        maybeCloseServer();
    }));

    performHttpRequestWithDomain(2, agent, common.mustCall(() => {
        ++nbReqComplete;
        maybeCloseServer();
    }));
});

exits with a status code of 1 when I'd expect it to exit with a status code of 0:

$ ./node test/parallel/test-agent-queued-request-domain.js 
req.domain._id !== res.domain._id
req.domain._id: 2
res.domain._id: 1
$ echo $?
1
$ 

In this repro, we basically force the http agent to queue the second request. When the first request finishes, the second request is assigned a socket from the agent's 'free' event handler.

However, it seems that event handler's execution is scheduled from within the lifecycle of the first request, and thus its active domain is the one of the first request.

As a result, when the parser for the response is instantiated and its corresponding async resource is initialized, it is attached to the first request's domain, and not to the active domain when the http request was originally created.

I'll see if I can put together a PR that fixes this.

@misterdjules misterdjules added http Issues or PRs related to the http subsystem. domain Issues and PRs related to the domain subsystem. labels Jan 12, 2019
@jasnell jasnell added the help wanted Issues that need assistance from volunteers or PRs that need help to proceed. label Jun 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
domain Issues and PRs related to the domain subsystem. help wanted Issues that need assistance from volunteers or PRs that need help to proceed. http Issues or PRs related to the http subsystem.
Projects
None yet
Development

No branches or pull requests

2 participants