Fix serialization of broken objects in the driver logs (#983)

* Fix serialization of broken objects in the driver logs

Trying to logging broken objects was causing the driver fails in unexpected moments.
This broken behaviour makes the driver fail different when the `debug` log is enabled.

Co-authored-by: Robsdedude <[email protected]>

Author: bigmontz

packages/bolt-connection/src/packstream/packstream-v1.js

@@ -32,7 +32,7 @@ import {
   internal
 } from 'neo4j-driver-core'
 
-const { util } = internal
+const { objectUtil } = internal
 const { PROTOCOL_ERROR } = error
 
 const TINY_STRING = 0x80
@@ -575,7 +575,7 @@ class Unpacker {
         return null
       }
     } catch (error) {
-      return util.createBrokenObject(error)
+      return objectUtil.createBrokenObject(error)
     }
 
   }

packages/core/src/internal/index.ts

@@ -31,6 +31,7 @@ import * as urlUtil from './url-util'
 import * as serverAddress from './server-address'
 import * as resolver from './resolver'
 import * as retryStrategy from './retry-strategy'
+import * as objectUtil from './object-util'
 
 export {
   util,
@@ -46,5 +47,6 @@ export {
   urlUtil,
   serverAddress,
   resolver,
-  retryStrategy
+  retryStrategy,
+  objectUtil
 }

packages/core/src/internal/object-util.ts

@@ -0,0 +1,87 @@
+/**
+ * Copyright (c) "Neo4j"
+ * Neo4j Sweden AB [http://neo4j.com]
+ *
+ * This file is part of Neo4j.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+// eslint-disable-next-line @typescript-eslint/naming-convention
+const __isBrokenObject__ = '__isBrokenObject__'
+// eslint-disable-next-line @typescript-eslint/naming-convention
+const __reason__ = '__reason__'
+
+/**
+ * Creates a object on which all method calls will throw the given error
+ *
+ * @param {Error} error The error
+ * @param {any} object The object. Default: {}
+ * @returns {any} A broken object
+ */
+function createBrokenObject<T extends object> (error: Error, object: any = {}): T {
+  const fail: <T>() => T = () => {
+    throw error
+  }
+
+  return new Proxy(object, {
+    get: (_: T, p: string | Symbol): any => {
+      if (p === __isBrokenObject__) {
+        return true
+      } else if (p === __reason__) {
+        return error
+      } else if (p === 'toJSON') {
+        return undefined
+      }
+      fail()
+    },
+    set: fail,
+    apply: fail,
+    construct: fail,
+    defineProperty: fail,
+    deleteProperty: fail,
+    getOwnPropertyDescriptor: fail,
+    getPrototypeOf: fail,
+    has: fail,
+    isExtensible: fail,
+    ownKeys: fail,
+    preventExtensions: fail,
+    setPrototypeOf: fail
+  })
+}
+
+/**
+ * Verifies if it is a Broken Object
+ * @param {any} object The object
+ * @returns {boolean} If it was created with createBrokenObject
+ */
+function isBrokenObject (object: any): boolean {
+  return object !== null && typeof object === 'object' && object[__isBrokenObject__] === true
+}
+
+/**
+ * Returns if the reason the object is broken.
+ *
+ * This method should only be called with instances create with {@link createBrokenObject}
+ *
+ * @param {any} object The object
+ * @returns {Error} The reason the object is broken
+ */
+function getBrokenObjectReason (object: any): Error {
+  return object[__reason__]
+}
+
+export {
+  createBrokenObject,
+  isBrokenObject,
+  getBrokenObjectReason
+}

packages/core/src/internal/util.ts

@@ -24,7 +24,6 @@ import { stringify } from '../json'
 
 const ENCRYPTION_ON: EncryptionLevel = 'ENCRYPTION_ON'
 const ENCRYPTION_OFF: EncryptionLevel = 'ENCRYPTION_OFF'
-
 /**
  * Verifies if the object is null or empty
  * @param obj The subject object
@@ -223,35 +222,6 @@ function isString(str: any): str is string {
   return Object.prototype.toString.call(str) === '[object String]'
 }
 
-/**
- * Creates a object which all method call will throw the given error
- * 
- * @param {Error} error The error
- * @param {any} object The object. Default: {}
- * @returns {any} A broken object
- */
-function createBrokenObject<T extends object> (error: Error, object: any = {}): T {
-  const fail = () => {
-    throw error
-  }
-
-  return new Proxy(object, {
-    get: fail,
-    set: fail,
-    apply: fail,
-    construct: fail,
-    defineProperty: fail,
-    deleteProperty: fail,
-    getOwnPropertyDescriptor: fail,
-    getPrototypeOf: fail,
-    has: fail,
-    isExtensible: fail,
-    ownKeys: fail,
-    preventExtensions: fail,
-    setPrototypeOf: fail,
-  })
-}
-
 export {
   isEmptyObjectOrNull,
   isObject,
@@ -263,6 +233,5 @@ export {
   assertValidDate,
   validateQueryAndParameters,
   ENCRYPTION_ON,
-  ENCRYPTION_OFF,
-  createBrokenObject
+  ENCRYPTION_OFF
 }

packages/core/src/json.ts

@@ -17,14 +17,25 @@
  * limitations under the License.
  */
 
+import { isBrokenObject, getBrokenObjectReason } from './internal/object-util'
+
 /**
  * Custom version on JSON.stringify that can handle values that normally don't support serialization, such as BigInt.
  * @private
  * @param val A JavaScript value, usually an object or array, to be converted.
  * @returns A JSON string representing the given value.
  */
-export function stringify (val: any) {
-  return JSON.stringify(val, (_, value) =>
-    typeof value === 'bigint' ? `${value}n` : value
-  )
+export function stringify (val: any): string {
+  return JSON.stringify(val, (_, value) => {
+    if (isBrokenObject(value)) {
+      return {
+        __isBrokenObject__: true,
+        __reason__: getBrokenObjectReason(value)
+      }
+    }
+    if (typeof value === 'bigint') {
+      return `${value}n`
+    }
+    return value
+  })
 }

packages/core/test/__snapshots__/json.test.ts.snap

@@ -0,0 +1,13 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`json .stringify should handle BigInt 1`] = `"\\"42n\\""`;
+
+exports[`json .stringify should handle BigInt in a list 1`] = `"[\\"42n\\",\\"-24n\\"]"`;
+
+exports[`json .stringify should handle BigInt in a object 1`] = `"{\\"theResponse\\":\\"42n\\"}"`;
+
+exports[`json .stringify should handle objects created with createBrokenObject 1`] = `"{\\"__isBrokenObject__\\":true,\\"__reason__\\":{\\"code\\":\\"N/A\\",\\"name\\":\\"Neo4jError\\"}}"`;
+
+exports[`json .stringify should handle objects created with createBrokenObject in list 1`] = `"[{\\"__isBrokenObject__\\":true,\\"__reason__\\":{\\"code\\":\\"N/A\\",\\"name\\":\\"Neo4jError\\"}}]"`;
+
+exports[`json .stringify should handle objects created with createBrokenObject inside other object 1`] = `"{\\"number\\":1,\\"broken\\":{\\"__isBrokenObject__\\":true,\\"__reason__\\":{\\"code\\":\\"N/A\\",\\"name\\":\\"Neo4jError\\"}}}"`;

packages/core/test/internal/object-util.test.ts

@@ -0,0 +1,67 @@
+/**
+ * Copyright (c) "Neo4j"
+ * Neo4j Sweden AB [http://neo4j.com]
+ *
+ * This file is part of Neo4j.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+import { newError } from '../../src'
+import {
+  createBrokenObject,
+  isBrokenObject,
+  getBrokenObjectReason
+} from '../../src/internal/object-util'
+
+describe('isBrokenObject', () => {
+  it('should return true when object created with createBrokenObject', () => {
+    const object = createBrokenObject(newError('error'), {})
+
+    expect(isBrokenObject(object)).toBe(true)
+  })
+
+  it('should return false for regular objects', () => {
+    const object = {}
+
+    expect(isBrokenObject(object)).toBe(false)
+  })
+
+  it('should return false for non-objects', () => {
+    expect(isBrokenObject(null)).toBe(false)
+    expect(isBrokenObject(undefined)).toBe(false)
+    expect(isBrokenObject(1)).toBe(false)
+    expect(isBrokenObject(() => {})).toBe(false)
+    expect(isBrokenObject('string')).toBe(false)
+  })
+})
+
+describe('getBrokenObjectReason', () => {
+  it('should return the reason the object is broken', () => {
+    const reason = newError('error')
+    const object = createBrokenObject(reason, {})
+
+    expect(getBrokenObjectReason(object)).toBe(reason)
+  })
+})
+
+describe('createBrokenObject', () => {
+  describe('toJSON', () => {
+    it('should return undefined', () => {
+      const reason = newError('error')
+      const object = createBrokenObject(reason, {})
+
+      // @ts-expect-error
+      expect(object.toJSON).toBeUndefined()
+    })
+  })
+})

packages/core/test/json.test.ts

@@ -0,0 +1,69 @@
+/**
+ * Copyright (c) "Neo4j"
+ * Neo4j Sweden AB [http://neo4j.com]
+ *
+ * This file is part of Neo4j.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+import { json, newError } from '../src'
+import { createBrokenObject } from '../src/internal/object-util'
+
+describe('json', () => {
+  describe('.stringify', () => {
+    it('should handle objects created with createBrokenObject', () => {
+      const reason = newError('some error')
+      const broken = createBrokenObject(reason, { })
+
+      expect(json.stringify(broken)).toMatchSnapshot()
+    })
+
+    it('should handle objects created with createBrokenObject in list', () => {
+      const reason = newError('some error')
+      const broken = createBrokenObject(reason, { })
+
+      expect(json.stringify([broken])).toMatchSnapshot()
+    })
+
+    it('should handle objects created with createBrokenObject inside other object', () => {
+      const reason = newError('some error')
+      const broken = createBrokenObject(reason, { })
+
+      expect(json.stringify({
+        number: 1,
+        broken
+      })).toMatchSnapshot()
+    })
+
+    it('should handle BigInt', () => {
+      const bigint = BigInt(42)
+
+      expect(json.stringify(bigint)).toMatchSnapshot()
+    })
+
+    it('should handle BigInt in a list', () => {
+      const bigintList = [BigInt(42), BigInt(-24)]
+
+      expect(json.stringify(bigintList)).toMatchSnapshot()
+    })
+
+    it('should handle BigInt in a object', () => {
+      const bigintInObject = {
+        theResponse: BigInt(42)
+      }
+
+      expect(json.stringify(bigintInObject)).toMatchSnapshot()
+    })
+  })
+})