Add error logs for connecting to HTTP server (#1113)

bigmontz · fbiville · web-flow · commit 43c4590bbbdf · 2023-07-19T13:06:13.000+02:00
The driver already throws an appropriated erro when the server returns HTTP. Add this information to the log helps with future debugging process.

Co-authored-by: Florent Biville &lt;445792+fbiville@users.noreply.github.com&gt;
diff --git a/packages/bolt-connection/src/bolt/handshake.js b/packages/bolt-connection/src/bolt/handshake.js
@@ -54,14 +54,16 @@ function createHandshakeMessage (versions) {
   return handshakeBuffer
 }
 
-function parseNegotiatedResponse (buffer) {
+function parseNegotiatedResponse (buffer, log) {
   const h = [
     buffer.readUInt8(),
     buffer.readUInt8(),
     buffer.readUInt8(),
     buffer.readUInt8()
   ]
   if (h[0] === 0x48 && h[1] === 0x54 && h[2] === 0x54 && h[3] === 0x50) {
+    log.error('Handshake failed since server responded with HTTP.')
+
     throw newError(
       'Server responded HTTP. Make sure you are not trying to connect to the http endpoint ' +
         '(HTTP defaults to port 7474 whereas BOLT defaults to port 7687)'
@@ -97,9 +99,10 @@ function newHandshakeBuffer () {
  * Shake hands using the channel and return the protocol version
  *
  * @param {Channel} channel the channel use to shake hands
+ * @param {Logger} log the log object
  * @returns {Promise<HandshakeResult>} Promise of protocol version and consumeRemainingBuffer
  */
-export default function handshake (channel) {
+export default function handshake (channel, log) {
   return new Promise((resolve, reject) => {
     const handshakeErrorHandler = error => {
       reject(error)
@@ -113,7 +116,7 @@ export default function handshake (channel) {
     channel.onmessage = buffer => {
       try {
         // read the response buffer and initialize the protocol
-        const protocolVersion = parseNegotiatedResponse(buffer)
+        const protocolVersion = parseNegotiatedResponse(buffer, log)
 
         resolve({
           protocolVersion,
diff --git a/packages/bolt-connection/src/connection/connection-channel.js b/packages/bolt-connection/src/connection/connection-channel.js
@@ -53,7 +53,7 @@ export function createChannelConnection (
 
   const channel = createChannel(channelConfig)
 
-  return Bolt.handshake(channel)
+  return Bolt.handshake(channel, log)
     .then(({ protocolVersion: version, consumeRemainingBuffer }) => {
       const chunker = new Chunker(channel)
       const dechunker = new Dechunker()
diff --git a/packages/bolt-connection/test/bolt/index.test.js b/packages/bolt-connection/test/bolt/index.test.js
@@ -74,7 +74,7 @@ describe('#unit Bolt', () => {
       channel.onmessage(packedHandshakeMessage(expectedProtocolVersion))
     })
 
-    it('should handle a successful handshake with reaining buffer', done => {
+    it('should handle a successful handshake with remaining buffer', done => {
       const { channel, handshakePromise } = subject()
       const expectedProtocolVersion = 4.3
       const expectedExtraBuffer = createExtraBuffer()
@@ -114,6 +114,29 @@ describe('#unit Bolt', () => {
 
       channel.onmessage(packedHandshakeMessage(httpMagicNumber))
     })
+
+    it('should log error if the server responds with http payload', async () => {
+      const { channel, handshakePromise, log } = subject()
+      const httpMagicNumber = 1213486160
+      const logErrorSpy = jest.spyOn(log, 'error')
+
+      channel.onmessage(packedHandshakeMessage(httpMagicNumber))
+
+      await expect(handshakePromise).rejects.toThrow()
+      expect(logErrorSpy).toHaveBeenCalledWith('Handshake failed since server responded with HTTP.')
+    })
+
+    it('should not log error if the server responds with a valid protocol version', async () => {
+      const { channel, handshakePromise, log } = subject()
+      const expectedProtocolVersion = 4.3
+      const logErrorSpy = jest.spyOn(log, 'error')
+
+      channel.onmessage(packedHandshakeMessage(expectedProtocolVersion))
+
+      await expect(handshakePromise).resolves.not.toThrow()
+      expect(logErrorSpy).not.toBeCalled()
+    })
+
     it('should handle a failed handshake', done => {
       const { channel, handshakePromise } = subject()
       const expectedError = new Error('Something got wrong')
@@ -143,9 +166,11 @@ describe('#unit Bolt', () => {
     })
 
     function subject ({ channel = new DummyChannel() } = {}) {
+      const log = new Logger('debug', () => {})
       return {
+        log,
         channel,
-        handshakePromise: Bolt.handshake(channel)
+        handshakePromise: Bolt.handshake(channel, log)
       }
     }
 
diff --git a/packages/neo4j-driver-deno/lib/bolt-connection/bolt/handshake.js b/packages/neo4j-driver-deno/lib/bolt-connection/bolt/handshake.js
@@ -54,14 +54,16 @@ function createHandshakeMessage (versions) {
   return handshakeBuffer
 }
 
-function parseNegotiatedResponse (buffer) {
+function parseNegotiatedResponse (buffer, log) {
   const h = [
     buffer.readUInt8(),
     buffer.readUInt8(),
     buffer.readUInt8(),
     buffer.readUInt8()
   ]
   if (h[0] === 0x48 && h[1] === 0x54 && h[2] === 0x54 && h[3] === 0x50) {
+    log.error('Handshake failed since server responded with HTTP.')
+
     throw newError(
       'Server responded HTTP. Make sure you are not trying to connect to the http endpoint ' +
         '(HTTP defaults to port 7474 whereas BOLT defaults to port 7687)'
@@ -97,9 +99,10 @@ function newHandshakeBuffer () {
  * Shake hands using the channel and return the protocol version
  *
  * @param {Channel} channel the channel use to shake hands
+ * @param {Logger} log the log object
  * @returns {Promise<HandshakeResult>} Promise of protocol version and consumeRemainingBuffer
  */
-export default function handshake (channel) {
+export default function handshake (channel, log) {
   return new Promise((resolve, reject) => {
     const handshakeErrorHandler = error => {
       reject(error)
@@ -113,7 +116,7 @@ export default function handshake (channel) {
     channel.onmessage = buffer => {
       try {
         // read the response buffer and initialize the protocol
-        const protocolVersion = parseNegotiatedResponse(buffer)
+        const protocolVersion = parseNegotiatedResponse(buffer, log)
 
         resolve({
           protocolVersion,
diff --git a/packages/neo4j-driver-deno/lib/bolt-connection/connection/connection-channel.js b/packages/neo4j-driver-deno/lib/bolt-connection/connection/connection-channel.js
@@ -53,7 +53,7 @@ export function createChannelConnection (
 
   const channel = createChannel(channelConfig)
 
-  return Bolt.handshake(channel)
+  return Bolt.handshake(channel, log)
     .then(({ protocolVersion: version, consumeRemainingBuffer }) => {
       const chunker = new Chunker(channel)
       const dechunker = new Dechunker()
diff --git a/packages/neo4j-driver/test/internal/connection-channel.test.js b/packages/neo4j-driver/test/internal/connection-channel.test.js
@@ -133,7 +133,7 @@ describe('#integration ChannelConnection', () => {
   it('should provide error message when connecting to http-port', async done => {
     await createConnection(`bolt://${sharedNeo4j.hostname}:7474`, {
       encrypted: false
-    })
+    }, null, new Logger('error', () => {}))
       .then(done.fail.bind(done))
       .catch(error => {
         expect(error).toBeDefined()
