diff --git a/packages/core/src/internal/pool/pool.ts b/packages/core/src/internal/pool/pool.ts index 18e222c1e..06920c176 100644 --- a/packages/core/src/internal/pool/pool.ts +++ b/packages/core/src/internal/pool/pool.ts @@ -262,10 +262,10 @@ class Pool { try { valid = await this._validateOnAcquire(acquisitionContext, resource) } catch (e) { - if (this._log.isErrorEnabled()) { - // eslint-disable-next-line @typescript-eslint/restrict-template-expressions - this._log.error(`Failure on validate ${resource}. This is a bug, please report it. Caused by: ${e.message}`) - } + resourceReleased(key, this._activeResourceCounts) + pool.removeInUse(resource) + await this._destroy(resource) + throw e } if (valid) { diff --git a/packages/core/test/internal/pool/pool.test.ts b/packages/core/test/internal/pool/pool.test.ts index 2c320fcc8..29721c6cf 100644 --- a/packages/core/test/internal/pool/pool.test.ts +++ b/packages/core/test/internal/pool/pool.test.ts @@ -969,10 +969,7 @@ describe('#unit Pool', () => { await conn2.release(address, conn2) }) - it.each([ - ['is not valid', (promise: any) => promise.resolve(false)], - ['validation fails', (promise: any) => promise.reject(new Error('failed'))] - ])('should create new connection if the current one when %s', async (_, resolver) => { + it('should create new connection if the current one breaks due to being invalid', async () => { const conns: any[] = [] const pool = new Pool({ // Hook into connection creation to track when and what connections that are @@ -1020,7 +1017,7 @@ describe('#unit Pool', () => { expect(conns.length).toEqual(1) // should resolve the promise with the configured value - resolver(conns[0].promises[0]) + conns[0].promises[0].resolve(false) // getting the connection 1 const conn1 = await req0 @@ -1038,6 +1035,63 @@ describe('#unit Pool', () => { expect(conns.length).toEqual(2) }) + it('should create new connection if the current one breaks from error during validation', async () => { + const conns: any[] = [] + const pool = new Pool({ + // Hook into connection creation to track when and what connections that are + // created. + create: async (_, server, release) => { + // Create a fake connection that makes it possible control when it's connected + // and released from the outer scope. + const conn: any = { + server, + release + } + conns.push(conn) + return conn + }, + validateOnAcquire: async (context, resource: any) => { + const promise = new Promise((resolve, reject) => { + if (resource.promises == null) { + resource.promises = [] + } + resource.promises.push({ + resolve, + reject + }) + }) + + return await promise + }, + // Setup pool to only allow one connection + config: new PoolConfig(1, 100000) + }) + // Make the first request for a connection, this will return a connection instantaneously + const conn0 = await pool.acquire({}, address) + expect(conns.length).toEqual(1) + + // Releasing connection back to the pool, so it can be re-acquired. + await conn0.release(address, conn0) + + // Request the same connection again, it will wait until resolve get called. + const req0 = pool.acquire({}, address) + expect(conns.length).toEqual(1) + + // should resolve the promise with the configured value + conns[0].promises[0].reject(new Error('Failed')) + await expect(async () => await req0).rejects.toThrow() + + // Request other connection, this should also resolve the same connection. + const conn2 = await pool.acquire({}, address) + expect(conns.length).toEqual(2) + + await conn2.release(address, conn2) + expect(conns.length).toEqual(2) + expect(conn0).not.toBe(conn2) + expect(idleResources(pool, address)).toBe(1) + expect(resourceInUse(pool, address)).toBe(0) + }) + it('should not time out if max pool size is not set', async () => { let counter = 0 diff --git a/packages/neo4j-driver-deno/lib/core/internal/pool/pool.ts b/packages/neo4j-driver-deno/lib/core/internal/pool/pool.ts index 0b46358d5..ecf4f1085 100644 --- a/packages/neo4j-driver-deno/lib/core/internal/pool/pool.ts +++ b/packages/neo4j-driver-deno/lib/core/internal/pool/pool.ts @@ -262,10 +262,10 @@ class Pool { try { valid = await this._validateOnAcquire(acquisitionContext, resource) } catch (e) { - if (this._log.isErrorEnabled()) { - // eslint-disable-next-line @typescript-eslint/restrict-template-expressions - this._log.error(`Failure on validate ${resource}. This is a bug, please report it. Caused by: ${e.message}`) - } + resourceReleased(key, this._activeResourceCounts) + pool.removeInUse(resource) + await this._destroy(resource) + throw e } if (valid) {