tulios / mappersmith

is a lightweight rest client for node.js and the browser
https://www.npmjs.com/package/mappersmith
MIT License
333 stars 71 forks source link

Handle connection errors with `RetryMiddleware` #513

Open simonhaenisch opened 3 months ago

simonhaenisch commented 3 months ago

I tried basically this:

const { RetryMiddleware } = require('mappersmith/middleware')

RetryMiddleware({
  retries: 1,
  validateRetry: (res) => res.error() !== null,
})

but it doesn't even get to the middleware I think, because the exception throws before the middleware has a chance to handle it (ECONNREFUSED in my case but I want to handle ECONNRESET because it happens with keep-alive enabled when the server closes the connection).

Do I need to write my own variant of the RetryMiddleware that is also able to catch the promise rejection and retry on that as well?

simonhaenisch commented 3 months ago

I looked at the implementation of RetryMiddleware and it does allow you to retry on rejection as well. Reason it doesn't work for me is because the request method was POST not GET, and it only enables retries for get requests (not sure yet how to enable it for other methods).

simonhaenisch commented 3 months ago

After looking through the issues and reading through the ones explaining why you shouldn't retry anything except GET because of idempotency, I believe this is a new use-case for allowing retries on all http verbs... "this" being the type of exception where we know the request never reached the server, e.g. connection reset (ECONNRESET). In that case the method doesn't matter, the request is always retry-able?

Node.js has keep-alive enabled by default in newer versions (20+), and if the client's socket timeout is higher than the server's keep-alive timeout, you easily end up with a connection reset, thus it's not so uncommon anymore.