Jon Brede Skaug created TINKERPOP-2708:
------------------------------------------
Summary: unhandledRejection upon connection failure
Key: TINKERPOP-2708
URL: https://issues.apache.org/jira/browse/TINKERPOP-2708
Project: TinkerPop
Issue Type: Bug
Components: javascript
Reporter: Jon Brede Skaug
In the Javascript driver is unable to connect to the graph database for
whatever reason an unhandledRejection warning occurs.
I have tested this with `new DriverRemoteConnection`
This is a silent error and it won't be able to catch the error due to the way
it is handled.
I've tracked it down to this line:
[https://github.com/apache/tinkerpop/blob/c22c0141bb7a00f366f929d0e5d3c6379d1004e0/gremlin-javascript/src/main/javascript/gremlin-javascript/lib/driver/connection.js#L156]
h2. *Solution suggestion*
A fairly quick solution (but possibly breaking change) to this is by not
opening the database in the constructor [(line reference
L105)|https://github.com/apache/tinkerpop/blob/c22c0141bb7a00f366f929d0e5d3c6379d1004e0/gremlin-javascript/src/main/javascript/gremlin-javascript/lib/driver/connection.js#L105]
but instead forcing the user to run the `DriverRemoteConnection.open()` after
the constructor has been initialized. `DriverRemoteConnection.open()` returns a
promise which makes more sense and is a bit more intuitive. The current error
message gives an error about DNS which is increadibly confusing without
deepdiving into the Gremlin driver code and navigating through 3 classes to
find the culprit. It's also an error which seems a bit more harmless than it
actually is.
It'salso possible to set option.connectOnDefault to "false" by default, this
however will require the user to be aware of the possible failure upon setting
it to true. I believe forcing the user to run .open() after initializing the
class may be more robust.
By doing it this way the user can instead handle the error raised by
DriverRemoteConnection.open() by using promise.catch() or an async function
using await. Promise.catch() is as provided:
{{}}
{code:java}
this.drc.open().catch(err => {
console.log("Unable to open connection to database", err);
});{code}
h2. *{{Temporary work around example}}*
{code:java}
// Using promises
const drc = new DriverRemoteConnection(url, {connectOnDefault:false});
drc.open().catch(err => {
// Handle error upon open, i.e using retry and backoff logic, notify an
alarm system or setting a global variable to reject requests.
});{code}
h2. *The issue with not handling the error properly:*
Not handling the error properly means that if you pass in an invalid URL or the
gremlin compatible database is down, it won't be able to handle the connection
error before a transaction is attempted.
In the future Node.js unhandledRejection will terminate the Node.js process.
This can cause critical failure of processes upon boot and may even cause DDoS
situations where processes may flood the gremlin compatible database with
connection attempts due to processes failing and being reinstated over and over
by a process monitor.
--
This message was sent by Atlassian Jira
(v8.20.1#820001)
