Skip to content

Commit cfdc2a2

Browse files
authored
fix(jdbc-driver): Log errors from connection pool factory (#8903)
generic-pool will just silently throw those away, so we should at least log them Some supporting changes: * Set logger to simple console wrapper in driver tests * Remove unused import * Actually call connection.close in pool's destroy
1 parent 0dbc499 commit cfdc2a2

File tree

2 files changed

+15
-6
lines changed

2 files changed

+15
-6
lines changed

packages/cubejs-jdbc-driver/src/JDBCDriver.ts

Lines changed: 14 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -20,10 +20,11 @@ import { promisify } from 'util';
2020
import genericPool, { Factory, Pool } from 'generic-pool';
2121
import path from 'path';
2222

23-
import { DriverOptionsInterface, SupportedDrivers } from './supported-drivers';
24-
// eslint-disable-next-line @typescript-eslint/no-unused-vars
25-
import { JDBCDriverConfiguration } from './types';
26-
import { QueryStream, nextFn, Row, transformRow } from './QueryStream';
23+
import { SupportedDrivers } from './supported-drivers';
24+
import type { DriverOptionsInterface } from './supported-drivers';
25+
import type { JDBCDriverConfiguration } from './types';
26+
import { QueryStream, transformRow } from './QueryStream';
27+
import type { nextFn } from './QueryStream';
2728

2829
/* eslint-disable no-restricted-syntax,import/no-extraneous-dependencies */
2930
const DriverManager = require('@cubejs-backend/jdbc/lib/drivermanager');
@@ -147,8 +148,7 @@ export class JDBCDriver extends BaseDriver {
147148
const getConnection = promisify(DriverManager.getConnection.bind(DriverManager));
148149
return new Connection(await getConnection(this.config.url, this.jdbcProps));
149150
},
150-
// @ts-expect-error Promise<Function> vs Promise<void>
151-
destroy: async (connection) => promisify(connection.close.bind(connection)),
151+
destroy: async (connection) => promisify(connection.close.bind(connection))(),
152152
validate: async (connection) => (
153153
new Promise((resolve) => {
154154
const isValid = promisify(connection.isValid.bind(connection));
@@ -183,6 +183,14 @@ export class JDBCDriver extends BaseDriver {
183183
acquireTimeoutMillis: 120000,
184184
...(poolOptions || {})
185185
}) as ExtendedPool;
186+
187+
// https://github.com/coopernurse/node-pool/blob/ee5db9ddb54ce3a142fde3500116b393d4f2f755/README.md#L220-L226
188+
this.pool.on('factoryCreateError', (err) => {
189+
this.databasePoolError(err);
190+
});
191+
this.pool.on('factoryDestroyError', (err) => {
192+
this.databasePoolError(err);
193+
});
186194
}
187195

188196
protected async getCustomClassPath() {

packages/cubejs-testing-drivers/src/helpers/getDriver.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ export async function getDriver(type: string): Promise<{
88
return import(`@cubejs-backend/${type}-driver`).then((module) => {
99
// eslint-disable-next-line new-cap
1010
const source: BaseDriver = new module.default();
11+
source.setLogger((msg: unknown, event: unknown) => console.log(`${msg}: ${JSON.stringify(event)}`));
1112
const storage = new CubeStoreDriver();
1213
return { source, storage };
1314
});

0 commit comments

Comments
 (0)