From 28edaeb416ed115b543e8e71ca94811aac4d8a60 Mon Sep 17 00:00:00 2001 From: isaacgr Date: Sat, 7 Oct 2023 08:06:46 -0400 Subject: [PATCH 1/6] Call serverDisconnected only if connection was closed without error - it should otherwise be handled by the .on('error') event - Refs #123 --- src/client/protocol/base.js | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/client/protocol/base.js b/src/client/protocol/base.js index 365f096..482a524 100644 --- a/src/client/protocol/base.js +++ b/src/client/protocol/base.js @@ -95,9 +95,13 @@ class JsonRpcClientProtocol { this.listen(); resolve(this.server); }); - this.connector.on("error", error => this._onConnectionFailed(error, resolve, reject)); - this.connector.on("close", () => { - this.factory.emit("serverDisconnected"); + this.connector.on("error", (error) => + this._onConnectionFailed(error, resolve, reject) + ); + this.connector.on("close", (hadError) => { + if (!hadError) { + this.factory.emit("serverDisconnected"); + } }); } From 0608b9b5633038c404b6a1d914cbcf09b6703270 Mon Sep 17 00:00:00 2001 From: isaacgr Date: Sat, 7 Oct 2023 08:07:42 -0400 Subject: [PATCH 2/6] Tests to cover #123 --- ...reconnect-after-serverDisconnected.test.js | 37 +++++++++++++++++++ 1 file changed, 37 insertions(+) create mode 100644 tests/issues/#123-reconnect-after-serverDisconnected.test.js diff --git a/tests/issues/#123-reconnect-after-serverDisconnected.test.js b/tests/issues/#123-reconnect-after-serverDisconnected.test.js new file mode 100644 index 0000000..ec94628 --- /dev/null +++ b/tests/issues/#123-reconnect-after-serverDisconnected.test.js @@ -0,0 +1,37 @@ +const { expect } = require("chai"); +const Jaysonic = require("../../src"); +const { server } = require("../test-server"); +const intercept = require("intercept-stdout"); + +const tcpclient = new Jaysonic.client.tcp({ + retries: 10 +}); + +describe("#123 Reconnect after serverDisconnected", () => { + before(async () => { + await server.listen(); + await tcpclient.connect(); + }); + after(async () => { + await tcpclient.end(); + }); + it("should attempt to reconnect at the default rate after receiving a serverDisconnected", async () => { + let reconnectAttempts = 0; + let capturedText = ""; + const unhook = intercept((text) => { + capturedText += text; + }); + tcpclient.serverDisconnected(async () => { + reconnectAttempts += 1; + tcpclient.pcolInstance = null; + await tcpclient.connect(); + }); + await server.close(); + await new Promise((r) => setTimeout(r, 10000)); + unhook(); + expect(capturedText).to.equal( + `Failed to connect. Address [127.0.0.1:8100]. Retrying. 9 attempts left.\nFailed to connect. Address [127.0.0.1:8100]. Retrying. 8 attempts left.\n` + ); + expect(reconnectAttempts).to.equal(1); + }).timeout(30000); +}); From 9d5daad36496d8cfed4a644b99bdb173fb0ef796 Mon Sep 17 00:00:00 2001 From: isaacgr Date: Sat, 7 Oct 2023 08:21:37 -0400 Subject: [PATCH 3/6] Run CI jobs on node 18 and 20 --- .github/workflows/main.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index 7239522..b893ce3 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -16,7 +16,7 @@ jobs: runs-on: ubuntu-latest strategy: matrix: - node-version: [10.x, 12.x, 14.x, 16.x] + node-version: [10.x, 12.x, 14.x, 16.x, 18.x, 20.x] # Steps represent a sequence of tasks that will be executed as part of the job steps: # Checks-out your repository under $GITHUB_WORKSPACE, so your job can access it From d89ef8294f7bbc62546e9eb6ac387cd73c52c51c Mon Sep 17 00:00:00 2001 From: isaacgr Date: Sat, 7 Oct 2023 08:28:21 -0400 Subject: [PATCH 4/6] Dont test on node 18 and 20 for now --- .github/workflows/main.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml index b893ce3..7239522 100644 --- a/.github/workflows/main.yml +++ b/.github/workflows/main.yml @@ -16,7 +16,7 @@ jobs: runs-on: ubuntu-latest strategy: matrix: - node-version: [10.x, 12.x, 14.x, 16.x, 18.x, 20.x] + node-version: [10.x, 12.x, 14.x, 16.x] # Steps represent a sequence of tasks that will be executed as part of the job steps: # Checks-out your repository under $GITHUB_WORKSPACE, so your job can access it From aa653fb075d3e43df9b9b99b4e2eeffb366595b2 Mon Sep 17 00:00:00 2001 From: isaacgr Date: Sat, 7 Oct 2023 17:55:54 -0400 Subject: [PATCH 5/6] Reset the remainingRetries after a successful connection --- src/client/protocol/base.js | 1 + 1 file changed, 1 insertion(+) diff --git a/src/client/protocol/base.js b/src/client/protocol/base.js index 482a524..717a5a6 100644 --- a/src/client/protocol/base.js +++ b/src/client/protocol/base.js @@ -91,6 +91,7 @@ class JsonRpcClientProtocol { this.connector.connect(this.server); this.connector.setEncoding("utf8"); this.connector.on("connect", () => { + this.factory.remainingRetries = this.factory.options.retries; this.listener = this.connector; this.listen(); resolve(this.server); From f8a6abe110d4618e349f1ccd186c6ad1a768f054 Mon Sep 17 00:00:00 2001 From: isaacgr Date: Sat, 7 Oct 2023 17:56:26 -0400 Subject: [PATCH 6/6] Update test with some comments --- .../#123-reconnect-after-serverDisconnected.test.js | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/tests/issues/#123-reconnect-after-serverDisconnected.test.js b/tests/issues/#123-reconnect-after-serverDisconnected.test.js index ec94628..6bcfc82 100644 --- a/tests/issues/#123-reconnect-after-serverDisconnected.test.js +++ b/tests/issues/#123-reconnect-after-serverDisconnected.test.js @@ -8,11 +8,10 @@ const tcpclient = new Jaysonic.client.tcp({ }); describe("#123 Reconnect after serverDisconnected", () => { - before(async () => { + beforeEach(async () => { await server.listen(); - await tcpclient.connect(); }); - after(async () => { + afterEach(async () => { await tcpclient.end(); }); it("should attempt to reconnect at the default rate after receiving a serverDisconnected", async () => { @@ -26,9 +25,14 @@ describe("#123 Reconnect after serverDisconnected", () => { tcpclient.pcolInstance = null; await tcpclient.connect(); }); + await tcpclient.connect(); await server.close(); await new Promise((r) => setTimeout(r, 10000)); unhook(); + // first re-connect attempt is done without error + // second is done with error and does not trigger the serverDisconnected call + // after which the test should timeout + // 2 attempts, 5s apart 10s total expect(capturedText).to.equal( `Failed to connect. Address [127.0.0.1:8100]. Retrying. 9 attempts left.\nFailed to connect. Address [127.0.0.1:8100]. Retrying. 8 attempts left.\n` );