Uploaded image for project: 'TinkerPop'
  1. TinkerPop
  2. TINKERPOP-2708

unhandledRejection upon connection failure

VotersWatch issueWatchersLinkCloneUpdate Comment AuthorReplace String in CommentUpdate Comment VisibilityDelete Comments
    XMLWordPrintableJSON

Details

    • Bug
    • Status: Closed
    • Major
    • Resolution: Done
    • 3.5.2
    • 3.7.0
    • javascript

    Description

      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

      Solution suggestion

      A fairly quick solution (but possibly breaking change) to this is by not opening the database in the constructor (line reference 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:

      this.drc.open().catch(err => {
          console.log("Unable to open connection to database", err);
      });

      Temporary work around example

       

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

       

      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.

       

      Attachments

        Issue Links

        Activity

          This comment will be Viewable by All Users Viewable by All Users
          Cancel

          People

            spmallette Stephen Mallette
            mrgemeco Jon Brede Skaug
            Votes:
            0 Vote for this issue
            Watchers:
            4 Start watching this issue

            Dates

              Created:
              Updated:
              Resolved:

              Slack

                Issue deployment