fix: add option to reconnect if connack has an error code (#1948)

Co-authored-by: Daniel Lando <daniel.sorridi@gmail.com>

Author: mgabeler-lee-6rs

README.md

@@ -275,6 +275,13 @@ the final connection when it drops.
 The default value is 1000 ms which means it will try to reconnect 1 second
 after losing the connection.
 
+Note that this will only enable reconnects after either a connection timeout, or
+after a successful connection. It will _not_ (by default) enable retrying
+connections that are actively denied with a CONNACK error by the server.
+
+To also enable automatic reconnects for CONNACK errors, set
+`reconnectOnConnackError: true`.
+
 <a name="topicalias"></a>
 
 ## About Topic Alias Management
@@ -415,6 +422,8 @@ The arguments are:
     offline
   - `reconnectPeriod`: `1000` milliseconds, interval between two
     reconnections. Disable auto reconnect by setting to `0`.
+  - `reconnectOnConnackError`: `false`, whether to also reconnect if a CONNACK
+    is received with an error.
   - `connectTimeout`: `30 * 1000` milliseconds, time to wait before a
     CONNACK is received
   - `username`: the username required by your broker, if any

src/lib/client.ts

@@ -162,6 +162,12 @@ export interface IClientOptions extends ISecureClientOptions {
 	 * 1000 milliseconds, interval between two reconnections
 	 */
 	reconnectPeriod?: number
+	/**
+	 * Set to true to enable the reconnect period to apply if the initial
+	 * connection is denied with an error in the CONNACK packet, such as with an
+	 * authentication error.
+	 */
+	reconnectOnConnackError?: boolean
 	/**
 	 * 30 * 1000 milliseconds, time to wait before a CONNACK is received
 	 */

src/lib/handlers/connack.ts

@@ -49,6 +49,9 @@ const handleConnack: PacketHandler = (client, packet: IConnackPacket) => {
 			rc,
 		)
 		client.emit('error', err)
+		if (client.options.reconnectOnConnackError) {
+			client['_cleanUp'](true)
+		}
 	}
 }
 

test/node/abstract_client.ts

@@ -3335,6 +3335,42 @@ export default function abstractTest(server, config, ports) {
 				})
 		})
 
+		it('should reconnect on connack error if requested', function _test(t, done) {
+			let connackErrors = 0
+			const rcNotAuthorized = 135
+			const server2 = serverBuilder(config.protocol, (serverClient) => {
+				serverClient.on('connect', () => {
+					const rc = connackErrors === 0 ? rcNotAuthorized : 0
+					const connack =
+						version === 5 ? { reasonCode: rc } : { returnCode: rc }
+					serverClient.connack(connack)
+				})
+			})
+			teardownHelper.addServer(server2)
+			server2.listen(ports.PORTAND50, () => {
+				const client = connect({
+					host: 'localhost',
+					port: ports.PORTAND50,
+					reconnectPeriod: 10,
+					reconnectOnConnackError: true,
+				})
+				teardownHelper.addClient(client)
+				client.on('error', (err) => {
+					assert.instanceOf(err, ErrorWithReasonCode)
+					assert.equal(
+						(err as ErrorWithReasonCode).code,
+						rcNotAuthorized,
+					)
+					assert.equal(connackErrors, 0)
+					connackErrors++
+				})
+				client.on('connect', () => {
+					assert.equal(connackErrors, 1)
+					done()
+				})
+			})
+		})
+
 		it(
 			'should resend in-flight QoS 1 publish messages from the client',
 			{