do not stuck if target returns always 503

fastify · Sep 29, 2021 · 5628d50 · 5628d50
1 parent 8535e7c
commit 5628d50
Show file tree

Hide file tree

Showing 2 changed files with 41 additions and 16 deletions.
diff --git a/index.js b/index.js
@@ -212,8 +212,9 @@ function isFastifyMultipartRegistered (fastify) {
   return fastify.hasContentTypeParser('multipart') && fastify.hasRequestDecorator('multipart')
 }
 
-function createRequestRetry (requestImpl, reply, retriesCount, retryOnError, retryOnCode = 503) {
+function createRequestRetry (requestImpl, reply, retriesCount, retryOnError) {
   function requestRetry (req, cb) {
+    const MAX_RETRIES_ON_503 = 10
     let retries = 0
 
     function run () {
@@ -224,11 +225,13 @@ function createRequestRetry (requestImpl, reply, retriesCount, retryOnError, ret
         if (res && res.headers['retry-after']) {
           retryAfter = res.headers['retry-after']
         }
-
         if (!reply.sent) {
           // always retry on 503 errors
-          if (res && res.statusCode === retryOnCode && req.method === 'GET') {
-            return retry(retryAfter)
+          if (res && res.statusCode === 503 && req.method === 'GET') {
+            if (retriesCount === 0 && retries < MAX_RETRIES_ON_503) {
+              // we should stop at some point
+              return retry(retryAfter)
+            }
           } else if (retriesCount > retries && err && err.code === retryOnError) {
             return retry(retryAfter)
           }

diff --git a/test/retry-on-503.js b/test/retry-on-503.js
@@ -6,27 +6,24 @@ const From = require('..')
 const http = require('http')
 const got = require('got')
 
-let requestCount = 0
-
-function createTargetServer (withRetryAfterHeader) {
+function createTargetServer (withRetryAfterHeader, stopAfter = 1) {
+  let requestCount = 0
   return http.createServer((req, res) => {
-    console.log('request', requestCount)
-    if (requestCount++ === 0) {
+    if (requestCount++ < stopAfter) {
       res.statusCode = 503
       res.setHeader('Content-Type', 'text/plain')
       if (withRetryAfterHeader) {
-        res.setHeader('Retry-After', 1000)
+        res.setHeader('Retry-After', 500)
       }
-      return res.end('This Service Unavailable')
+      return res.end('This Service is Unavailable')
     }
     res.statusCode = 205
     res.setHeader('Content-Type', 'text/plain')
-    return res.end('Hello World Twice!')
+    return res.end(`Hello World ${requestCount}!`)
   })
 }
 
 test('Should retry on 503 HTTP error', async function (t) {
-  t.teardown(() => { requestCount = 0 })
   t.plan(4)
   const target = createTargetServer()
   await target.listen(0)
@@ -48,12 +45,11 @@ test('Should retry on 503 HTTP error', async function (t) {
   const res = await got.get(`http://localhost:${instance.server.address().port}`)
   t.equal(res.headers['content-type'], 'text/plain')
   t.equal(res.statusCode, 205)
-  t.equal(res.body.toString(), 'Hello World Twice!')
+  t.equal(res.body.toString(), 'Hello World 2!')
   t.pass()
 })
 
 test('Should retry on 503 HTTP error with Retry-After response header', async function (t) {
-  t.teardown(() => { requestCount = 0 })
   t.plan(4)
   const target = createTargetServer(true)
   await target.listen(0)
@@ -75,6 +71,32 @@ test('Should retry on 503 HTTP error with Retry-After response header', async fu
   const res = await got.get(`http://localhost:${instance.server.address().port}`)
   t.equal(res.headers['content-type'], 'text/plain')
   t.equal(res.statusCode, 205)
-  t.equal(res.body.toString(), 'Hello World Twice!')
+  t.equal(res.body.toString(), 'Hello World 2!')
+  t.pass()
+})
+
+test('Should abort if server is always returning 503', async function (t) {
+  t.plan(4)
+  const target = createTargetServer(true, Number.MAX_SAFE_INTEGER)
+  await target.listen(0)
+  t.teardown(target.close.bind(target))
+
+  const instance = Fastify()
+
+  instance.register(From, {
+    base: `http://localhost:${target.address().port}`
+  })
+
+  instance.get('/', (request, reply) => {
+    reply.from()
+  })
+
+  t.teardown(instance.close.bind(instance))
+  await instance.listen(0)
+
+  const res = await got.get(`http://localhost:${instance.server.address().port}`)
+  t.equal(res.headers['content-type'], 'text/plain')
+  t.equal(res.statusCode, 503)
+  t.equal(res.body.toString(), 'This Service is Unavailable')
   t.pass()
 })