Skip to content

[OGUI-1891] Add conditional creation of pool in query service #3432

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
graduta merged 4 commits into from
May 5, 2026
Merged

[OGUI-1891] Add conditional creation of pool in query service #3432

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
4ac6752
Add conditional creation of pool in query service
graduta May 5, 2026
c8c0c95
As this is a unit test with no mariadb instance, never create a pool
graduta May 5, 2026
8a4391a
Make sure SQL config is not overwritten across test suite
graduta May 5, 2026
06dea50
Add logging for better tracing of connreset
graduta May 5, 2026
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
47 changes: 36 additions & 11 deletions InfoLogger/lib/services/QueryService.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
*/

const mariadb = require('mariadb');
const { LogManager } = require('@aliceo2/web-ui');
const { LogManager, InvalidInputError } = require('@aliceo2/web-ui');
const { fromSqlToNativeError } = require('../utils/fromSqlToNativeError');
const { processPreparedSQLStatement } = require('../utils/preparedStatementParser');

Expand All @@ -23,19 +23,27 @@ class QueryService {
* @param {object} configMySql - mysql config
*/
constructor(configMySql = {}) {
configMySql.user = configMySql?.user ?? 'gui';
configMySql.password = configMySql?.password ?? '';
configMySql.host = configMySql?.host ?? 'localhost';
configMySql.port = configMySql?.port ?? 3306;
configMySql.database = configMySql?.database ?? 'info_logger';
configMySql.connectionLimit = configMySql?.connectionLimit ?? 25;
this._timeout = configMySql?.timeout ?? 10000;
this._host = configMySql.host;
this._port = configMySql.port;

this._pool = mariadb.createPool(configMySql);
this._host = configMySql?.host;
this._port = configMySql?.port;
this._isAvailable = false;
this._logger = LogManager.getLogger(`${process.env.npm_config_log_label ?? 'ilg'}/query-service`);

// Only create a connection pool if configuration is provided
if (configMySql?.host && configMySql?.port) {
configMySql.user = configMySql.user ?? 'gui';
configMySql.password = configMySql.password ?? '';
configMySql.host = configMySql.host ?? 'localhost';
configMySql.port = configMySql.port ?? 3306;
configMySql.database = configMySql.database ?? 'info_logger';
configMySql.connectionLimit = configMySql.connectionLimit ?? 25;
this._host = configMySql.host;
this._port = configMySql.port;

this._pool = mariadb.createPool(configMySql);
} else {
this._pool = null;
}
}

/**
Expand All @@ -45,6 +53,17 @@ class QueryService {
* @returns {Promise} - a promise that resolves if connection is successful
*/
async checkConnection(timeout = this._timeout, shouldThrow = true) {
if (!this._pool) {
this._isAvailable = false;
const error = new InvalidInputError('No database configuration provided');
if (shouldThrow) {
throw error;
} else {
this._logger.errorMessage(error);
}
return;
}

try {
await this._pool.query({
sql: 'SELECT 1',
Expand Down Expand Up @@ -87,6 +106,9 @@ class QueryService {

let rows = [];
try {
if (!this._pool) {
throw new Error('No database connection available');
}
rows = await this._pool.query(
{
sql: requestRows,
Expand Down Expand Up @@ -119,6 +141,9 @@ class QueryService {
+ 'in (\'D\', \'I\', \'W\', \'E\', \'F\') GROUP BY severity;';
let data = [];
try {
if (!this._pool) {
throw new Error('No database connection available');
}
data = await this._pool.query({
sql: groupByStatement,
timeout: this._timeout,
Expand Down
21 changes: 17 additions & 4 deletions InfoLogger/test/lib/controller/mocha-status-controller.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,11 +19,24 @@ const config = require('./../../test-config.js');
const { StatusController } = require('./../../../lib/controller/StatusController.js');

describe('Status Service test suite', () => {
config.mysql = {
const mysqlConfig = {
host: 'localhost',
port: 6103,
database: 'INFOLOGGER',
};

// Store original config.mysql value to restore later
const originalMysqlConfig = config.mysql;

before(() => {
config.mysql = mysqlConfig;
});

after(() => {
// Restore original config to avoid affecting other tests
config.mysql = originalMysqlConfig;
});

describe('Creating a new StatusController instance', () => {
it('should successfully initialize StatusController', () => {
assert.doesNotThrow(() => new StatusController({ hostname: 'localhost', port: 8080 }, {}));
Expand Down Expand Up @@ -88,7 +101,7 @@ describe('Status Service test suite', () => {
ok: false, message: 'Data source is not available',
},
};
const mysql = await statusController._getDataSourceStatus(config.mysql);
const mysql = await statusController._getDataSourceStatus(mysqlConfig);
assert.deepStrictEqual(mysql, info);
});

Expand All @@ -102,7 +115,7 @@ describe('Status Service test suite', () => {
isAvailable: true,
};
statusController.querySource = dataSource;
const mysql = await statusController._getDataSourceStatus(config.mysql);
const mysql = await statusController._getDataSourceStatus(mysqlConfig);
assert.deepStrictEqual(mysql, info);
},
);
Expand All @@ -124,7 +137,7 @@ describe('Status Service test suite', () => {
isAvailable: false,
};
statusController.querySource = dataSource;
const mysql = await statusController._getDataSourceStatus(config.mysql);
const mysql = await statusController._getDataSourceStatus(mysqlConfig);
assert.deepStrictEqual(mysql, info);
},
);
Expand Down
64 changes: 52 additions & 12 deletions InfoLogger/test/lib/services/mocha-query-service.test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,9 +14,8 @@

const assert = require('assert');
const sinon = require('sinon');
const config = require('../../../config-default.js');
const { QueryService } = require('../../../lib/services/QueryService.js');
const { UnauthorizedAccessError, TimeoutError } = require('@aliceo2/web-ui');
const { UnauthorizedAccessError, TimeoutError, InvalidInputError } = require('@aliceo2/web-ui');

describe('\'QueryService\' test suite', () => {
const filters = {
Expand Down Expand Up @@ -71,11 +70,11 @@ describe('\'QueryService\' test suite', () => {
$max: null, // 0, 1, 6, 11, 21
},
};
const emptySqlDataSource = new QueryService(undefined, {});
const emptySqlDataSource = new QueryService();

describe('\'checkConnection()\' - test suite', () => {
it('should reject with error when simple query fails', async () => {
const sqlDataSource = new QueryService(config.mysql);
const sqlDataSource = new QueryService();
sqlDataSource._isAvailable = true;
sqlDataSource._pool = {
query: sinon.stub().rejects({
Expand All @@ -93,7 +92,7 @@ describe('\'QueryService\' test suite', () => {
});

it('should do nothing when checking connection with mysql driver and driver returns resolved Promise', async () => {
const sqlDataSource = new QueryService(config.mysql);
const sqlDataSource = new QueryService();
sqlDataSource._isAvailable = false;
sqlDataSource._pool = {
query: sinon.stub().resolves(),
Expand All @@ -102,6 +101,48 @@ describe('\'QueryService\' test suite', () => {
await assert.doesNotReject(sqlDataSource.checkConnection());
assert.ok(sqlDataSource.isAvailable);
});

it('should throw InvalidInputError when no pool is configured and shouldThrow is true', async () => {
const sqlDataSource = new QueryService();
await assert.rejects(
sqlDataSource.checkConnection(),
new InvalidInputError('No database configuration provided'),
);
assert.ok(sqlDataSource.isAvailable === false);
});

it('should not throw and log error when no pool is configured and shouldThrow is false', async () => {
const sqlDataSource = new QueryService();
const logStub = sinon.stub();
sqlDataSource._logger = {
errorMessage: logStub,
};

await assert.doesNotReject(sqlDataSource.checkConnection(10000, false));
assert.ok(sqlDataSource.isAvailable === false);
assert.ok(logStub.calledOnce);
assert.ok(logStub.firstCall.args[0].message === 'No database configuration provided');
});

it('should not throw and log error when connection fails and shouldThrow is false', async () => {
const sqlDataSource = new QueryService();
const logStub = sinon.stub();
sqlDataSource._logger = {
errorMessage: logStub,
};
sqlDataSource._pool = {
query: sinon.stub().rejects({
code: 'ER_ACCESS_DENIED_ERROR',
errno: 1045,
sqlMessage: 'Access denied',
}),
};

await assert.doesNotReject(sqlDataSource.checkConnection(10000, false));
assert.ok(sqlDataSource.isAvailable === false);
assert.ok(logStub.calledOnce);
assert.ok(logStub.firstCall.args[0].code === 'ER_ACCESS_DENIED_ERROR');
});
});

describe('Filter to SQL Conditions', () => {
Expand Down Expand Up @@ -191,7 +232,7 @@ describe('\'QueryService\' test suite', () => {

describe('queryFromFilters() - test suite', () => {
it('should throw an error when unable to query(API) due to rejected promise', async () => {
const sqlDataSource = new QueryService(config.mysql);
const sqlDataSource = new QueryService();
sqlDataSource._pool = {
query: sinon.stub().rejects({
code: 'ER_ACCESS_DENIED_ERROR',
Expand All @@ -209,7 +250,7 @@ describe('\'QueryService\' test suite', () => {
const query = 'SELECT * FROM `messages` WHERE `timestamp`>=? AND `timestamp`<=? AND `hostname` = ? '
+ 'AND NOT(`hostname` = ? AND `hostname` IS NOT NULL) AND `severity` IN (?) ORDER BY `TIMESTAMP` LIMIT 10';

const sqlDataSource = new QueryService(config.mysql);
const sqlDataSource = new QueryService();
sqlDataSource._pool = {
query: sinon.stub().resolves([
{ hostname: 'test', severity: 'W' },
Expand All @@ -232,7 +273,7 @@ describe('\'QueryService\' test suite', () => {
});

it('should log every executed sql query as debug', async () => {
const sqlDataSource = new QueryService(config.mysql);
const sqlDataSource = new QueryService();
sqlDataSource._logger = {
debugMessage: sinon.stub(),
};
Expand All @@ -250,7 +291,7 @@ describe('\'QueryService\' test suite', () => {
describe('queryGroupCountLogsBySeverity() - test suite', () => {
it(`should successfully return stats when queried for all known severities
even if none is some are not returned by data service`, async () => {
const dataService = new QueryService(config.mysql);
const dataService = new QueryService();
dataService._pool = {
query: sinon.stub().resolves([
{ severity: 'E', 'COUNT(*)': 102 },
Expand All @@ -268,9 +309,8 @@ describe('\'QueryService\' test suite', () => {
});

it('should throw error if data service throws SQL', async () => {
const dataService = new QueryService(config.mysql);
dataService._pool =
{
const dataService = new QueryService();
dataService._pool = {
query: sinon.stub().rejects({
code: 'ER_ACCESS_DENIED_ERROR',
errno: 1045,
Expand Down
41 changes: 34 additions & 7 deletions InfoLogger/test/live-simulator/infoLoggerServer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,20 @@
const port = 6102; // infoLoggerServer default port

function connectionListener(client) {
console.log('Client connected');
console.log('[InfoLogger Server] Client connected at:', new Date().toISOString());
let timer;
let currentLogIndex = 0;

client.on('close', onClientDisconnect);
client.on('end', onClientDisconnect);
client.on('error', (error) => {
console.error('[InfoLogger Server] Client socket error:', error.code, error.message);

Check warning

Code scanning / CodeQL

Log injection Medium test

Log entry depends on a
user-provided value
Loading
.

Check warning

Code scanning / CodeQL

Log injection Medium test

Log entry depends on a
user-provided value
Loading
.
Comment thread
graduta marked this conversation as resolved.
Dismissed
Comment thread
graduta marked this conversation as resolved.
Dismissed
console.error('[InfoLogger Server] Error occurred at:', new Date().toISOString());
if (error.stack) {
console.error('[InfoLogger Server] Stack trace:', error.stack);

Check warning

Code scanning / CodeQL

Log injection Medium test

Log entry depends on a
user-provided value
Loading
.
Comment thread
graduta marked this conversation as resolved.
Dismissed
}
clearTimeout(timer);
});
sendNextLog();

function sendNextLog() {
Expand Down Expand Up @@ -84,27 +92,46 @@
}

function onClientDisconnect() {
console.log('Client disconnected');
console.log('[InfoLogger Server] Client disconnected at:', new Date().toISOString());
clearTimeout(timer);
}
}

server.on('error', (error) => {
console.error('InfoLogger Server crashed due to:');
console.trace(error);
console.error('[InfoLogger Server] Server error occurred at:', new Date().toISOString());
console.error('[InfoLogger Server] Error code:', error.code);
console.error('[InfoLogger Server] Error message:', error.message);
if (error.stack) {
console.error('[InfoLogger Server] Stack trace:');
console.trace(error);
}
});

server.on('close', () => {
console.log('[InfoLogger Server] Server closed at:', new Date().toISOString());
});

server.listen(port, () => {
console.log(`InfoLoggerServer is running on port ${port}`);
console.log(`[InfoLogger Server] InfoLoggerServer is running on port ${port}`);
});
return server
}

const closeServer = (server) => {
console.log('[InfoLogger Server] Closing server at:', new Date().toISOString());
try {
server.close();
if (server && server.listening) {
server.close((err) => {
if (err) {
console.error('[InfoLogger Server] Error closing server:', err.message);
} else {
console.log('[InfoLogger Server] Server closed successfully');
}
});
}
} catch (err) {
console.error(err);
console.error('[InfoLogger Server] Exception while closing server:', err.message);
console.error('[InfoLogger Server] Stack:', err.stack);
}
}

Expand Down
17 changes: 17 additions & 0 deletions InfoLogger/test/mocha-index.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,23 @@ describe('InfoLogger', function() {
const baseUrl = `http://${config.http.hostname}:${config.http.port}/`;

before(async () => {
// Add error handlers for uncaught errors
process.on('unhandledRejection', (error) => {
console.error('[Test Setup] Unhandled Promise Rejection at:', new Date().toISOString());
console.error('[Test Setup] Error:', error);
if (error && error.stack) {
console.error('[Test Setup] Stack:', error.stack);
}
});

process.on('uncaughtException', (error) => {
console.error('[Test Setup] Uncaught Exception at:', new Date().toISOString());
console.error('[Test Setup] Error:', error);
if (error && error.stack) {
console.error('[Test Setup] Stack:', error.stack);
}
});

// Start infologger server simulator
ilgServer = createServer();

Expand Down
Loading