Repository navigation
Consistent failure of test-http-client-timeout-option-with-agent #25746
Description
Activity
Do you see the failure when you run
tools/test.py test/parallel/test-http-client-timeout-option-with-agent.jsor only when you runmake testor what exactly?Seems to me like maybe
const start = Date.now();should be moved immediately beforeconst req = http.request(options);instead of coming immediately after it? If that seems about right to you, does that change fix things for you?What happens with this one (which also switches to
process.hrtime.bigint())?:'use strict'; const common = require('../common'); // Test that when http request uses both timeout and agent, // timeout will work as expected. const assert = require('assert'); const http = require('http'); const HTTP_AGENT_TIMEOUT = 1000; const HTTP_CLIENT_TIMEOUT = 3000; const agent = new http.Agent({ timeout: HTTP_AGENT_TIMEOUT }); const options = { method: 'GET', port: undefined, host: '127.0.0.1', path: '/', timeout: HTTP_CLIENT_TIMEOUT, agent, }; const server = http.createServer(() => { // Never respond. }); server.listen(0, options.host, () => { doRequest(); }); function doRequest() { options.port = server.address().port; const start = process.hrtime.bigint(); const req = http.request(options); req.on('error', () => { // This space is intentionally left blank. }); req.on('close', common.mustCall(() => server.close())); let timeout_events = 0; req.on('timeout', common.mustCall(() => { timeout_events += 1; const duration = process.hrtime.bigint() - start; // The timeout event cannot be precisely timed. It will delay // some number of milliseconds. assert.ok( duration >= BigInt(HTTP_CLIENT_TIMEOUT * 1e6), `duration ${duration}ms less than timeout ${HTTP_CLIENT_TIMEOUT}ms` ); })); req.end(); setTimeout(() => { req.destroy(); assert.strictEqual(timeout_events, 1); // Ensure the `timeout` event fired only once. }, common.platformTimeout(HTTP_CLIENT_TIMEOUT * 2)); }
I'll be mildly surprised if that actually fixes it for you, but if so, hey, great.
Another thing to try might be to see if the test passes if you set the agent timeout constant to 3000 instead of 1000. If the test starts passing then, that suggests that this may be a legitimate bug and not a problem with the test.
@nodejs/http
I think switching to
process.hrtime.bigint()is probably a good change anyway, so I'd be inclined to do that and call it good. Thoughts? @nodejs/testing- added a commit that references this issue
on Jan 28, 2019 That's Mojave, right? You didn't update very recently, did you?
- added a commit that references this issue
on Jan 30, 2019 - added a commit that references this issue
on Jan 30, 2019
Every time I run the suite I see this failure:
cc @Trott who messed with the timers in this test last