Merge branch 'master' into SNOW-950429

snowflakedb · Oct 30, 2023 · a59dc40 · a59dc40
2 parents 3be12ac + cf17fa0
commit a59dc40
Show file tree

Hide file tree

Showing 9 changed files with 282 additions and 18 deletions.
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -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:
             {