microsoft / typed-rest-client

Node Rest and Http Clients with typings for use with TypeScript
Other
675 stars 118 forks source link

ECONNRESET Errors not handled by retry mechanism #220

Closed mastrzyz closed 4 years ago

mastrzyz commented 4 years ago
  1. Please check our current Issues to see if someone already reported this https://github.com/Microsoft/typed-rest-client/issues
  2. Take a look at our Instructions for logging issues https://github.com/Microsoft/typed-rest-client/blob/master/CONTRIBUTING.md#instructions-for-logging-issues

Environment

Node version: v12.14.1 Npm version: 6.13.4 OS and version: OSX Catalin typed-rest-client version: 1.7.3

Issue Description

The retry mechanism takes in account if we are able to send a request correctly and get a status code, but incase we get a "ECONNRESET" back from the code 'http' or 'https' node library we will reject the main promise and raise an exception

Expected behaviour

We retry ECONNRESET errors the same way we retry non successful status code errors

ie:


try{
response = await this.requestRaw(info, data);
} catch(e){
// add handling to look for `ECONNRESET` ? and retry
}

### Actual behaviour
<!--- Tell us what happens instead -->

We fail immediately and do not perform any retries

### Steps to reproduce
This is very tricky for a repro since this relies on a server closing the connection. ADO Api sometimes seems to do this for us.
### Logs
<!-- Any logs you have/stack traces/etc. -->
mastrzyz commented 4 years ago

I'm willing to help out with this issue, just want to confirm with folks we want to retry on ECONNRESET as well

damccorm commented 4 years ago

Retrying this makes sense to me, I'd be happy to review :)

mattcobb commented 4 years ago

Don't know if it actually works, but the SO post talks about a way to trigger the econnreset client side for testing. https://stackoverflow.com/questions/22836424/simulate-an-econnreset-error-on-a-node-js-net-socket-instance

mastrzyz commented 4 years ago

@damccorm What would be needed to get this in a released version, its been a long time for a release for this repo.