diff --git a/src/v1/internal/pool.js b/src/v1/internal/pool.js index 355af6d6c..600d1b999 100644 --- a/src/v1/internal/pool.js +++ b/src/v1/internal/pool.js @@ -162,10 +162,10 @@ class Pool { // check if there are any pending requests const requests = this._acquireRequests[key]; if (requests) { - var pending = requests.shift(); + const pending = requests.shift(); if (pending) { - var resource = this._acquire(key); + const resource = this._acquire(key); if (resource) { pending.resolve(resource); diff --git a/src/v1/internal/util.js b/src/v1/internal/util.js index f1afd856d..88a50b9ed 100644 --- a/src/v1/internal/util.js +++ b/src/v1/internal/util.js @@ -17,7 +17,7 @@ * limitations under the License. */ -import { newError } from "../error"; +import {newError} from '../error'; const ENCRYPTION_ON = "ENCRYPTION_ON"; const ENCRYPTION_OFF = "ENCRYPTION_OFF"; @@ -125,6 +125,8 @@ function trimAndVerify(string, name, url) { } function promiseOrTimeout(timeout, otherPromise, onTimeout) { + let resultPromise = null; + const timeoutPromise = new Promise((resolve, reject) => { const id = setTimeout(() => { if (onTimeout && typeof onTimeout === 'function') { @@ -134,10 +136,22 @@ function promiseOrTimeout(timeout, otherPromise, onTimeout) { reject(newError(`Operation timed out in ${timeout} ms.`)); }, timeout); - otherPromise.then(() => clearTimeout(id), () => clearTimeout(id)); + // this "executor" function is executed immediately, even before the Promise constructor returns + // thus it's safe to initialize resultPromise variable here, where timeout id variable is accessible + resultPromise = otherPromise.then(result => { + clearTimeout(id); + return result; + }).catch(error => { + clearTimeout(id); + throw error; + }); }); - return Promise.race([ otherPromise, timeoutPromise ]); + if (resultPromise == null) { + throw new Error('Result promise not initialized'); + } + + return Promise.race([resultPromise, timeoutPromise]); } export { diff --git a/test/internal/util.test.js b/test/internal/util.test.js index c63e307e9..7b432a053 100644 --- a/test/internal/util.test.js +++ b/test/internal/util.test.js @@ -204,6 +204,47 @@ describe('util', () => { }); }); + it('should not trigger both promise and timeout', done => { + const timeout = 500; + + let timeoutFired = false; + let result = null; + let error = null; + + const resultPromise = util.promiseOrTimeout( + timeout, + new Promise(resolve => { + setTimeout(() => { + resolve(42); + }, timeout); + }), + () => { + timeoutFired = true; + } + ); + + resultPromise.then(r => { + result = r; + }).catch(e => { + error = e; + }); + + setTimeout(() => { + if (timeoutFired) { + // timeout fired - result should not be set, error should be set + expect(result).toBeNull(); + expect(error).not.toBeNull(); + expect(error.message).toEqual(`Operation timed out in ${timeout} ms.`); + done(); + } else { + // timeout did not fire - result should be set, error should not be set + expect(result).toEqual(42); + expect(error).toBeNull(); + done(); + } + }, timeout * 2); + }); + function verifyValidString(str) { expect(util.assertString(str, 'Test string')).toBe(str); }