Merge pull request neo4j#413 from lutovich/1.7-goodbye-msg

Send GOODBYE message before closing connections in Bolt V3

Author: ali-ince

src/v1/internal/bolt-protocol-v1.js

@@ -72,6 +72,10 @@ export default class BoltProtocol {
     this._connection.write(message, observer, true);
   }
 
+  prepareToClose(observer) {
+    // no need to notify the database in this protocol version
+  }
+
   /**
    * Begin an explicit transaction.
    * @param {Bookmark} bookmark the bookmark.

src/v1/internal/bolt-protocol-v3.js

@@ -47,6 +47,11 @@ export default class BoltProtocol extends BoltProtocolV2 {
     this._connection.write(message, observer, true);
   }
 
+  prepareToClose(observer) {
+    const message = RequestMessage.goodbye();
+    this._connection.write(message, observer, true);
+  }
+
   beginTransaction(bookmark, txConfig, observer) {
     prepareToHandleSingleResponse(observer);
     const message = RequestMessage.begin(bookmark, txConfig);

src/v1/internal/connection.js

@@ -382,11 +382,23 @@ export default class Connection {
    * Call close on the channel.
    * @param {function} cb - Function to call on close.
    */
-  close(cb) {
+  close(cb = (() => null)) {
     if (this._log.isDebugEnabled()) {
       this._log.debug(`${this} closing`);
     }
-    this._ch.close(cb);
+
+    if (this._protocol) {
+      // protocol has been initialized
+      // use it to notify the database about the upcoming close of the connection
+      this._protocol.prepareToClose(NO_OP_OBSERVER);
+    }
+
+    this._ch.close(() => {
+      if (this._log.isDebugEnabled()) {
+        this._log.debug(`${this} closed`);
+      }
+      cb();
+    });
   }
 
   toString() {

src/v1/internal/request-message.js

@@ -26,6 +26,7 @@ const DISCARD_ALL = 0x2F;     // 0010 1111 // DISCARD_ALL - unused
 const PULL_ALL = 0x3F;        // 0011 1111 // PULL_ALL
 
 const HELLO = 0x01;           // 0000 0001 // HELLO <metadata>
+const GOODBYE = 0x02;         // 0000 0010 // GOODBYE
 const BEGIN = 0x11;           // 0001 0001 // BEGIN <metadata>
 const COMMIT = 0x12;          // 0001 0010 // COMMIT
 const ROLLBACK = 0x13;        // 0001 0011 // ROLLBACK
@@ -125,6 +126,14 @@ export default class RequestMessage {
     return new RequestMessage(RUN, [statement, parameters, metadata],
       () => `RUN ${statement} ${JSON.stringify(parameters)} ${JSON.stringify(metadata)}`);
   }
+
+  /**
+   * Get a GOODBYE message.
+   * @return {RequestMessage} the GOODBYE message.
+   */
+  static goodbye() {
+    return GOODBYE_MESSAGE;
+  }
 }
 
 /**
@@ -152,3 +161,4 @@ const PULL_ALL_MESSAGE = new RequestMessage(PULL_ALL, [], () => 'PULL_ALL');
 const RESET_MESSAGE = new RequestMessage(RESET, [], () => 'RESET');
 const COMMIT_MESSAGE = new RequestMessage(COMMIT, [], () => 'COMMIT');
 const ROLLBACK_MESSAGE = new RequestMessage(ROLLBACK, [], () => 'ROLLBACK');
+const GOODBYE_MESSAGE = new RequestMessage(GOODBYE, [], () => 'GOODBYE');

test/internal/connection.test.js

@@ -32,6 +32,7 @@ import ConnectionErrorHandler from '../../src/v1/internal/connection-error-handl
 import testUtils from '../internal/test-utils';
 import Bookmark from '../../src/v1/internal/bookmark';
 import TxConfig from '../../src/v1/internal/tx-config';
+import boltStub from '../internal/bolt-stub';
 
 const ILLEGAL_MESSAGE = {signature: 42, fields: []};
 const SUCCESS_MESSAGE = {signature: 0x70, fields: [{}]};
@@ -346,6 +347,29 @@ describe('Connection', () => {
     connection._handleFatalError(newError('Hello', SERVICE_UNAVAILABLE));
   });
 
+  it('should send hello and goodbye messages', done => {
+    if (!boltStub.supported) {
+      done();
+      return;
+    }
+
+    const server = boltStub.start('./test/resources/boltstub/hello_goodbye.script', 9001);
+
+    boltStub.run(() => {
+      connection = createConnection('bolt://127.0.0.1:9001', {encrypted: false});
+      connection.connect('single-connection/1.2.3', basicAuthToken())
+        .then(() => {
+          connection.close(() => {
+            server.exit(code => {
+              expect(code).toEqual(0);
+              done();
+            });
+          });
+        })
+        .catch(error => done.fail(error));
+    });
+  });
+
   function packedHandshakeMessage() {
     const result = alloc(4);
     result.putInt32(0, 1);

test/internal/request-message.test.js

@@ -115,4 +115,11 @@ describe('RequestMessage', () => {
     expect(message.toString()).toEqual(`RUN ${statement} ${JSON.stringify(parameters)} ${JSON.stringify(expectedMetadata)}`);
   });
 
+  it('should create GOODBYE message', () => {
+    const message = RequestMessage.goodbye();
+
+    expect(message.signature).toEqual(0x02);
+    expect(message.fields).toEqual([]);
+    expect(message.toString()).toEqual('GOODBYE');
+  });
 });

test/resources/boltstub/hello_goodbye.script

@@ -0,0 +1,6 @@
+!: BOLT 3
+
+C: HELLO {"credentials": "password", "scheme": "basic", "user_agent": "single-connection/1.2.3", "principal": "neo4j"}
+S: SUCCESS {"server": "Neo4j/9.9.9"}
+C: GOODBYE
+S: <EXIT>

test/resources/boltstub/hello_run_goodbye.script

@@ -0,0 +1,13 @@
+!: BOLT 3
+!: AUTO RESET
+
+C: HELLO {"credentials": "password", "scheme": "basic", "user_agent": "neo4j-javascript/0.0.0-dev", "principal": "neo4j"}
+S: SUCCESS {"server": "Neo4j/9.9.9"}
+C: RUN "MATCH (n) RETURN n.name" {} {}
+   PULL_ALL
+S: SUCCESS {"fields": ["n.name"]}
+   RECORD ["Foo"]
+   RECORD ["Bar"]
+   SUCCESS {}
+C: GOODBYE
+S: <EXIT>

test/resources/boltstub/query_with_error.script

@@ -6,6 +6,6 @@ C: PULL_ALL
 S: FAILURE {"code": "Neo.ClientError.Statement.ArithmeticError", "message": "/ by zero"}
 S: IGNORED
 C: RESET
-S: SUCCESS
+S: SUCCESS {}
 C: RESET
-S: SUCCESS
+S: SUCCESS {}

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

@@ -333,4 +333,33 @@ describe('direct driver with stub server', () => {
     });
   });
 
+  it('should send goodbye message when closed', done => {
+    if (!boltStub.supported) {
+      done();
+      return;
+    }
+
+    const server = boltStub.start('./test/resources/boltstub/hello_run_goodbye.script', 9001);
+
+    boltStub.run(() => {
+      const driver = boltStub.newDriver('bolt://127.0.0.1:9001');
+      const session = driver.session();
+
+      session.run('MATCH (n) RETURN n.name')
+        .then(result => {
+          const names = result.records.map(record => record.get('n.name'));
+          expect(names).toEqual(['Foo', 'Bar']);
+
+          session.close(() => {
+            driver.close();
+            server.exit(code => {
+              expect(code).toEqual(0);
+              done();
+            });
+          });
+        })
+        .catch(error => done.fail(error));
+    });
+  });
+
 });