fix: anchor retry backoff reset to the first event of each connection

README.md

@@ -90,13 +90,13 @@ The following additional events may be emitted:
 
 By default, `EventSource` automatically attempts to reconnect if a connection attempt fails or if an existing connection is broken. To prevent a flood of requests, there is always a delay before retrying the connection; the default value for this is 1000 milliseconds.
 
-For backward compatibility, the default behavior is to use the same delay each time. However, it is highly recommended that you enable both exponential backoff (the delay doubles on each successive retry, up to a configurable maximum) and jitter (a random amount is subtracted from each delay), so that if a server outage causes clients to all lose their connections at the same time they will not all retry at the same time. The backoff can also be configured to reset back to the initial delay if the stream has remained active for some amount of time.
+For backward compatibility, the default behavior is to use the same delay each time. However, it is highly recommended that you enable both exponential backoff (the delay doubles on each successive retry, up to a configurable maximum) and jitter (a random amount is subtracted from each delay), so that if a server outage causes clients to all lose their connections at the same time they will not all retry at the same time. The backoff can also be configured to reset back to the initial delay once a connection has remained active for some amount of time, measured from the first event received on that connection.
 
 ```javascript
 var eventSourceInitDict = {
   initialRetryDelayMillis: 2000,   // sets initial retry delay to 2 seconds
   maxBackoffMillis: 30000,         // enables backoff, with a maximum of 30 seconds
-  retryResetIntervalMillis: 60000, // backoff will reset to initial level if stream got an event at least 60 seconds before failing
+  retryResetIntervalMillis: 60000, // backoff resets to the initial delay once a connection has been active for 60 seconds, measured from its first event
   jitterRatio: 0.5                 // each delay will be reduced by a randomized jitter of up to 50%
 };
 ```

lib/eventsource.js

@@ -83,6 +83,8 @@ function EventSource (url, eventSourceInitDict) {
     config.jitterRatio ? retryDelay.defaultJitter(config.jitterRatio) : null
   )
 
+  var goodSinceAnchored = false
+
   var streamOriginUrl = new URL(url).origin
 
   let reconnectTimer
@@ -239,6 +241,7 @@ function EventSource (url, eventSourceInitDict) {
       data = ''
       eventName = ''
       eventId = undefined
+      goodSinceAnchored = false
 
       readyState = EventSource.OPEN
       res.on('close', function () {
@@ -383,7 +386,12 @@ function EventSource (url, eventSourceInitDict) {
   }
 
   function receivedEvent (event) {
-    retryDelayStrategy.setGoodSince(new Date().getTime())
+    // The reset interval measures how long the current connection has been delivering data,
+    // so the "good since" time is anchored to the first event of each connection.
+    if (!goodSinceAnchored) {
+      goodSinceAnchored = true
+      retryDelayStrategy.setGoodSince(new Date().getTime())
+    }
     _emit(event)
   }
 

test/eventsource_test.js

@@ -717,9 +717,9 @@ describe('Reconnection', () => {
 })
 
 describe('retry delay', () => {
-  async function verifyDelays (options, count, delaysAssertion) {
+  async function verifyDelaysWithHandler (options, handler, count, delaysAssertion) {
     await withServer(async server => {
-      server.byDefault(TestHttpHandlers.respond(500))
+      server.byDefault(handler)
 
       await withEventSource(server, options, async es => {
         const delays = new AsyncQueue()
@@ -734,6 +734,10 @@ describe('retry delay', () => {
     })
   }
 
+  async function verifyDelays (options, count, delaysAssertion) {
+    await verifyDelaysWithHandler(options, TestHttpHandlers.respond(500), count, delaysAssertion)
+  }
+
   it('uses constant delay by default', async () => {
     const delay = 5
     await verifyDelays(
@@ -771,6 +775,85 @@ describe('retry delay', () => {
       }
     )
   })
+
+  it('resets backoff once a connection has been active for the reset interval, measured from its first event', async () => {
+    const delay = 5
+    const resetInterval = 150
+
+    // The first two connections fail immediately, so the backoff progresses. The third
+    // connection delivers events continuously for longer than the reset interval and then
+    // drops; the delay after it must restart at the initial value even though the last
+    // event arrived only moments before the connection dropped.
+    let connection = 0
+    const handler = (req, res) => {
+      connection++
+      if (connection === 3) {
+        res.writeHead(200, { 'Content-Type': 'text/event-stream' })
+        res.write('data: one\n\n')
+        let writes = 0
+        const timer = setInterval(() => {
+          writes++
+          if (writes > 6) {
+            clearInterval(timer)
+            res.destroy()
+          } else {
+            res.write('data: more\n\n')
+          }
+        }, 40)
+      } else {
+        res.writeHead(500)
+        res.end()
+      }
+    }
+
+    await verifyDelaysWithHandler(
+      { initialRetryDelayMillis: delay, maxBackoffMillis: 1000, retryResetIntervalMillis: resetInterval },
+      handler,
+      3,
+      function (delays) {
+        assert.deepEqual(delays, [ delay, delay * 2, delay ])
+      }
+    )
+  })
+
+  it('does not reset backoff for a connection that stays open past the reset interval without delivering events', async () => {
+    const delay = 5
+    const resetInterval = 150
+
+    // The third connection stays open past the reset interval but sends only comment
+    // heartbeats, never an event. An open connection that has delivered no data does not
+    // count as healthy, so the backoff keeps progressing.
+    let connection = 0
+    const handler = (req, res) => {
+      connection++
+      if (connection === 3) {
+        res.writeHead(200, { 'Content-Type': 'text/event-stream' })
+        res.write(':hi\n')
+        let writes = 0
+        const timer = setInterval(() => {
+          writes++
+          if (writes > 6) {
+            clearInterval(timer)
+            res.destroy()
+          } else {
+            res.write(':hi\n')
+          }
+        }, 40)
+      } else {
+        res.writeHead(500)
+        res.end()
+      }
+    }
+
+    await verifyDelaysWithHandler(
+      { initialRetryDelayMillis: delay, maxBackoffMillis: 1000, retryResetIntervalMillis: resetInterval },
+      handler,
+      3,
+      function (delays) {
+        assert.deepEqual(delays, [ delay, delay * 2, delay * 4 ])
+      }
+    )
+  })
 })
 
 describe('readyState', function () {