Connection.prototype.isIdle: Cannot read properties of undefined (reading 'empty')

[oklemenz2]

We face a lot of occurrences of the following exception in hdb module:

TypeError: Cannot read properties of undefined (reading 'empty')
    at Connection.isIdle (/home/vcap/app/node_modules/hdb/lib/protocol/Connection.js:770:22)
    at Connection.disconnect (/home/vcap/app/node_modules/hdb/lib/protocol/Connection.js:644:12)
    at Client.disconnect (/home/vcap/app/node_modules/hdb/lib/Client.js:218:20)
    at /home/vcap/app/node_modules/@cap-js/hana/lib/drivers/base.js:168:16
    at new Promise (<anonymous>)
    at /home/vcap/app/node_modules/@cap-js/hana/lib/drivers/base.js:167:12
    at /home/vcap/app/node_modules/@cap-js/telemetry/lib/tracing/trace.js:375:16
    at AsyncLocalStorage.run (node:internal/async_local_storage/async_context_frame:63:14)
    at AsyncLocalStorageContextManager.with (/home/vcap/app/node_modules/@opentelemetry/context-async-hooks/build/src/AsyncLocalStorageContextManager.js:33:40)
    at ContextAPI.with (/home/vcap/app/node_modules/@opentelemetry/api/build/src/api/context.js:51:46)
    at Tracer.startActiveSpan (/home/vcap/app/node_modules/@opentelemetry/sdk-trace-base/build/src/Tracer.js:121:32)
    at trace (/home/vcap/app/node_modules/@cap-js/telemetry/lib/tracing/trace.js:319:17)
    at prom (/home/vcap/app/node_modules/@cap-js/telemetry/lib/tracing/cds.js:114:24)
    at HDBDriver.disconnect (/home/vcap/app/node_modules/@cap-js/hana/lib/drivers/base.js:143:61)
    at process.processTicksAndRejections (node:internal/process/task_queues:103:5)
    at async #destroy (/home/vcap/app/node_modules/@cap-js/db-service/lib/common/generic-pool.js:261:9)

Version currently used in production: 2.27.1: https://github.com/SAP/node-hdb/tree/v2.27.1
Version used in dev: 2.29.4: https://github.com/SAP/node-hdb/tree/v2.29.4

As the code still is in master like this:

Connection.prototype.isIdle = function isIdle() {
  return this._queue.empty && !this._queue.busy;
};

We assume this is not fixed also with newest version.

Access to undefined shall not happen. Can you check, and correct the code accordingly, so that this is prevented.

Thanks.

[linjun-he-sap]

Hi @oklemenz2 ,

Thanks for your reporting. I'd like to double confirm that you're seeing this error in v2.27.1 as well, not starting from v2.29.4?
I am asking since I did a quick and brief investigation, I think this issue was introduced prior to v2.27.1.

In addition, could you please describe the reproduction or the secanrio in short so that we could confirm our fix will indeed fix the issue?
Based on the stack trace, it happens when generic-pool tries to destroy the connection, is the destroyed connection staying in the pool (idle) for a bit of time before (so that the server may already drop the connection)?

Thanks!

[oklemenz2]

Yes, I can confirm, that the issue occurs with version 2.27.1.

It's not easy to reconstruct the exact flow. It's initiated by CDS. Their generic pool wants to destroy the connection. Possible reasons could be eviction. This is then propagated to the hdb connection disconnect function. It seems, that _clearQueue is already called before, the disconnect and isIdle accessed a cleared queue. As it only happens from time to time, I suspect some race condition, where normally, it works, but in some cases, the order is different, leading to the undefined access.

Easiest fix would be of course, to check if this._queue is still valid. Harder it is to find out the exact parallel operation clearing the queue before, because this is not in the logs, I guess.

[oklemenz2]

@swaldmann Seems to happen with the new CDS pool implementation. Any idea?
It seems hdb and CDS register on socket close, probably, the execution of the socket handlers is not stable, so that the order leads to a race condition...

[johannes-vogel]

It can be simply reproduced with the following snippet:

const fn = () => {
    const hdb = require('hdb')
    const c = hdb.createClient(creds) // creds being some credentials
    c.connect((err) => {
        if (err) throw err
        console.log('connected')
        c.on('close', () => c.disconnect()) // simplifying what @cap-js/hana does
        c.emit('close') // simulating the server closes the connection
        
    })
}
fn()

[linjun-he-sap]

Issue fixed in release v2.29.5