From 584b9fd314385ddcb6ad931c2154d0a6bb8fc7de Mon Sep 17 00:00:00 2001 From: Mikhail Cheshkov Date: Sun, 3 Nov 2024 12:25:20 +0200 Subject: [PATCH 1/5] fix(jdbc-driver): Log errors from connection pool factory generic-pool will just silently throw those away, so we should at least log them --- packages/cubejs-jdbc-driver/src/JDBCDriver.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/packages/cubejs-jdbc-driver/src/JDBCDriver.ts b/packages/cubejs-jdbc-driver/src/JDBCDriver.ts index 136d21c289e07..73b0cd4e735b1 100644 --- a/packages/cubejs-jdbc-driver/src/JDBCDriver.ts +++ b/packages/cubejs-jdbc-driver/src/JDBCDriver.ts @@ -183,6 +183,14 @@ export class JDBCDriver extends BaseDriver { acquireTimeoutMillis: 120000, ...(poolOptions || {}) }) as ExtendedPool; + + // https://github.com/coopernurse/node-pool/blob/ee5db9ddb54ce3a142fde3500116b393d4f2f755/README.md#L220-L226 + this.pool.on('factoryCreateError', (err) => { + this.databasePoolError(err); + }); + this.pool.on('factoryDestroyError', (err) => { + this.databasePoolError(err); + }); } protected async getCustomClassPath() { From 942c41ca030b824a598b4c5205d1a2632c06a7df Mon Sep 17 00:00:00 2001 From: Mikhail Cheshkov Date: Sun, 3 Nov 2024 21:31:42 +0200 Subject: [PATCH 2/5] test(testing-drivers): Set logger to simple console wrapper in driver tests --- packages/cubejs-testing-drivers/src/helpers/getDriver.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/cubejs-testing-drivers/src/helpers/getDriver.ts b/packages/cubejs-testing-drivers/src/helpers/getDriver.ts index 9e61439c309ad..b1908cbfed3b7 100644 --- a/packages/cubejs-testing-drivers/src/helpers/getDriver.ts +++ b/packages/cubejs-testing-drivers/src/helpers/getDriver.ts @@ -8,6 +8,7 @@ export async function getDriver(type: string): Promise<{ return import(`@cubejs-backend/${type}-driver`).then((module) => { // eslint-disable-next-line new-cap const source: BaseDriver = new module.default(); + source.setLogger((msg: unknown, event: unknown) => console.log(`${msg}: ${JSON.stringify(event)}`)); const storage = new CubeStoreDriver(); return { source, storage }; }); From 9af493631dd9ecbcbad8a82cab6dc00d921a6c95 Mon Sep 17 00:00:00 2001 From: Mikhail Cheshkov Date: Thu, 14 Nov 2024 12:53:27 +0200 Subject: [PATCH 3/5] chore(jdbc-driver): Remove unused import --- packages/cubejs-jdbc-driver/src/JDBCDriver.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/cubejs-jdbc-driver/src/JDBCDriver.ts b/packages/cubejs-jdbc-driver/src/JDBCDriver.ts index 73b0cd4e735b1..30513a1bd3cdf 100644 --- a/packages/cubejs-jdbc-driver/src/JDBCDriver.ts +++ b/packages/cubejs-jdbc-driver/src/JDBCDriver.ts @@ -23,7 +23,7 @@ import path from 'path'; import { DriverOptionsInterface, SupportedDrivers } from './supported-drivers'; // eslint-disable-next-line @typescript-eslint/no-unused-vars import { JDBCDriverConfiguration } from './types'; -import { QueryStream, nextFn, Row, transformRow } from './QueryStream'; +import { QueryStream, nextFn, transformRow } from './QueryStream'; /* eslint-disable no-restricted-syntax,import/no-extraneous-dependencies */ const DriverManager = require('@cubejs-backend/jdbc/lib/drivermanager'); From 9f6ccdca7d72bd623aae2ca94e0566326dde71e8 Mon Sep 17 00:00:00 2001 From: Mikhail Cheshkov Date: Thu, 14 Nov 2024 12:55:43 +0200 Subject: [PATCH 4/5] chore(jdbc-driver): Use import type --- packages/cubejs-jdbc-driver/src/JDBCDriver.ts | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/packages/cubejs-jdbc-driver/src/JDBCDriver.ts b/packages/cubejs-jdbc-driver/src/JDBCDriver.ts index 30513a1bd3cdf..7e66463c33874 100644 --- a/packages/cubejs-jdbc-driver/src/JDBCDriver.ts +++ b/packages/cubejs-jdbc-driver/src/JDBCDriver.ts @@ -20,10 +20,11 @@ import { promisify } from 'util'; import genericPool, { Factory, Pool } from 'generic-pool'; import path from 'path'; -import { DriverOptionsInterface, SupportedDrivers } from './supported-drivers'; -// eslint-disable-next-line @typescript-eslint/no-unused-vars -import { JDBCDriverConfiguration } from './types'; -import { QueryStream, nextFn, transformRow } from './QueryStream'; +import { SupportedDrivers } from './supported-drivers'; +import type { DriverOptionsInterface } from './supported-drivers'; +import type { JDBCDriverConfiguration } from './types'; +import { QueryStream, transformRow } from './QueryStream'; +import type { nextFn } from './QueryStream'; /* eslint-disable no-restricted-syntax,import/no-extraneous-dependencies */ const DriverManager = require('@cubejs-backend/jdbc/lib/drivermanager'); From 59e4bcf4d20d426253c1f771ecda27fbda3e9996 Mon Sep 17 00:00:00 2001 From: Mikhail Cheshkov Date: Thu, 14 Nov 2024 12:56:33 +0200 Subject: [PATCH 5/5] fix(jdbc-driver): Actually call connection.close in pool's destroy --- packages/cubejs-jdbc-driver/src/JDBCDriver.ts | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/packages/cubejs-jdbc-driver/src/JDBCDriver.ts b/packages/cubejs-jdbc-driver/src/JDBCDriver.ts index 7e66463c33874..cb9f58011a2cd 100644 --- a/packages/cubejs-jdbc-driver/src/JDBCDriver.ts +++ b/packages/cubejs-jdbc-driver/src/JDBCDriver.ts @@ -148,8 +148,7 @@ export class JDBCDriver extends BaseDriver { const getConnection = promisify(DriverManager.getConnection.bind(DriverManager)); return new Connection(await getConnection(this.config.url, this.jdbcProps)); }, - // @ts-expect-error Promise vs Promise - destroy: async (connection) => promisify(connection.close.bind(connection)), + destroy: async (connection) => promisify(connection.close.bind(connection))(), validate: async (connection) => ( new Promise((resolve) => { const isValid = promisify(connection.isValid.bind(connection));