From 1e9172b3957d00cb70c32c4f8d6b92cddc97c326 Mon Sep 17 00:00:00 2001 From: lgrd Date: Fri, 20 Oct 2023 16:23:22 +0200 Subject: [PATCH] [fix] better connection and error catching with pg pool (#96) --- changelog.md | 5 +++ documentation/test/integration/readme.md | 6 +-- package.json | 4 +- src/js/base/base.js | 49 +++++++++++++++--------- src/js/sources/osrmSource.js | 4 +- src/js/sources/pgrSource.js | 14 ++++--- test/unit/mocha/base/testsBase.js | 10 +++-- 7 files changed, 57 insertions(+), 35 deletions(-) diff --git a/changelog.md b/changelog.md index 72f72521..f8a08974 100644 --- a/changelog.md +++ b/changelog.md @@ -1,5 +1,10 @@ # CHANGELOG +## 2.2.4 +FIXED: +- The pg module can emit error event and they were not catched and so it caused some crashs of Road2 +- Some orange states were deleted from pgrSource and osrmSource + ## 2.2.3 FIXED: - Sources were not disconnected during a restart #93 diff --git a/documentation/test/integration/readme.md b/documentation/test/integration/readme.md index d5287214..8c3551b1 100644 --- a/documentation/test/integration/readme.md +++ b/documentation/test/integration/readme.md @@ -167,9 +167,9 @@ C'est l'approche bottom-up qui a été choisie pour ces tests. On va tester les - pg {Pool} - base/base.js - - () - - connect() - - end() + - Pool() + - pool.on('error') + - pool.end() - base/baseManager.js - sources/sourcesManager.js - sources/pgrSource.js diff --git a/package.json b/package.json index f8f9ffea..a6fb3853 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "road2", - "version": "2.2.4-DEVELOP", + "version": "2.2.4", "description": "Calcul d'itinéraire", "author": "RDEV - IGN", "main": "src/js/road2.js", @@ -31,7 +31,7 @@ }, "optionalDependencies": { "osrm": "5.26.0", - "pg": "8.8.0" + "pg": "8.9.0" }, "devDependencies": { "sinon": "^7.2.7", diff --git a/src/js/base/base.js b/src/js/base/base.js index 903ed33a..7edc2007 100644 --- a/src/js/base/base.js +++ b/src/js/base/base.js @@ -31,18 +31,28 @@ module.exports = class Base { */ constructor(configuration) { + // Configuration pour se connecter à la base + this._configuration = configuration; + // Pool de clients PostgreSQL - if (Pool) { - this._pool = new Pool(configuration); - } else { - this._pool = null; - } + this._pool = null; // État de la connexion this._connected = false; } + /** + * + * @function + * @name get configuration + * @description Récupérer la configuration de la base + * + */ + get configuration () { + return this._configuration; + } + /** * * @function @@ -76,24 +86,25 @@ module.exports = class Base { // Connection à la base de données LOGGER.info("Connection a la base de données"); - try { - if (this._pool) { - await this._pool.connect(); - LOGGER.info("Pool connecté à la base"); - this._connected = true; - } else { - throw errorManager.createError("PG is not available"); - } + // On crée le Pool ici car c'est à cet appel qu'il se connecte à la base + if (Pool) { - // TODO : supprimer le return si pas utile - // return new Promise(); + this._pool = new Pool(this._configuration); + this._connected = true; - } catch (err) { - LOGGER.error("connection error", err.stack) - throw errorManager.createError("Cannot connect to database"); - } + // On ajoute la gestion des events ici + // From the PG module doc : + // the pool will emit an error on behalf of any idle clients + // it contains if a backend error or network partition happens + // The client will be automatically terminated and removed from the pool, it is only passed to the error handler in case you want to inspect it. + this._pool.on('error', (err, client) => { + LOGGER.error('Unexpected error on idle client', err); + }); + } else { + throw errorManager.createError("PG is not available"); + } } diff --git a/src/js/sources/osrmSource.js b/src/js/sources/osrmSource.js index fdad5ead..b163f569 100644 --- a/src/js/sources/osrmSource.js +++ b/src/js/sources/osrmSource.js @@ -265,7 +265,7 @@ module.exports = class osrmSource extends Source { } catch (error) { // Pour une raison que l'on ignore, la source n'est plus joignable - this.state = "orange"; + this.state = "red"; LOGGER.error(error); reject("Internal OSRM error"); } @@ -328,7 +328,7 @@ module.exports = class osrmSource extends Source { } catch (error) { // Pour une raison que l'on ignore, la source n'est plus joignable - this.state = "orange"; + this.state = "red"; LOGGER.error(error); reject("Internal OSRM error"); } diff --git a/src/js/sources/pgrSource.js b/src/js/sources/pgrSource.js index 9e4033fa..5670819c 100644 --- a/src/js/sources/pgrSource.js +++ b/src/js/sources/pgrSource.js @@ -390,8 +390,6 @@ module.exports = class pgrSource extends Source { this._base.pool.query(queryString, SQLParametersTable, (err, result) => { - this.state = "green"; - if (err) { LOGGER.error("pgr error:"); @@ -399,12 +397,14 @@ module.exports = class pgrSource extends Source { // Traitement spécifique de certains codes pour dire au client qu'on n'a pas trouvé de routes if (err.code === "38001") { + this.state = "green"; reject(errorManager.createError(" No path found ", 404)); } else if (err.code === "42703") { // cette erreur remonte quand il n'y a pas de données dans PGR this.state = "red"; reject(errorManager.createError(" No data found ", 503)); } else { + this.state = "red"; reject(err); } @@ -412,6 +412,7 @@ module.exports = class pgrSource extends Source { LOGGER.debug("pgr response:"); LOGGER.debug(result); + this.state = "green"; try { resolve(this.writeRouteResponse(request, pgrRequest, result)); @@ -425,7 +426,7 @@ module.exports = class pgrSource extends Source { } catch (error) { // Pour une raison que l'on ignore, la source n'est plus joignable - this.state = "orange"; + this.state = "red"; LOGGER.error(error); reject("Internal PGR error"); } @@ -495,17 +496,17 @@ module.exports = class pgrSource extends Source { this._base.pool.query(queryString, SQLParametersTable, (err, result) => { - this.state = "green"; - if (err) { // Traitement spécifique de certains codes pour dire au client qu'on n'a pas trouvé d'iso if (err.code === "XX000") { // Cette erreur remonte souvent quand PGR n'a pas assez de données pour créer ou calculer une iso (ex. costValue trop petit) + this.state = "green"; reject(errorManager.createError(" No iso found ", 404)); } else { LOGGER.error("pgr error:"); LOGGER.error(err); + this.state = "red"; reject(err); } @@ -513,6 +514,7 @@ module.exports = class pgrSource extends Source { LOGGER.debug("pgr response:"); LOGGER.debug(result); + this.state = "green"; try { resolve(this.writeIsochroneResponse(request, result)); @@ -526,7 +528,7 @@ module.exports = class pgrSource extends Source { } catch (error) { // Pour une raison que l'on ignore, la source n'est plus joignable - this.state = "orange"; + this.state = "red"; LOGGER.error(error); reject("Internal PGR error"); } diff --git a/test/unit/mocha/base/testsBase.js b/test/unit/mocha/base/testsBase.js index 660a52d6..1323215d 100644 --- a/test/unit/mocha/base/testsBase.js +++ b/test/unit/mocha/base/testsBase.js @@ -23,19 +23,23 @@ describe('Test de la classe Base', function() { }); it('Get pool', function() { - assert.equal(base.pool.options.host, "pgrouting"); + assert.equal(base.pool, null); + }); + + it('Get configuration', function() { + assert.equal(base.configuration, configuration); }); }); describe('Test de la connexion et de la deconnexion', function() { - xit('Connect()', async function() { + it('Connect()', async function() { await base.connect(); assert.equal(base.connected, true); }); - xit('Disconnect()', async function() { + it('Disconnect()', async function() { await base.disconnect(); assert.equal(base.connected, false); });