Logging cases where number will be ceiled

Author: bigmontz

packages/core/src/driver.ts

@@ -98,6 +98,7 @@ type CreateSession = (args: {
   bookmarkManager?: BookmarkManager
   notificationFilter?: NotificationFilter
   auth?: AuthToken
+  log: Logger
 }) => Session
 
 type CreateQueryExecutor = (createSession: (config: { database?: string, bookmarkManager?: BookmarkManager }) => Session) => QueryExecutor
@@ -827,7 +828,8 @@ class Driver {
       fetchSize,
       bookmarkManager,
       notificationFilter,
-      auth
+      auth,
+      log: this._log
     })
   }
 

packages/core/src/internal/tx-config.ts

@@ -20,6 +20,7 @@
 import * as util from './util'
 import { newError } from '../error'
 import Integer, { int } from '../integer'
+import { Logger } from './logger'
 
 /**
  * Internal holder of the transaction configuration.
@@ -35,9 +36,9 @@ export class TxConfig {
    * @constructor
    * @param {Object} config the raw configuration object.
    */
-  constructor (config: any) {
+  constructor (config: any, log?: Logger) {
     assertValidConfig(config)
-    this.timeout = extractTimeout(config)
+    this.timeout = extractTimeout(config, log)
     this.metadata = extractMetadata(config)
   }
 
@@ -63,9 +64,13 @@ const EMPTY_CONFIG = new TxConfig({})
 /**
  * @return {Integer|null}
  */
-function extractTimeout (config: any): Integer | null {
+function extractTimeout (config: any, log?: Logger): Integer | null {
   if (util.isObject(config) && config.timeout != null) {
     util.assertNumberOrInteger(config.timeout, 'Transaction timeout')
+    if (isTimeoutFloat(config) && log?.isInfoEnabled() === true) {
+      // eslint-disable-next-line @typescript-eslint/restrict-template-expressions
+      log?.info(`Transaction timeout expected to be an integer, got: ${config.timeout}. The value will be round up.`)
+    }
     const timeout = int(config.timeout, { ceilFloat: true })
     if (timeout.isNegative()) {
       throw newError('Transaction timeout should not be negative')
@@ -75,6 +80,10 @@ function extractTimeout (config: any): Integer | null {
   return null
 }
 
+function isTimeoutFloat (config: any): boolean {
+  return typeof config.timeout === 'number' && !Number.isInteger(config.timeout)
+}
+
 /**
  * @return {object|null}
  */

packages/core/src/session.ts

@@ -38,6 +38,7 @@ import ManagedTransaction from './transaction-managed'
 import BookmarkManager from './bookmark-manager'
 import { Dict } from './record'
 import NotificationFilter from './notification-filter'
+import { Logger } from './internal/logger'
 
 type ConnectionConsumer = (connection: Connection | null) => any | undefined | Promise<any> | Promise<undefined>
 type TransactionWork<T> = (tx: Transaction) => Promise<T> | T
@@ -74,6 +75,7 @@ class Session {
   private readonly _results: Result[]
   private readonly _bookmarkManager?: BookmarkManager
   private readonly _notificationFilter?: NotificationFilter
+  private readonly _log?: Logger
   /**
    * @constructor
    * @protected
@@ -100,7 +102,8 @@ class Session {
     impersonatedUser,
     bookmarkManager,
     notificationFilter,
-    auth
+    auth,
+    log
   }: {
     mode: SessionMode
     connectionProvider: ConnectionProvider
@@ -113,6 +116,7 @@ class Session {
     bookmarkManager?: BookmarkManager
     notificationFilter?: NotificationFilter
     auth?: AuthToken
+    log: Logger
   }) {
     this._mode = mode
     this._database = database
@@ -153,6 +157,7 @@ class Session {
     this._results = []
     this._bookmarkManager = bookmarkManager
     this._notificationFilter = notificationFilter
+    this._log = log
   }
 
   /**
@@ -176,7 +181,7 @@ class Session {
       parameters
     )
     const autoCommitTxConfig = (transactionConfig != null)
-      ? new TxConfig(transactionConfig)
+      ? new TxConfig(transactionConfig, this._log)
       : TxConfig.empty()
 
     const result = this._run(validatedQuery, params, async connection => {
@@ -279,7 +284,7 @@ class Session {
 
     let txConfig = TxConfig.empty()
     if (arg != null) {
-      txConfig = new TxConfig(arg)
+      txConfig = new TxConfig(arg, this._log)
     }
 
     return this._beginTransaction(this._mode, txConfig)
@@ -385,7 +390,7 @@ class Session {
     transactionWork: TransactionWork<T>,
     transactionConfig?: TransactionConfig
   ): Promise<T> {
-    const config = new TxConfig(transactionConfig)
+    const config = new TxConfig(transactionConfig, this._log)
     return this._runTransaction(ACCESS_MODE_READ, config, transactionWork)
   }
 
@@ -410,7 +415,7 @@ class Session {
     transactionWork: TransactionWork<T>,
     transactionConfig?: TransactionConfig
   ): Promise<T> {
-    const config = new TxConfig(transactionConfig)
+    const config = new TxConfig(transactionConfig, this._log)
     return this._runTransaction(ACCESS_MODE_WRITE, config, transactionWork)
   }
 
@@ -443,7 +448,7 @@ class Session {
     transactionWork: ManagedTransactionWork<T>,
     transactionConfig?: TransactionConfig
   ): Promise<T> {
-    const config = new TxConfig(transactionConfig)
+    const config = new TxConfig(transactionConfig, this._log)
     return this._executeInTransaction(ACCESS_MODE_READ, config, transactionWork)
   }
 
@@ -465,7 +470,7 @@ class Session {
     transactionWork: ManagedTransactionWork<T>,
     transactionConfig?: TransactionConfig
   ): Promise<T> {
-    const config = new TxConfig(transactionConfig)
+    const config = new TxConfig(transactionConfig, this._log)
     return this._executeInTransaction(ACCESS_MODE_WRITE, config, transactionWork)
   }
 

packages/core/test/driver.test.ts

@@ -625,6 +625,8 @@ describe('Driver', () => {
       mode: 'WRITE',
       reactive: false,
       impersonatedUser: undefined,
+      // @ts-expect-error
+      log: driver?._log,
       ...extra
     }
   }

packages/core/test/session.test.ts

@@ -19,8 +19,9 @@
 import { ConnectionProvider, Session, Connection, TransactionPromise, Transaction, BookmarkManager, bookmarkManager, NotificationFilter, int } from '../src'
 import { bookmarks } from '../src/internal'
 import { ACCESS_MODE_READ, FETCH_ALL } from '../src/internal/constants'
+import { Logger } from '../src/internal/logger'
 import ManagedTransaction from '../src/transaction-managed'
-import { AuthToken } from '../src/types'
+import { AuthToken, LoggerFunction } from '../src/types'
 import FakeConnection from './utils/connection.fake'
 import { validNotificationFilters } from './utils/notification-filters.fixtures'
 import fc from 'fast-check'
@@ -497,6 +498,56 @@ describe('session', () => {
         )
       )
     })
+
+    it('should log a warning for timeout configurations with sub milliseconds', async () => {
+      return await fc.assert(
+        fc.asyncProperty(
+          fc
+            .float({ min: 0, noNaN: true })
+            .filter((timeout: number) => !Number.isInteger(timeout)),
+          async (timeout: number) => {
+            const connection = mockBeginWithSuccess(newFakeConnection())
+
+            const { session, loggerFunction } = setupSession({
+              connection,
+              beginTx: false,
+              database: 'neo4j'
+            })
+
+            await session.beginTransaction({ timeout })
+
+            expect(loggerFunction).toBeCalledWith(
+              'info',
+              `Transaction timeout expected to be an integer, got: ${timeout}. The value will be round up.`
+            )
+          }
+        )
+      )
+    })
+
+    it('should not log a warning for timeout configurations without sub milliseconds', async () => {
+      return await fc.assert(
+        fc.asyncProperty(
+          fc.nat(),
+          async (timeout: number) => {
+            const connection = mockBeginWithSuccess(newFakeConnection())
+
+            const { session, loggerFunction } = setupSession({
+              connection,
+              beginTx: false,
+              database: 'neo4j'
+            })
+
+            await session.beginTransaction({ timeout })
+
+            expect(loggerFunction).not.toBeCalledWith(
+              'info',
+              `Transaction timeout expected to be an integer, got: ${timeout}. The value will be round up.`
+            )
+          }
+        )
+      )
+    })
   })
 
   describe('.commit()', () => {
@@ -634,6 +685,68 @@ describe('session', () => {
         )
       )
     })
+
+    it('should log a warning for timeout configurations with sub milliseconds', async () => {
+      return await fc.assert(
+        fc.asyncProperty(
+          fc
+            .float({ min: 0, noNaN: true })
+            .filter((timeout: number) => !Number.isInteger(timeout)),
+          async (timeout: number) => {
+            const connection = mockBeginWithSuccess(newFakeConnection())
+            const { session, loggerFunction } = setupSession({
+              connection,
+              beginTx: false,
+              fetchSize: FETCH_ALL
+            })
+
+            // @ts-expect-error
+            jest.spyOn(Transaction.prototype, 'run').mockImplementation(async () => await Promise.resolve())
+            const query = 'RETURN $a'
+            const params = { a: 1 }
+
+            await execute(session)(async (tx: ManagedTransaction) => {
+              await tx.run(query, params)
+            }, { timeout })
+
+            expect(loggerFunction).toBeCalledWith(
+              'info',
+              `Transaction timeout expected to be an integer, got: ${timeout}. The value will be round up.`
+            )
+          }
+        )
+      )
+    })
+
+    it('should not log a warning for timeout configurations without sub milliseconds', async () => {
+      return await fc.assert(
+        fc.asyncProperty(
+          fc.nat(),
+          async (timeout: number) => {
+            const connection = mockBeginWithSuccess(newFakeConnection())
+            const { session, loggerFunction } = setupSession({
+              connection,
+              beginTx: false,
+              fetchSize: FETCH_ALL
+            })
+
+            // @ts-expect-error
+            jest.spyOn(Transaction.prototype, 'run').mockImplementation(async () => await Promise.resolve())
+            const query = 'RETURN $a'
+            const params = { a: 1 }
+
+            await execute(session)(async (tx: ManagedTransaction) => {
+              await tx.run(query, params)
+            }, { timeout })
+
+            expect(loggerFunction).not.toBeCalledWith(
+              'info',
+              `Transaction timeout expected to be an integer, got: ${timeout}. The value will be round up.`
+            )
+          }
+        )
+      )
+    })
   })
 
   describe('.run()', () => {
@@ -986,6 +1099,56 @@ describe('session', () => {
         )
       )
     })
+
+    it('should log a warning for timeout configurations with sub milliseconds', async () => {
+      return await fc.assert(
+        fc.asyncProperty(
+          fc
+            .float({ min: 0, noNaN: true })
+            .filter((timeout: number) => !Number.isInteger(timeout)),
+          async (timeout: number) => {
+            const connection = newFakeConnection()
+
+            const { session, loggerFunction } = setupSession({
+              connection,
+              beginTx: false,
+              database: 'neo4j'
+            })
+
+            await session.run('query', {}, { timeout })
+
+            expect(loggerFunction).toBeCalledWith(
+              'info',
+              `Transaction timeout expected to be an integer, got: ${timeout}. The value will be round up.`
+            )
+          }
+        )
+      )
+    })
+
+    it('should not log a warning for timeout configurations without sub milliseconds', async () => {
+      return await fc.assert(
+        fc.asyncProperty(
+          fc.nat(),
+          async (timeout: number) => {
+            const connection = newFakeConnection()
+
+            const { session, loggerFunction } = setupSession({
+              connection,
+              beginTx: false,
+              database: 'neo4j'
+            })
+
+            await session.run('query', {}, { timeout })
+
+            expect(loggerFunction).not.toBeCalledWith(
+              'info',
+              `Transaction timeout expected to be an integer, got: ${timeout}. The value will be round up.`
+            )
+          }
+        )
+      )
+    })
   })
 })
 
@@ -1049,8 +1212,10 @@ function setupSession ({
   bookmarkManager?: BookmarkManager
   notificationFilter?: NotificationFilter
   auth?: AuthToken
-}): { session: Session, connectionProvider: ConnectionProvider } {
+}): { session: Session, connectionProvider: ConnectionProvider, loggerFunction: LoggerFunction } {
   const connectionProvider = new ConnectionProvider()
+  const loggerFunction = jest.fn()
+  const log = new Logger('debug', loggerFunction)
   connectionProvider.acquireConnection = jest.fn(async () => await Promise.resolve(connection))
   connectionProvider.close = async () => await Promise.resolve()
 
@@ -1064,13 +1229,14 @@ function setupSession ({
     bookmarks: lastBookmarks,
     bookmarkManager,
     notificationFilter,
-    auth
+    auth,
+    log
   })
 
   if (beginTx) {
     session.beginTransaction().catch(e => { }) // force session to acquire new connection
   }
-  return { session, connectionProvider }
+  return { session, connectionProvider, loggerFunction }
 }
 
 function newFakeConnection (): FakeConnection {