This is an automated email from the ASF dual-hosted git repository.
CritasWang pushed a commit to branch develop
in repository https://gitbox.apache.org/repos/asf/iotdb-client-nodejs.git
The following commit(s) were added to refs/heads/develop by this push:
new df7c891 Tear down the connection when session setup fails in
Connection.open() (#20)
df7c891 is described below
commit df7c8911e2de16f303c0b6274dd6e76bcf6d7431
Author: ZIHAN DAI <[email protected]>
AuthorDate: Fri Jul 24 13:22:41 2026 +1000
Tear down the connection when session setup fails in Connection.open() (#20)
* Tear down the connection when session setup fails in Connection.open()
After the TCP connection is established (createConnection + createClient)
and
its 'error'/'close' listeners are registered, if openSession() or
requestStatementId() rejects (bad credentials, timeout, or a non-200
status),
the catch only logged the error and rethrew — leaking the open socket and
its
listeners. In the pool this propagates through init() with no close(), so
each
failed connect attempt leaks a socket.
Mirror close()'s teardown (removeAllListeners + destroy/end + null the refs
+
isConnected=false) in the catch before rethrowing, and add a regression
test.
Signed-off-by: Zihan Dai <[email protected]>
* Reset session ids and guard teardown in Connection.open() error path
Address review feedback on #20: on a failed session setup the open()
catch now (1) clears sessionId/statementId so getSessionId() cannot return
a stale id after openSession succeeds but requestStatementId fails, and
(2) wraps the socket teardown in its own try/catch so a cleanup failure
cannot mask the original error that is rethrown below.
Strengthen the regression tests: assert the original setup error surfaces
rather than a teardown error, and add a case asserting that a failure
after openSession clears the session id.
Signed-off-by: Zihan Dai <[email protected]>
---------
Signed-off-by: Zihan Dai <[email protected]>
---
src/connection/Connection.ts | 24 ++++++++++++++++
tests/unit/Connection.test.ts | 64 +++++++++++++++++++++++++++++++++++++++++++
2 files changed, 88 insertions(+)
diff --git a/src/connection/Connection.ts b/src/connection/Connection.ts
index 54d0b2b..38969d0 100644
--- a/src/connection/Connection.ts
+++ b/src/connection/Connection.ts
@@ -92,6 +92,30 @@ export class Connection {
this.isConnected = true;
} catch (error) {
logger.error("Failed to connect:", error);
+ // Tear down the half-open connection so its socket and event listeners
+ // don't leak when session setup (openSession/requestStatementId) fails
+ // after the TCP connection was already established. Mirrors close().
+ // Guard the teardown itself so a cleanup failure can't mask the
+ // original error that we rethrow below.
+ try {
+ if (this.connection) {
+ this.connection.removeAllListeners();
+ if (typeof this.connection.destroy === "function") {
+ this.connection.destroy();
+ } else {
+ this.connection.end();
+ }
+ this.connection = null;
+ }
+ } catch (cleanupError) {
+ logger.warn("Error during connection teardown:", cleanupError);
+ }
+ this.client = null;
+ // Mirror close(): clear session/statement ids so a failed setup does
+ // not leave a stale sessionId reachable via getSessionId().
+ this.sessionId = null;
+ this.statementId = null;
+ this.isConnected = false;
throw error;
}
}
diff --git a/tests/unit/Connection.test.ts b/tests/unit/Connection.test.ts
index 26eca2d..7d5b01b 100644
--- a/tests/unit/Connection.test.ts
+++ b/tests/unit/Connection.test.ts
@@ -129,4 +129,68 @@ describe("Connection", () => {
await connection.close();
});
+
+ test("Should tear down the socket when session setup fails", async () => {
+ // openSession rejects after the TCP connection was established.
+ thriftMock.createClient.mockReturnValueOnce({
+ openSession: jest.fn((_req: unknown, callback: (e: Error | null, r:
unknown) => void) =>
+ callback(new Error("auth failed"), null),
+ ),
+ requestStatementId: jest.fn((_sid: unknown, callback: (e: Error | null,
r: unknown) => void) =>
+ callback(null, 456),
+ ),
+ closeSession: jest.fn((_req: unknown, callback: (e: Error | null, r:
unknown) => void) =>
+ callback(null, { status: { code: 200 } }),
+ ),
+ });
+
+ const config: InternalConfig = {
+ host: "localhost",
+ port: 6667,
+ username: "root",
+ password: "bad",
+ enableSSL: false,
+ sqlDialect: "tree",
+ };
+ const connection = new Connection(config);
+
+ // The original setup error must surface, not be masked by the teardown.
+ await expect(connection.open()).rejects.toThrow("auth failed");
+
+ // The half-open connection must be torn down (mirrors close()); the buggy
+ // catch only logged + rethrew, leaking the socket and its listeners.
+ expect(thriftMock.__mockConnection.removeAllListeners).toHaveBeenCalled();
+ expect(thriftMock.__mockConnection.destroy).toHaveBeenCalled();
+ });
+
+ test("Should clear sessionId when statement setup fails after openSession",
async () => {
+ // openSession succeeds (sets sessionId), then requestStatementId rejects.
+ thriftMock.createClient.mockReturnValueOnce({
+ openSession: jest.fn((_req: unknown, callback: (e: Error | null, r:
unknown) => void) =>
+ callback(null, { status: { code: 200 }, sessionId: 123 }),
+ ),
+ requestStatementId: jest.fn((_sid: unknown, callback: (e: Error | null,
r: unknown) => void) =>
+ callback(new Error("statement setup failed"), null),
+ ),
+ closeSession: jest.fn((_req: unknown, callback: (e: Error | null, r:
unknown) => void) =>
+ callback(null, { status: { code: 200 } }),
+ ),
+ });
+
+ const config: InternalConfig = {
+ host: "localhost",
+ port: 6667,
+ username: "root",
+ password: "root",
+ enableSSL: false,
+ sqlDialect: "tree",
+ };
+ const connection = new Connection(config);
+
+ await expect(connection.open()).rejects.toThrow("statement setup failed");
+
+ // The failed setup must not leave a stale sessionId reachable (mirrors
+ // close()); getSessionId() throws once the id is cleared.
+ expect(() => connection.getSessionId()).toThrow("Session is not open");
+ });
});
