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

retry on ECONNREST broken #3131

Closed
themez opened this issue Mar 19, 2019 · 2 comments
Closed

retry on ECONNREST broken #3131

themez opened this issue Mar 19, 2019 · 2 comments
Labels

Comments

@themez
Copy link

themez commented Mar 19, 2019

Summary

I found ECONNRESET is handled by retry in this PR #148 , but since forever agent is only used in Nodejs v0.xx, it's not working currently.

Simplest Example to Reproduce

const Agent = require("http").Agent;

const agent = new Agent({ keepAlive: true });
const request = require('request') 
http
  .createServer((req, res) => {
    res.write("foo\n");
    res.end();
  })
  .listen(3000);

setInterval(() => {
  request.get('http://localhost:3000', { agent }, (err, res) => {      
      if (err) console.error(err)
  });
}, 5000);

Expected Behavior

No error.

Current Behavior

Encounter an error if wait 10~20 seconds.

{ Error: socket hang up
    at createHangUpError (_http_client.js:342:15)
    at Socket.socketCloseListener (_http_client.js:377:23)
    at emitOne (events.js:121:20)
    at Socket.emit (events.js:211:7)
    at TCP._handle.close [as _onclose] (net.js:561:12) code: 'ECONNRESET' }

Possible Solution

Maybe make req as resued in http_agent, or create a wrapper for default agent.

Context

Your Environment

software version
request 2.88.0
node 10.12.0
npm 6.4.1
Operating System MacOS Mojave 10.14.1
starkwang pushed a commit to nodejs/node that referenced this issue Oct 12, 2019
Set ClientRequest.reusedSocket property when reusing socket for request,
so user can handle retry base on wether the request is reusing a socket.

Refs: request/request#3131

PR-URL: #29715
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Weijia Wang <starkwang@126.com>
@themez themez mentioned this issue Oct 14, 2019
3 tasks
@themez
Copy link
Author

themez commented Oct 14, 2019

Since nodejs/node/pull/29715 is landed, this issue could be fixed.

targos pushed a commit to nodejs/node that referenced this issue Jan 8, 2020
Set ClientRequest.reusedSocket property when reusing socket for request,
so user can handle retry base on wether the request is reusing a socket.

Refs: request/request#3131

PR-URL: #29715
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Weijia Wang <starkwang@126.com>
BethGriggs pushed a commit to nodejs/node that referenced this issue Feb 6, 2020
Set ClientRequest.reusedSocket property when reusing socket for request,
so user can handle retry base on wether the request is reusing a socket.

Refs: request/request#3131

PR-URL: #29715
Reviewed-By: Matteo Collina <matteo.collina@gmail.com>
Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
Reviewed-By: Weijia Wang <starkwang@126.com>
@stale
Copy link

stale bot commented Oct 17, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label Oct 17, 2020
@stale stale bot closed this as completed Oct 31, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant