wip

Author: simone-sanfratello

README.md

@@ -232,15 +232,15 @@ The default implementation forwards the `cookie` header.
 The `wsReconnect` option contains the configuration for the WebSocket reconnection feature; is an object with the following properties:
 
 - `pingInterval`: The interval between ping messages in ms (default: `30_000`).
-- `maxReconnectionRetries`: The maximum number of reconnection attempts (default: `3`). The counter is reset when the connection is established.
+- `maxReconnectionRetries`: The maximum number of reconnection retries (`1` to `Infinity`, default: `Infinity`).
 - `reconnectInterval`: The interval between reconnection attempts in ms (default: `1_000`).
 - `reconnectDecay`: The decay factor for the reconnection interval (default: `1.5`).
-- `connectionTimeout`: The timeout for the connection in ms (default: `5_000`).
-- `reconnectOnClose`: Whether to reconnect on close, as long as the source connection is active (default: `false`).
+- `connectionTimeout`: The timeout for establishing the connection in ms (default: `5_000`).
+- `reconnectOnClose`: Whether to reconnect on close, as long as the connection from the related client to the proxy is active (default: `false`).
 - TODO logs option?
 
-Reconnection feature detects and closes broken connections and reconnects automatically, see [how to detect and close broken connections](https://github.com/websockets/ws#how-to-detect-and-close-broken-connections); the mechanism is based on ping/pong messages.
-It verifies the connection status from the service to the target.
+Reconnection feature detects and closes broken connections and reconnects automatically, see [how to detect and close broken connections](https://github.com/websockets/ws#how-to-detect-and-close-broken-connections).
+The connection is considered broken if the target does not respond to the ping messages or no data is received from the target.
 
 Example:
 

index.js

@@ -43,7 +43,6 @@ function waitConnection (socket, write) {
   }
 }
 
-// TODO merge with waitConnection
 function waitForConnection (target, timeout) {
   return new Promise((resolve, reject) => {
     const timeoutId = setTimeout(() => {
@@ -145,7 +144,7 @@ function proxyWebSocketsWithReconnection (logger, source, target, options, targe
     if (source.isAlive && (target.broken || options.reconnectOnClose)) {
       target.isAlive = false
       target.removeAllListeners()
-      // TODO source.removeAllListeners()
+      // TODO FIXME! source.removeAllListeners()
       reconnect(logger, source, options, targetParams)
       return
     }
@@ -185,11 +184,17 @@ function proxyWebSocketsWithReconnection (logger, source, target, options, targe
   /* c8 ignore stop */
 
   // source WebSocket is already connected because it is created by ws server
-  target.on('message', (data, binary) => source.send(data, { binary }))
+  target.on('message', (data, binary) => {
+    target.isAlive = true
+    source.send(data, { binary })
+  })
   /* c8 ignore start */
   target.on('ping', data => source.ping(data))
   /* c8 ignore stop */
-  target.on('pong', data => source.pong(data))
+  target.on('pong', data => {
+    target.isAlive = true
+    source.pong(data)
+  })
   target.on('close', (code, reason) => {
     logger.warn({ target: targetParams.url, code, reason }, 'proxy ws target close event')
     close(code, reason)

src/options.js

@@ -1,7 +1,6 @@
 'use strict'
 
 const DEFAULT_PING_INTERVAL = 30_000
-const DEFAULT_MAX_RECONNECT_ATTEMPTS = 3
 const DEFAULT_MAX_RECONNECTION_RETRIES = Infinity
 const DEFAULT_RECONNECT_INTERVAL = 1_000
 const DEFAULT_RECONNECT_DECAY = 1.5
@@ -21,10 +20,10 @@ function validateOptions (options) {
     }
     wsReconnect.pingInterval = wsReconnect.pingInterval ?? DEFAULT_PING_INTERVAL
 
-    if (wsReconnect.maxReconnectionRetries !== undefined && (typeof wsReconnect.maxReconnectionRetries !== 'number' || wsReconnect.maxReconnectionRetries < 0)) {
-      throw new Error('wsReconnect.maxReconnectionRetries must be a non-negative number')
+    if (wsReconnect.maxReconnectionRetries !== undefined && (typeof wsReconnect.maxReconnectionRetries !== 'number' || wsReconnect.maxReconnectionRetries < 1)) {
+      throw new Error('wsReconnect.maxReconnectionRetries must be a number greater than or equal to 1')
     }
-    wsReconnect.maxReconnectionRetries = wsReconnect.maxReconnectionRetries ?? DEFAULT_MAX_RECONNECT_ATTEMPTS
+    wsReconnect.maxReconnectionRetries = wsReconnect.maxReconnectionRetries ?? DEFAULT_MAX_RECONNECTION_RETRIES
 
     if (wsReconnect.reconnectInterval !== undefined && (typeof wsReconnect.reconnectInterval !== 'number' || wsReconnect.reconnectInterval < 100)) {
       throw new Error('wsReconnect.reconnectInterval (ms) must be a number greater than or equal to 100')
@@ -53,7 +52,6 @@ function validateOptions (options) {
 module.exports = {
   validateOptions,
   DEFAULT_PING_INTERVAL,
-  DEFAULT_MAX_RECONNECT_ATTEMPTS,
   DEFAULT_MAX_RECONNECTION_RETRIES,
   DEFAULT_RECONNECT_INTERVAL,
   DEFAULT_RECONNECT_DECAY,

test/options.js

@@ -2,13 +2,53 @@
 
 const { test } = require('tap')
 const { validateOptions } = require('../src/options')
-
+const { DEFAULT_PING_INTERVAL, DEFAULT_MAX_RECONNECTION_RETRIES, DEFAULT_RECONNECT_INTERVAL, DEFAULT_RECONNECT_DECAY, DEFAULT_CONNECTION_TIMEOUT, DEFAULT_RECONNECT_ON_CLOSE } = require('../src/options')
 test('validateOptions', (t) => {
   const requiredOptions = {
     upstream: 'someUpstream'
   }
 
-  // TODO
+  t.throws(() => validateOptions({}), 'upstream must be specified')
+
+  t.throws(() => validateOptions({ ...requiredOptions, wsReconnect: { pingInterval: -1 } }), 'wsReconnect.pingInterval must be a non-negative number')
+  t.throws(() => validateOptions({ ...requiredOptions, wsReconnect: { pingInterval: '1' } }), 'wsReconnect.pingInterval must be a non-negative number')
+  t.doesNotThrow(() => validateOptions({ ...requiredOptions, wsReconnect: { pingInterval: 1 } }))
+
+  t.throws(() => validateOptions({ ...requiredOptions, wsReconnect: { maxReconnectionRetries: 0 } }), 'wsReconnect.maxReconnectionRetries must be a number greater than or equal to 1')
+  t.throws(() => validateOptions({ ...requiredOptions, wsReconnect: { maxReconnectionRetries: -1 } }), 'wsReconnect.maxReconnectionRetries must be a number greater than or equal to 1')
+  t.throws(() => validateOptions({ ...requiredOptions, wsReconnect: { maxReconnectionRetries: '1' } }), 'wsReconnect.maxReconnectionRetries must be a number greater than or equal to 1')
+  t.doesNotThrow(() => validateOptions({ ...requiredOptions, wsReconnect: { maxReconnectionRetries: 1 } }))
+
+  t.throws(() => validateOptions({ ...requiredOptions, wsReconnect: { reconnectInterval: 0 } }), 'wsReconnect.reconnectInterval (ms) must be a number greater than or equal to 100')
+  t.throws(() => validateOptions({ ...requiredOptions, wsReconnect: { reconnectInterval: -1 } }), 'wsReconnect.reconnectInterval (ms) must be a number greater than or equal to 100')
+  t.throws(() => validateOptions({ ...requiredOptions, wsReconnect: { reconnectInterval: '1' } }), 'wsReconnect.reconnectInterval (ms) must be a number greater than or equal to 100')
+  t.doesNotThrow(() => validateOptions({ ...requiredOptions, wsReconnect: { reconnectInterval: 100 } }))
+
+  t.throws(() => validateOptions({ ...requiredOptions, wsReconnect: { reconnectDecay: 0 } }), 'wsReconnect.reconnectDecay must be a number greater than or equal to 1')
+  t.throws(() => validateOptions({ ...requiredOptions, wsReconnect: { reconnectDecay: -1 } }), 'wsReconnect.reconnectDecay must be a number greater than or equal to 1')
+  t.throws(() => validateOptions({ ...requiredOptions, wsReconnect: { reconnectDecay: '1' } }), 'wsReconnect.reconnectDecay must be a number greater than or equal to 1')
+  t.doesNotThrow(() => validateOptions({ ...requiredOptions, wsReconnect: { reconnectDecay: 1 } }))
+
+  t.throws(() => validateOptions({ ...requiredOptions, wsReconnect: { connectionTimeout: -1 } }), 'wsReconnect.connectionTimeout must be a non-negative number')
+  t.throws(() => validateOptions({ ...requiredOptions, wsReconnect: { connectionTimeout: '1' } }), 'wsReconnect.connectionTimeout must be a non-negative number')
+  t.doesNotThrow(() => validateOptions({ ...requiredOptions, wsReconnect: { connectionTimeout: 1 } }))
+
+  t.throws(() => validateOptions({ ...requiredOptions, wsReconnect: { reconnectOnClose: '1' } }), 'wsReconnect.reconnectOnClose must be a boolean')
+  t.doesNotThrow(() => validateOptions({ ...requiredOptions, wsReconnect: { reconnectOnClose: true } }))
+
+  t.doesNotThrow(() => validateOptions({ ...requiredOptions, wsReconnect: { pingInterval: 1, maxReconnectionRetries: 1, reconnectInterval: 100, reconnectDecay: 1, connectionTimeout: 1, reconnectOnClose: true } }))
+
+  t.equal(validateOptions({ ...requiredOptions, wsReconnect: { } }), {
+    ...requiredOptions,
+    wsReconnect: {
+      pingInterval: DEFAULT_PING_INTERVAL,
+      maxReconnectionRetries: DEFAULT_MAX_RECONNECTION_RETRIES,
+      reconnectInterval: DEFAULT_RECONNECT_INTERVAL,
+      reconnectDecay: DEFAULT_RECONNECT_DECAY,
+      connectionTimeout: DEFAULT_CONNECTION_TIMEOUT,
+      reconnectOnClose: DEFAULT_RECONNECT_ON_CLOSE
+    }
+  })
 
   t.end()
 })

test/ws-reconnect.js

@@ -11,11 +11,11 @@ const pinoTest = require('pino-test')
 const pino = require('pino')
 const proxyPlugin = require('../')
 
-function waitForLogMessage(loggerSpy, message, max = 100) {
+function waitForLogMessage (loggerSpy, message, max = 100) {
   return new Promise((resolve, reject) => {
     let count = 0
     const fn = (received) => {
-      console.log(received)
+      // console.log(received)
 
       if (received.msg === message) {
         loggerSpy.off('data', fn)
@@ -31,7 +31,7 @@ function waitForLogMessage(loggerSpy, message, max = 100) {
   })
 }
 
-async function createServices({ t, upstream, wsReconnectOptions, wsTargetOptions, wsServerOptions }) {
+async function createServices ({ t, upstream, wsReconnectOptions, wsTargetOptions, wsServerOptions }) {
   const targetServer = createServer()
   const targetWs = new WebSocket.Server({ server: targetServer, ...wsTargetOptions })
 
@@ -70,7 +70,7 @@ async function createServices({ t, upstream, wsReconnectOptions, wsTargetOptions
     upstream
   }
 }
-/*
+
 test('should use ping/pong to verify connection is alive - from source (server on proxy) to target', async (t) => {
   const wsReconnectOptions = { pingInterval: 100, reconnectInterval: 100, maxReconnectionRetries: 1 }
 
@@ -110,8 +110,6 @@ test('should reconnect on broken connection', async (t) => {
   await waitForLogMessage(loggerSpy, 'proxy ws reconnected')
 
   // TODO fix with source.removeAllListeners
-
-  t.end()
 })
 
 test('should not reconnect after max retries', async (t) => {
@@ -140,48 +138,28 @@ test('should not reconnect after max retries', async (t) => {
   await waitForLogMessage(loggerSpy, 'proxy ws target close event')
   await waitForLogMessage(loggerSpy, 'proxy ws reconnect error')
   await waitForLogMessage(loggerSpy, 'proxy ws failed to reconnect! No more retries')
-
-  t.end()
 })
-*/
-
-test('should not reconnect because of connection timeout', async (t) => {
-  const wsReconnectOptions = { pingInterval: 150, reconnectInterval: 100, maxReconnectionRetries: 1, connectionTimeout: 100 }
-
-  const { target, loggerSpy } = await createServices({ t, wsReconnectOptions, wsTargetOptions: { autoPong: false } })
 
-  let breakConnection = true
+test('should reconnect on regular target connection close', async (t) => {
+  const wsReconnectOptions = { pingInterval: 200, reconnectInterval: 100, maxReconnectionRetries: 1, reconnectOnClose: false }
 
-  target.ws.on('upgrade', (request, socket, head) => {
-    console.log('upgrade')
-  })
+  const { target, loggerSpy } = await createServices({ t, wsReconnectOptions })
 
   target.ws.on('connection', async (socket) => {
     socket.on('ping', async () => {
-      // add latency to break the connection once
-      if (breakConnection) {
-        await wait(wsReconnectOptions.pingInterval * 2)
-        breakConnection = false
-      }
       socket.pong()
     })
-  })
-
-  await waitForLogMessage(loggerSpy, 'proxy ws connection is broken')
 
-  target.ws.close()
-  target.server.close()
+    await wait(1_000)
+    socket.close()
+  })
 
   await waitForLogMessage(loggerSpy, 'proxy ws target close event')
-  await waitForLogMessage(loggerSpy, 'proxy ws reconnect error')
-  await waitForLogMessage(loggerSpy, 'proxy ws failed to reconnect! No more retries')
-
-  t.end()
+  await waitForLogMessage(loggerSpy, 'proxy ws close link')
 })
 
-// TODO reconnect regular close
-
 /*
+TODO fix!
 test('should reconnect with retry', async (t) => {
   const wsReconnectOptions = { pingInterval: 150, reconnectInterval: 100, reconnectOnClose: true }
 
@@ -211,9 +189,5 @@ test('should reconnect with retry', async (t) => {
   await waitForLogMessage(loggerSpy, 'proxy ws target close event')
   await waitForLogMessage(loggerSpy, 'proxy ws reconnect error')
   await waitForLogMessage(loggerSpy, 'proxy ws reconnected')
-
-  t.end()
 })
 */
-
-// TODO reconnectOnClose but close all on shutdown