Cleanup error codes

This commit makes sure direct and routing driver throw exceptions with correct
error codes. Direct driver throws errors with code `ServiceUnavailable` on
connection failures. Routing driver throws errors with code
`ServiceUnavailable` only when rediscovery fails or no servers are available.
It also throws errors with code `SessionExpired` on connection failures.

Author: lutovich

src/v1/driver.js

@@ -68,7 +68,7 @@ class Driver {
    */
   _createConnection(url, release) {
     let sessionId = this._sessionIdGenerator++;
-    let conn = connect(url, this._config);
+    let conn = connect(url, this._config, this._connectionErrorCode());
     let streamObserver = new _ConnectionStreamObserver(this, conn);
     conn.initialize(this._userAgent, this._token, streamObserver);
     conn._id = sessionId;
@@ -126,16 +126,22 @@ class Driver {
     return mode;
   }
 
-  //Extension point
+  // Extension point
   _createConnectionProvider(address, connectionPool, driverOnErrorCallback) {
     return new DirectConnectionProvider(address, connectionPool, driverOnErrorCallback);
   }
 
-  //Extension point
+  // Extension point
   _createSession(mode, connectionProvider, bookmark, config) {
     return new Session(mode, connectionProvider, bookmark, config);
   }
 
+  // Extension point
+  _connectionErrorCode() {
+    // connection errors might result in different error codes depending on the driver
+    return SERVICE_UNAVAILABLE;
+  }
+
   _driverOnErrorCallback(error) {
     const userDefinedOnErrorCallback = this.onError;
     if (userDefinedOnErrorCallback && error.code === SERVICE_UNAVAILABLE) {

src/v1/internal/ch-node.js

@@ -16,14 +16,14 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-import net from "net";
-import tls from "tls";
-import fs from "fs";
-import path from "path";
-import {EOL} from "os";
-import {NodeBuffer} from "./buf";
-import {ENCRYPTION_OFF, isEmptyObjectOrNull} from "./util";
-import {newError, SESSION_EXPIRED} from "./../error";
+import net from 'net';
+import tls from 'tls';
+import fs from 'fs';
+import path from 'path';
+import {EOL} from 'os';
+import {NodeBuffer} from './buf';
+import {ENCRYPTION_OFF, isEmptyObjectOrNull} from './util';
+import {newError, SERVICE_UNAVAILABLE} from './../error';
 
 let _CONNECTION_IDGEN = 0;
 
@@ -291,6 +291,7 @@ class NodeChannel {
     this._error = null;
     this._handleConnectionError = this._handleConnectionError.bind(this);
     this._handleConnectionTerminated = this._handleConnectionTerminated.bind(this);
+    this._errorCode = opts.errorCode || SERVICE_UNAVAILABLE;
 
     this._encrypted = opts.encrypted;
     this._conn = connect(opts, () => {
@@ -318,14 +319,14 @@ class NodeChannel {
 
   _handleConnectionError( err ) {
     let msg = err.message || 'Failed to connect to server';
-    this._error = newError(msg, SESSION_EXPIRED);
+    this._error = newError(msg, this._errorCode);
     if( this.onerror ) {
       this.onerror(this._error);
     }
   }
 
   _handleConnectionTerminated() {
-      this._error = newError('Connection was closed by server', SESSION_EXPIRED);
+      this._error = newError('Connection was closed by server', this._errorCode);
       if( this.onerror ) {
         this.onerror(this._error);
       }

src/v1/internal/ch-websocket.js

@@ -18,8 +18,8 @@
  */
 
 import {HeapBuffer} from './buf';
-import {newError} from './../error';
-import {ENCRYPTION_ON, ENCRYPTION_OFF} from './util';
+import {newError, SERVICE_UNAVAILABLE} from './../error';
+import {ENCRYPTION_OFF, ENCRYPTION_ON} from './util';
 
 /**
  * Create a new WebSocketChannel to be used in web browsers.
@@ -39,6 +39,7 @@ class WebSocketChannel {
     this._pending = [];
     this._error = null;
     this._handleConnectionError = this._handleConnectionError.bind(this);
+    this._errorCode = opts.errorCode || SERVICE_UNAVAILABLE;
 
     this._encrypted = opts.encrypted;
 
@@ -95,7 +96,7 @@ class WebSocketChannel {
         "the root cause of the failure. Common reasons include the database being " +
         "unavailable, using the wrong connection URL or temporary network problems. " +
         "If you have enabled encryption, ensure your browser is configured to trust the " +
-        "certificate Neo4j is configured to use. WebSocket `readyState` is: " + this._ws.readyState );
+        "certificate Neo4j is configured to use. WebSocket `readyState` is: " + this._ws.readyState, this._errorCode );
       if (this.onerror) {
         this.onerror(this._error);
       }

src/v1/internal/connection-providers.js

@@ -17,7 +17,7 @@
  * limitations under the License.
  */
 
-import {newError, SERVICE_UNAVAILABLE, SESSION_EXPIRED} from '../error';
+import {newError, SERVICE_UNAVAILABLE} from '../error';
 import {READ, WRITE} from '../driver';
 import Session from '../session';
 import RoundRobinArray from './round-robin-array';
@@ -96,7 +96,7 @@ export class LoadBalancer extends ConnectionProvider {
   _acquireConnectionToServer(serversRoundRobinArray, serverName) {
     const address = serversRoundRobinArray.next();
     if (!address) {
-      return Promise.reject(newError('No ' + serverName + ' servers available', SESSION_EXPIRED));
+      return Promise.reject(newError('No ' + serverName + ' servers available', SERVICE_UNAVAILABLE));
     }
     return this._connectionPool.acquire(address);
   }

src/v1/internal/connector.js

@@ -23,7 +23,7 @@ import hasFeature from './features';
 import {Packer, Unpacker} from './packstream';
 import {alloc} from './buf';
 import {Node, Path, PathSegment, Relationship, UnboundRelationship} from '../graph-types';
-import {newError} from './../error';
+import {newError, SERVICE_UNAVAILABLE} from './../error';
 
 let Channel;
 if( NodeChannel.available ) {
@@ -475,10 +475,11 @@ class Connection {
  * Crete new connection to the provided url.
  * @access private
  * @param {string} url - 'neo4j'-prefixed URL to Neo4j Bolt endpoint
- * @param {object} config
+ * @param {object} config - this driver configuration
+ * @param {string} errorCode - error code for errors raised on connection errors
  * @return {Connection} - New connection
  */
-function connect( url, config = {}) {
+function connect( url, config = {}, errorCode = SERVICE_UNAVAILABLE) {
   let Ch = config.channel || Channel;
   const host = parseHost(url);
   const port = parsePort(url) || 7687;
@@ -492,7 +493,8 @@ function connect( url, config = {}) {
     // Default to using TRUST_ALL_CERTIFICATES if it is available
     trust : config.trust || (hasFeature("trust_all_certificates") ? "TRUST_ALL_CERTIFICATES" : "TRUST_CUSTOM_CA_SIGNED_CERTIFICATES"),
     trustedCertificates : config.trustedCertificates || [],
-    knownHosts : config.knownHosts
+    knownHosts : config.knownHosts,
+    errorCode: errorCode
   }), completeUrl);
 }
 

src/v1/routing-driver.js

@@ -19,7 +19,7 @@
 
 import Session from './session';
 import {Driver} from './driver';
-import {newError, SERVICE_UNAVAILABLE, SESSION_EXPIRED} from './error';
+import {newError, SESSION_EXPIRED} from './error';
 import {LoadBalancer} from './internal/connection-providers';
 
 /**
@@ -37,13 +37,10 @@ class RoutingDriver extends Driver {
 
   _createSession(mode, connectionProvider, bookmark, config) {
     return new RoutingSession(mode, connectionProvider, bookmark, config, (error, conn) => {
-      if (error.code === SERVICE_UNAVAILABLE || error.code === SESSION_EXPIRED) {
-        // connection is undefined if error happened before connection was acquired
-        if (conn) {
-          this._connectionProvider.forget(conn.url);
-        }
+      if (error.code === SESSION_EXPIRED) {
+        this._forgetConnection(conn);
         return error;
-      } else if (error.code === 'Neo.ClientError.Cluster.NotALeader') {
+      } else if (RoutingDriver._isFailureToWrite(error)) {
         let url = 'UNKNOWN';
         // connection is undefined if error happened before connection was acquired
         if (conn) {
@@ -57,12 +54,30 @@ class RoutingDriver extends Driver {
     });
   }
 
+  _connectionErrorCode() {
+    // connection errors mean SERVICE_UNAVAILABLE for direct driver but for routing driver they should only
+    // result in SESSION_EXPIRED because there might still exist other servers capable of serving the request
+    return SESSION_EXPIRED;
+  }
+
+  _forgetConnection(connection) {
+    // connection is undefined if error happened before connection was acquired
+    if (connection) {
+      this._connectionProvider.forget(connection.url);
+    }
+  }
+
   static _validateConfig(config) {
     if(config.trust === 'TRUST_ON_FIRST_USE') {
       throw newError('The chosen trust mode is not compatible with a routing driver');
     }
     return config;
   }
+
+  static _isFailureToWrite(error) {
+    return error.code === 'Neo.ClientError.Cluster.NotALeader' ||
+      error.code === 'Neo.ClientError.General.ForbiddenOnReadOnlyDatabase';
+  }
 }
 
 class RoutingSession extends Session {

test/internal/connection-providers.test.js

@@ -996,6 +996,30 @@ describe('LoadBalancer', () => {
     });
   });
 
+  it('throws service unavailable when refreshed routing table has no readers', done => {
+    const pool = newPool();
+    const updatedRoutingTable = newRoutingTable(
+      ['server-A', 'server-B'],
+      [],
+      ['server-C', 'server-D']
+    );
+    const loadBalancer = newLoadBalancer(
+      ['server-1', 'server-2'],
+      ['server-3', 'server-4'],
+      ['server-5', 'server-6'],
+      pool,
+      int(0), // expired routing table
+      {
+        'server-1': updatedRoutingTable,
+      }
+    );
+
+    loadBalancer.acquireConnection(READ).catch(error => {
+      expect(error.code).toEqual(SERVICE_UNAVAILABLE);
+      done();
+    });
+  });
+
 });
 
 function newDirectConnectionProvider(address, pool) {

test/v1/direct.driver.boltkit.it.js

@@ -245,6 +245,31 @@ describe('direct driver', () => {
       });
     });
   });
+
+  it('should throw service unavailable when server dies', done => {
+    if (!boltkit.BoltKitSupport) {
+      done();
+      return;
+    }
+
+    const kit = new boltkit.BoltKit();
+    const server = kit.start('./test/resources/boltkit/dead_read_server.script', 9001);
+
+    kit.run(() => {
+      const driver = createDriver();
+      const session = driver.session();
+      session.run('MATCH (n) RETURN n.name').catch(error => {
+        expect(error.code).toEqual(neo4j.error.SERVICE_UNAVAILABLE);
+
+        driver.close();
+        server.exit(code => {
+          expect(code).toEqual(0);
+          done();
+        });
+      });
+    });
+  });
+
 });
 
 function createDriver() {

test/v1/driver.test.js

@@ -50,9 +50,10 @@ describe('driver', () => {
     driver = neo4j.driver("bolt://localhoste", neo4j.auth.basic("neo4j", "neo4j"));
 
     // Expect
-    driver.onError = err => {
+    driver.onError = error => {
       //the error message is different whether in browser or node
-      expect(err.message).not.toBeNull();
+      expect(error.message).not.toBeNull();
+      expect(error.code).toEqual(neo4j.error.SERVICE_UNAVAILABLE);
       done();
     };