diff --git a/lib/connection/connection_config.js b/lib/connection/connection_config.js index 81950265d..86a49212c 100644 --- a/lib/connection/connection_config.js +++ b/lib/connection/connection_config.js @@ -51,6 +51,7 @@ const DEFAULT_PARAMS = 'forceStageBindError', 'includeRetryReason', 'disableQueryContextCache', + 'loginTimeout', ]; const Logger = require('../logger'); @@ -484,6 +485,14 @@ function ConnectionConfig(options, validateCredentials, qaMode, clientInfo) disableQueryContextCache = options.disableQueryContextCache; } + let loginTimeout = 300; + if (Util.exists(options.loginTimeout)) { + Errors.checkArgumentValid(Util.isNumber(options.loginTimeout), + ErrorCodes.ERR_CONN_CREATE_INVALID_MAX_LOGIN_TIMEOUT); + + loginTimeout = Math.max(loginTimeout,options.loginTimeout); + } + if (validateDefaultParameters) { for (const [key] of Object.entries(options)) @@ -794,6 +803,15 @@ function ConnectionConfig(options, validateCredentials, qaMode, clientInfo) return clientConfigFile; }; + /** + * Returns the max login timeout + * + * @returns {Number} + */ + this.getLoginTimeout = function () { + return loginTimeout; + } + // save config options this.username = options.username; this.password = options.password; @@ -949,7 +967,7 @@ function createParameters() }, { name: PARAM_RETRY_SF_MAX_LOGIN_RETRIES, - defaultValue: 5, + defaultValue: 7, external: true, validate: isNonNegativeInteger }, @@ -960,7 +978,7 @@ function createParameters() }, { name: PARAM_RETRY_SF_STARTING_SLEEP_TIME, - defaultValue: 0.25, + defaultValue: 4, validate: isNonNegativeNumber }, { diff --git a/lib/constants/error_messages.js b/lib/constants/error_messages.js index ad6b407c2..fe7157917 100644 --- a/lib/constants/error_messages.js +++ b/lib/constants/error_messages.js @@ -68,6 +68,7 @@ exports[404040] = 'Invalid browser timeout value. The specified value must be a exports[404041] = 'Invalid disablQueryContextCache. The specified value must be a boolean.'; exports[404042] = 'Invalid includeRetryReason. The specified value must be a boolean.' exports[404043] = 'Invalid clientConfigFile value. The specified value must be a string.'; +exports[404044] = 'Invalid loginTimeout value. The specified value must be a number.'; // 405001 exports[405001] = 'Invalid callback. The specified value must be a function.'; diff --git a/lib/errors.js b/lib/errors.js index c04fe7b5a..3b08a888b 100644 --- a/lib/errors.js +++ b/lib/errors.js @@ -73,6 +73,7 @@ codes.ERR_CONN_CREATE_INVALID_BROWSER_TIMEOUT = 404040; codes.ERR_CONN_CREATE_INVALID_DISABLED_QUERY_CONTEXT_CACHE = 404041 codes.ERR_CONN_CREATE_INVALID_INCLUDE_RETRY_REASON =404042 codes.ERR_CONN_CREATE_INVALID_CLIENT_CONFIG_FILE = 404043; +codes.ERR_CONN_CREATE_INVALID_LOGIN_TIMEOUT = 404044; // 405001 codes.ERR_CONN_CONNECT_INVALID_CALLBACK = 405001; diff --git a/lib/services/sf.js b/lib/services/sf.js index c183bad70..2ebfa0664 100644 --- a/lib/services/sf.js +++ b/lib/services/sf.js @@ -782,6 +782,13 @@ function StateAbstract(options) requestOptions.headers = Util.apply(this.getDefaultReqHeaders(), requestOptions.headers || {}); + if (Util.isLoginRequest(requestOptions.url)) { + Util.apply(requestOptions.headers, { + "CLIENT_APP_VERSION": requestOptions.json.data.CLIENT_APP_VERSION, + "CLIENT_APP_ID": requestOptions.json.data.CLIENT_APP_ID, + }) + } + // augment the options with the absolute url requestOptions.absoluteUrl = this.buildFullUrl(requestOptions.url); }; @@ -1172,9 +1179,10 @@ StateConnecting.prototype.continue = function () const startTime = connectionConfig.accessUrl.startsWith('https://') ? Date.now() : 'FIXEDTIMESTAMP'; const maxLoginRetries = connectionConfig.getRetrySfMaxLoginRetries(); - let sleep = connectionConfig.getRetrySfStartingSleepTime(); - const cap = connectionConfig.getRetrySfMaxSleepTime(); - Logger.getInstance().debug("Retry Max sleep time = " + cap); + const maxLoginTimeout = connectionConfig.getLoginTimeout(); + let sleep = connectionConfig.getRetrySfStartingSleepTime();; + let totalTimeout = sleep; + Logger.getInstance().debug("Total loginTimeout is for the retries = " + maxLoginTimeout); const parent = this; const requestCallback = function (err, body) { @@ -1204,10 +1212,18 @@ StateConnecting.prototype.continue = function () if (Errors.isNetworkError(err) || Errors.isRequestFailedError(err)) { if (numRetries < maxLoginRetries && ( - isRetryableNetworkError(err) || isRetryableHttpError(err))) + isRetryableNetworkError(err) || isRetryableHttpError(err)) && + totalTimeout < maxLoginTimeout) { numRetries++; - sleep = Util.nextSleepTime(1, cap, sleep); + const jitter = Util.jitteredSleepTime(numRetries, sleep, totalTimeout, maxLoginTimeout); + sleep = jitter.sleep; + totalTimeout = jitter.totalTimeout; + + if(sleep <= 0) { + Logger.getInstance().debug("Reached out to the Login Timeout"); + parent.snowflakeService.transitionToDisconnected(); + } setTimeout(sendRequest, sleep * 1000); return; } diff --git a/lib/util.js b/lib/util.js index 8c822b4f1..4deb72c7b 100644 --- a/lib/util.js +++ b/lib/util.js @@ -405,7 +405,7 @@ exports.isNode = function () /** * Returns the next sleep time calculated by exponential backoff with * decorrelated jitter. - * sleep = min(cap, random_between(base, sleep * 3)) + * sleep = min(cap, random_between(base, sleep * 3)) * for more details, check out: * http://www.awsarchitectureblog.com/2015/03/backoff.html * @param base minimum seconds @@ -420,6 +420,63 @@ exports.nextSleepTime = function ( Math.min(base, previousSleep * 3)); }; + +/** + * Return next sleep time calculated by the jitter rule. + * + * @param {Number} numofRetries + * @param {Number} currentSleepTime + * @param {Number} totalTimeout + * @param {Number} maxLoginTimeout + * @returns {JSON} return next sleep Time and totalTime. + */ +exports.jitteredSleepTime = function (numofRetries, currentSleepTime, totalTimeout, maxLoginTimeout) { + const sleep = Math.min((maxLoginTimeout - totalTimeout), getNextSleepTime(numofRetries, currentSleepTime) ); + totalTimeout += sleep + return {sleep, totalTimeout} +} + +/** + * Choose one of the number between two numbers. + * + * @param {Number} firstNumber + * @param {Number} secondNumber + * @returns {Boolean} return a random number between two numbers. + */ +function chooseRandom (firstNumber, secondNumber) { + let bigNumber = Math.max(firstNumber, secondNumber); + let smallNumber = Math.min(firstNumber, secondNumber); + + return (Math.random() * (bigNumber - smallNumber)) + smallNumber; +} + +exports.chooseRandom = chooseRandom; + +/** + * return the jitter value. + * @param {Number} numofRetries + * @param {Number} currentSleepTime + * @returns {Boolean} return jitter. + */ +function getNextSleepTime (numofRetries, currentSleepTime) { + const multiplicationFactor = chooseRandom(1, -1); + const nextSleep = (2 ** (numofRetries)); + const jitterAmount = 0.5 * currentSleepTime * multiplicationFactor; + + return nextSleep + jitterAmount; +} +exports.getNextSleepTime = getNextSleepTime; +/** + * Check whether the request is the login-request or not. + * + * @param loginurl HTTP request url + * @returns {Boolean} true if it is loginRequest, otherwise false. + */ +exports.isLoginRequest = function (loginUrl) { + const endPoints = ['/v1/login-request', '/authenticator-request', '/token-request',]; + return endPoints.some((endPoint) => loginUrl.includes(endPoint)); +} + /** * Checks if the HTTP response code is retryable * diff --git a/test/integration/ocsp_mock/testConnectionOcspMock.js b/test/integration/ocsp_mock/testConnectionOcspMock.js index ff2361e9e..a1a7ea598 100644 --- a/test/integration/ocsp_mock/testConnectionOcspMock.js +++ b/test/integration/ocsp_mock/testConnectionOcspMock.js @@ -23,6 +23,8 @@ function cloneConnOption(connOption) describe('Connection test with OCSP Mock', function () { + this.timeout(300000); + const valid = cloneConnOption(connOption.valid); const isHttps = valid.accessUrl.startsWith("https"); diff --git a/test/unit/connection/connection_config_test.js b/test/unit/connection/connection_config_test.js index e331a579a..d21250fd1 100644 --- a/test/unit/connection/connection_config_test.js +++ b/test/unit/connection/connection_config_test.js @@ -546,6 +546,16 @@ describe('ConnectionConfig: basic', function () }, errorCode: ErrorCodes.ERR_CONN_CREATE_INVALID_CLIENT_CONFIG_FILE }, + { + name: 'invalid maxLoginTimeout', + options: { + account: 'account', + username: 'username', + password: 'password', + loginTimeout: 'invalud' + }, + errorCode: ErrorCodes.ERR_CONN_CREATE_INVALID_MAX_LOGIN_TIMEOUT + }, ]; var createNegativeITCallback = function (testCase) @@ -887,13 +897,13 @@ describe('ConnectionConfig: basic', function () username: 'username', password: 'password', account: 'account', - disableQueryContextCache: true + disableQueryContextCache: true, }, options: { accessUrl: 'https://account.snowflakecomputing.com', username: 'username', - password: 'password' + password: 'password', } }, { @@ -914,6 +924,23 @@ describe('ConnectionConfig: basic', function () clientConfigFile: 'easy_logging_config.json' } }, + { + name: 'login time out', + input: + { + account: 'account', + username: 'username', + password: 'password', + loginTimeout: 1234, + }, + options: + { + accessUrl: 'https://account.snowflakecomputing.com', + username: 'username', + password: 'password', + account: 'account', + } + }, ]; var createItCallback = function (testCase) diff --git a/test/unit/mock/mock_http_client.js b/test/unit/mock/mock_http_client.js index 05d3c346b..551733cf7 100644 --- a/test/unit/mock/mock_http_client.js +++ b/test/unit/mock/mock_http_client.js @@ -174,7 +174,9 @@ function buildRequestOutputMappings(clientInfo) headers: { 'Accept': 'application/json', - 'Content-Type': 'application/json' + 'Content-Type': 'application/json', + "CLIENT_APP_VERSION": clientInfo.version, + "CLIENT_APP_ID": "JavaScript", }, json: { @@ -1158,7 +1160,9 @@ function buildRequestOutputMappings(clientInfo) headers: { 'Accept': 'application/json', - 'Content-Type': 'application/json' + 'Content-Type': 'application/json', + "CLIENT_APP_VERSION": clientInfo.version, + "CLIENT_APP_ID": "JavaScript" }, json: { @@ -1373,7 +1377,9 @@ function buildRequestOutputMappings(clientInfo) headers: { 'Accept': 'application/json', - 'Content-Type': 'application/json' + 'Content-Type': 'application/json', + "CLIENT_APP_VERSION": clientInfo.version, + "CLIENT_APP_ID": "JavaScript" }, json: { @@ -1478,7 +1484,9 @@ function buildRequestOutputMappings(clientInfo) headers: { 'Accept': 'application/json', - 'Content-Type': 'application/json' + 'Content-Type': 'application/json', + "CLIENT_APP_VERSION": clientInfo.version, + "CLIENT_APP_ID": "JavaScript" }, json: { @@ -1583,7 +1591,9 @@ function buildRequestOutputMappings(clientInfo) headers: { 'Accept': 'application/json', - 'Content-Type': 'application/json' + 'Content-Type': 'application/json', + "CLIENT_APP_VERSION": clientInfo.version, + "CLIENT_APP_ID": "JavaScript" }, json: { @@ -1670,7 +1680,9 @@ function buildRequestOutputMappings(clientInfo) headers: { 'Accept': 'application/json', - 'Content-Type': 'application/json' + 'Content-Type': 'application/json', + "CLIENT_APP_VERSION": clientInfo.version, + "CLIENT_APP_ID": "JavaScript" }, json: { @@ -1706,7 +1718,10 @@ function buildRequestOutputMappings(clientInfo) headers: { 'Accept': 'application/json', - 'Content-Type': 'application/json' + 'Content-Type': 'application/json', + "CLIENT_APP_VERSION": clientInfo.version, + "CLIENT_APP_ID": "JavaScript" + }, json: { @@ -1742,7 +1757,9 @@ function buildRequestOutputMappings(clientInfo) headers: { 'Accept': 'application/json', - 'Content-Type': 'application/json' + 'Content-Type': 'application/json', + "CLIENT_APP_VERSION": clientInfo.version, + "CLIENT_APP_ID": "JavaScript" }, json: { diff --git a/test/unit/util_test.js b/test/unit/util_test.js index 6a0e11e57..8b90a4683 100644 --- a/test/unit/util_test.js +++ b/test/unit/util_test.js @@ -520,6 +520,131 @@ describe('Util', function () assert.strictEqual(url, testCase.result); }) } + }); + + describe("Util.isLoginRequest Test", function () { + const baseUrl = 'wwww.test.com'; + const testCases = + [ + { + testName: 'test URL with a right login end point', + endPoint: '/v1/login-request', + result: true, + }, + { + testName: 'test URL with a wrong login end point', + endPoint: '/login-request', + result: false, + }, + { + testName: 'test URL with a right authenticator-request point', + endPoint: '/authenticator-request', + result:true, + }, + { + testName: 'test URL with a wrong authenticator-request point', + endPoint: '/authenticator-requ', + result:false, + }, + { + testName: 'test URL with a right token point', + endPoint: '/token-request', + result:true, + }, + { + testName: 'test URL with a wrong token point', + endPoint: '/tokenRequest', + result:false, + }, + ]; + + for (const {testName, endPoint,result} of testCases) + { + it(testName, function () { + const isLoginRequest = Util.isLoginRequest(baseUrl + endPoint); + assert.strictEqual(isLoginRequest, result); + }) + } + }); + + it("Util.jitterSleepTime Test", function () { + const errorCodes = + [ + { + statusCode: 403, + retry403: true, + isRetryable: true, + }, + { + statusCode: 408, + retry403: false, + isRetryable: true, + }, + { + statusCode: 429, + retry403: false, + isRetryable: true, + }, + { + statusCode: 500, + retry403: false, + isRetryable: true, + }, + { + statusCode: 503, + retry403: false, + isRetryable: true, + }, + { + statusCode: 538, + retry403: false, + isRetryable: true, + }, + ]; + + const maxLoginTimeout = 300; + let currentSleepTime = 4; + let retryCount = 1; + let totalTimeout = currentSleepTime; + for (const response of errorCodes) { + retryCount++; + assert.strictEqual(Util.isRetryableHttpError(response,true), true); + + const result = Util.jitteredSleepTime(retryCount, currentSleepTime, totalTimeout, maxLoginTimeout); + const jitter = currentSleepTime / 2 + const nextSleep = 2 ** retryCount; + currentSleepTime = result.sleep; + totalTimeout = result.totalTimeout; + + assert.ok(currentSleepTime <= nextSleep + jitter || currentSleepTime >= nextSleep - jitter) + } + + assert.strictEqual(retryCount, 7); + assert.ok(totalTimeout <= maxLoginTimeout); + }); + + it("test exponential jitter back off", function () { + const numofRetries = 10; + let sleep = 1; + let retries = []; + for(let i = 0; i < numofRetries; i++) { + retries.push(Util.getNextSleepTime(i + 1, sleep)); + sleep = retries[i]; + } + + for(let i = 0; i< numofRetries -1; i++) { + assert.ok(retries[i] < retries[i + 1]); + } + }) + + it("Util.chooseRandom Test", function () { + const positiveInteger = Util.chooseRandom(1, 5); + const negativeInteger = Util.chooseRandom(-1, -5); + assert.ok(1 <= positiveInteger && positiveInteger <= 5); + assert.ok(-5 <= negativeInteger && negativeInteger <= -1); + + const randomNumber = Util.chooseRandom(positiveInteger, negativeInteger); + assert.ok(negativeInteger <= randomNumber && randomNumber <= positiveInteger) }) it('Util.apply()', function ()