Skip to content

Commit

Permalink
[fix] better connection and error catching with pg pool (#96)
Browse files Browse the repository at this point in the history
  • Loading branch information
lgrd authored Oct 20, 2023
1 parent c7cee21 commit 1e9172b
Show file tree
Hide file tree
Showing 7 changed files with 57 additions and 35 deletions.
5 changes: 5 additions & 0 deletions changelog.md
Original file line number Diff line number Diff line change
@@ -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
Expand Down
6 changes: 3 additions & 3 deletions documentation/test/integration/readme.md
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
@@ -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",
Expand Down Expand Up @@ -31,7 +31,7 @@
},
"optionalDependencies": {
"osrm": "5.26.0",
"pg": "8.8.0"
"pg": "8.9.0"
},
"devDependencies": {
"sinon": "^7.2.7",
Expand Down
49 changes: 30 additions & 19 deletions src/js/base/base.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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");
}

}

Expand Down
4 changes: 2 additions & 2 deletions src/js/sources/osrmSource.js
Original file line number Diff line number Diff line change
Expand Up @@ -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");
}
Expand Down Expand Up @@ -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");
}
Expand Down
14 changes: 8 additions & 6 deletions src/js/sources/pgrSource.js
Original file line number Diff line number Diff line change
Expand Up @@ -390,28 +390,29 @@ module.exports = class pgrSource extends Source {

this._base.pool.query(queryString, SQLParametersTable, (err, result) => {

this.state = "green";

if (err) {

LOGGER.error("pgr error:");
LOGGER.error(err);

// 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);
}

} else {

LOGGER.debug("pgr response:");
LOGGER.debug(result);
this.state = "green";

try {
resolve(this.writeRouteResponse(request, pgrRequest, result));
Expand All @@ -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");
}
Expand Down Expand Up @@ -495,24 +496,25 @@ 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);
}

} else {

LOGGER.debug("pgr response:");
LOGGER.debug(result);
this.state = "green";

try {
resolve(this.writeIsochroneResponse(request, result));
Expand All @@ -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");
}
Expand Down
10 changes: 7 additions & 3 deletions test/unit/mocha/base/testsBase.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
});
Expand Down

0 comments on commit 1e9172b

Please sign in to comment.