Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/connectors/__tests__/connect-failure-cleanup.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,7 @@ describe("connect() failure cleanup", () => {
mariadbCreatePool.mockReturnValue({
query: vi.fn().mockRejectedValue(PROBE_FAILURE),
end,
on: vi.fn(),
});

const connector = new MariaDBConnector();
Expand All @@ -98,6 +99,7 @@ describe("connect() failure cleanup", () => {
pgPoolCtor.mockReturnValue({
connect: vi.fn().mockRejectedValue(PROBE_FAILURE),
end,
on: vi.fn(),
});

const connector = new PostgresConnector();
Expand Down
118 changes: 118 additions & 0 deletions src/connectors/__tests__/pool-error-listener.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,118 @@
import { describe, it, expect, vi, beforeEach, afterEach } from "vitest";
import { EventEmitter } from "events";

/**
* Pools must carry an 'error' listener (bytebase/dbhub#422).
*
* pg-pool re-emits an idle client's error (server restart, failover,
* idle-timeout close) as an 'error' event on the pool, and the mariadb pool
* emits 'error' when a background reconnect attempt fails. An EventEmitter
* with no 'error' listener throws on emit, which Node reports as an
* unhandled 'error' event and exits the process. Over stdio, MCP clients do
* not restart the server, so the crash takes the database tools away until
* the user reconnects manually.
*
* The fake pools below are real EventEmitters, so `emit("error")` throws
* exactly like the drivers' pools would if the connector forgot the listener.
*/

class FakePgPool extends EventEmitter {
connect = vi.fn().mockResolvedValue({ release: vi.fn() });
end = vi.fn().mockResolvedValue(undefined);
}

class FakeMariadbPool extends EventEmitter {
query = vi.fn().mockResolvedValue([{ version: "11.4.0-MariaDB" }]);
end = vi.fn().mockResolvedValue(undefined);
}

let pgPool: FakePgPool;
let mariadbPool: FakeMariadbPool;

vi.mock("pg", () => ({
default: {
Pool: function (this: any) {
return pgPool;
},
},
}));

vi.mock("mariadb", () => ({
createPool: () => mariadbPool,
}));

const { PostgresConnector } = await import("../postgres/index.js");
const { MariaDBConnector } = await import("../mariadb/index.js");

const IDLE_DROP = new Error("terminating connection due to administrator command");

const spyConsoleError = () => vi.spyOn(console, "error").mockImplementation(() => {});

describe("pool 'error' listener", () => {
let consoleError: ReturnType<typeof spyConsoleError>;

beforeEach(() => {
pgPool = new FakePgPool();
mariadbPool = new FakeMariadbPool();
consoleError = spyConsoleError();
});

afterEach(() => {
vi.restoreAllMocks();
});

it("an unlistened pool would crash the process (sanity check of the fake)", () => {
expect(() => pgPool.emit("error", IDLE_DROP)).toThrow(IDLE_DROP);
});

it("PostgreSQL survives an idle connection being dropped", async () => {
const connector = new PostgresConnector();
await connector.connect("postgres://u:p@localhost:5432/db");

expect(pgPool.listenerCount("error")).toBe(1);
// Mirrors pg-pool's idleListener: the client is already purged, then the
// pool re-emits. With no listener this emit would throw.
expect(() => pgPool.emit("error", IDLE_DROP, {})).not.toThrow();

expect(consoleError).toHaveBeenCalledTimes(1);
expect(consoleError.mock.calls[0].join(" ")).toContain("PostgreSQL pool");
expect(consoleError.mock.calls[0].join(" ")).toContain(IDLE_DROP.message);

await connector.disconnect();
});

it("PostgreSQL log line names the source", async () => {
const connector = new PostgresConnector();
(connector as any).sourceId = "prod_pg";
await connector.connect("postgres://u:p@localhost:5432/db");

pgPool.emit("error", IDLE_DROP, {});
expect(consoleError.mock.calls[0][0]).toContain('source "prod_pg"');
});

it("MariaDB survives a background reconnect failure", async () => {
const connector = new MariaDBConnector();
await connector.connect("mariadb://u:p@localhost:3306/db");

expect(mariadbPool.listenerCount("error")).toBe(1);
const reconnectFailure = new Error("Pool fails to create connection: ECONNREFUSED");
expect(() => mariadbPool.emit("error", reconnectFailure)).not.toThrow();

expect(consoleError).toHaveBeenCalledTimes(1);
expect(consoleError.mock.calls[0].join(" ")).toContain("MariaDB pool");
expect(consoleError.mock.calls[0].join(" ")).toContain(reconnectFailure.message);

await connector.disconnect();
});

it("does not stack listeners across reconnects of the same connector", async () => {
const connector = new PostgresConnector();
await connector.connect("postgres://u:p@localhost:5432/db");
await connector.disconnect();

// A fresh pool per connect(): the listener is attached to the new pool.
pgPool = new FakePgPool();
await connector.connect("postgres://u:p@localhost:5432/db");
expect(pgPool.listenerCount("error")).toBe(1);
});
});
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,8 @@ function makeFakePool(version: string, wrapResults: (rows: any[]) => any) {
query: vi.fn(async () => wrapResults([{ version }])),
getConnection: vi.fn(async () => conn),
end: vi.fn(),
// Connectors attach a pool 'error' listener at connect time.
on: vi.fn(),
};
return { pool, conn, statements };
}
Expand Down
14 changes: 14 additions & 0 deletions src/connectors/mariadb/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,20 @@ export class MariaDBConnector implements Connector {

this.pool = mariadb.createPool(connectionConfig);

// The mariadb pool keeps `minimumIdle` connections open in the background
// (defaults to connectionLimit) and emits an 'error' event on the pool when
// one of those background reconnect attempts fails, e.g. while the server
// is restarting. Without a listener Node treats it as unhandled and exits
// the whole process. The pool retries with backoff on its own, so logging
// is all that is needed here. The typings omit this event, but the runtime
// Pool is an EventEmitter.
(this.pool as unknown as NodeJS.EventEmitter).on("error", (err: Error) => {
console.error(
`MariaDB pool (source "${this.sourceId}"): background connection error, pool will retry:`,
err.message
);
});

// Test the connection and detect the server flavor in the same round trip.
const rows = await this.pool.query("SELECT VERSION() AS version");
this.supportsReadOnlyTransaction = !isTiDBVersion(rows?.[0]?.version);
Expand Down
12 changes: 12 additions & 0 deletions src/connectors/postgres/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,18 @@ export class PostgresConnector implements Connector {

this.pool = new Pool(poolConfig);

// pg-pool re-emits an idle client's error (server restart, failover,
// idle-timeout close) as an 'error' event on the pool. Without a listener
// Node treats it as unhandled and exits the whole process. The client has
// already been purged from the pool by the time this fires, so logging is
// all that is needed; the next query checks out a fresh connection.
this.pool.on("error", (err: Error) => {
console.error(
`PostgreSQL pool (source "${this.sourceId}"): idle connection dropped, will reconnect on next query:`,
err.message
);
});

// Test the connection
const client = await this.pool.connect();
client.release();
Expand Down
Loading