[ https://issues.apache.org/jira/browse/TINKERPOP-2708?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17551800#comment-17551800 ]
ASF GitHub Bot commented on TINKERPOP-2708: ------------------------------------------- xiazcy commented on PR #1680: URL: https://github.com/apache/tinkerpop/pull/1680#issuecomment-1150431636 > breaking changes are typically documented in the Upgrade Documentation I have added a section on that for the Upgrade Docs > finally, i would assume the deprecation to be removed on merge to master for 3.7.0, so perhaps you should consider a follow-on PR to get rid of the warning message and parameter all together on that branch. I will open a separate PR for master then. > separately, there is a merge commit in the history. could you please remove that? Done. Thank you @spmallette ! > unhandledRejection upon connection failure > ------------------------------------------ > > Key: TINKERPOP-2708 > URL: https://issues.apache.org/jira/browse/TINKERPOP-2708 > Project: TinkerPop > Issue Type: Bug > Components: javascript > Affects Versions: 3.5.2 > Reporter: Jon Brede Skaug > Priority: Major > > 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.connectOnStartup 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, {connectOnStartup: 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.7#820007)