fix bug

snowflakedb · Dec 10, 2024 · 51b0702 · 51b0702
1 parent c531d0f
commit 51b0702
Show file tree

Hide file tree

Showing 3 changed files with 11 additions and 19 deletions.
diff --git a/lib/file_transfer_agent/gcs_util.js b/lib/file_transfer_agent/gcs_util.js
@@ -54,8 +54,8 @@ function GCSUtil(connectionConfig, httpClient, fileStream) {
   const fs = typeof fileStream !== 'undefined' ? fileStream : require('fs');
   const connectionProxy = connectionConfig.getProxy();
   const originalHttpsProxy = process.env[HTTPS_PROXY];
+  const proxyString = ProxyUtil.stringifyProxy(connectionProxy);
   let isEnvProxyOverridden = false;
-  let proxyString = null;
 
   /**
   * Retrieve the GCS token from the stage info metadata.
@@ -94,19 +94,20 @@ function GCSUtil(connectionConfig, httpClient, fileStream) {
 
     const isByPassed = isBypassProxy(connectionProxy, endPoint || `storage.${stageInfo.region.toLowerCase()}.rep.googleapis.com`);
     if (Util.isEmptyObject(connectionProxy) || isByPassed) {
-      proxyString = null;
       isEnvProxyOverridden = false;
     } else if (!Util.exists(originalHttpsProxy)) {
-      proxyString = ProxyUtil.stringifyProxy(connectionProxy);
       isEnvProxyOverridden = true;
     } else if (connectionConfig.getOverrideEnvProxy()) {
       if (proxyString !==  originalHttpsProxy) {
-        Logger.getInstance().warn('GCS proxy warning: %s',  ProxyUtil.getCompareAndLogEnvAndAgentProxies(connectionProxy).warnings);
+        Logger.getInstance().debug(`The destination host is: ${ProxyUtil.getHostFromURL(endPoint || `https://storage.${stageInfo.region.toLowerCase()}.rep.googleapis.com`)} and the proxy host is: ${connectionProxy.host}`);
         Logger.getInstance().warn(`The environment variable ${HTTPS_PROXY} value will be overwritten during the Storage HTTP calls`);
-        proxyString = ProxyUtil.stringifyProxy(connectionProxy);
         isEnvProxyOverridden = true;
       }
     }
+
+    if (isEnvProxyOverridden) {
+      Logger.getInstance().trace(`Initializing the proxy information for the GCS Client: ${ProxyUtil.describeProxy(connectionProxy)}`);
+    }
 
     if (typeof httpClient === 'undefined') {
       const proxyAgent = getProxyAgent(connectionProxy, new URL(connectionConfig.accessUrl), endPoint || `storage.${stageInfo.region.toLowerCase()}.rep.googleapis.com` );
@@ -531,11 +532,7 @@ function GCSUtil(connectionConfig, httpClient, fileStream) {
     return link;
   };
 
-  //getProxyString and getIsEnvProxyOverridden are for testing purpose.
-  this.getProxyString = function () {
-    return proxyString;
-  };
-
+  //getIsEnvProxyOverridden are for testing purpose
   this.getIsEnvProxyOverridden = function () {
     return isEnvProxyOverridden;
   };

diff --git a/lib/proxy_util.js b/lib/proxy_util.js
@@ -307,6 +307,9 @@ exports.describeProxy = function (proxy) {
  * @returns {string} 
  */
 exports.stringifyProxy = function (proxy) {
+  if (Util.isEmptyObject(proxy)) {
+    return null;
+  }
   return `${(proxy.protocol).startsWith('https') ? 'https' : 'http'}://` + 
   `${Util.exists(proxy.user) ? `${proxy.user}:${proxy.password}@` : ''}` +
   `${proxy.host}:${proxy.port}`;

diff --git a/test/unit/file_transfer_agent/gcs_test.js b/test/unit/file_transfer_agent/gcs_test.js
@@ -383,14 +383,12 @@ describe('GCS client', function () {
         connectionConfig: connectionConfig,
         HTTPS_PROXY: null,
         isOverwriteEnvProxy: false,
-        proxyString: null,
       },
       {
         name: 'when HTTPS_PROXY only exists',
         connectionConfig: connectionConfig,
         HTTPS_PROXY: 'https://abc:[email protected]:2345',
         isOverwriteEnvProxy: false,
-        proxyString: null,
       },
       {
         name: 'when the connectionProxy is different from the Env Proxy(HTTPS_PROXY)',
@@ -406,7 +404,6 @@ describe('GCS client', function () {
         },
         HTTPS_PROXY: 'https://abc:[email protected]:2345',
         isOverwriteEnvProxy: true,
-        proxyString: 'https://user:[email protected]:1234',
       },
       {
         name: 'when the connectionProxy and HTTPS_PROXY is the same.',
@@ -422,7 +419,6 @@ describe('GCS client', function () {
         },
         HTTPS_PROXY: 'https://user:[email protected]:1234',
         isOverwriteEnvProxy: false,
-        proxyString: null,
       },
       {
         name: 'when the connectionProxy and HTTPS_PROXY are different, but overrideEnvProxy is false.',
@@ -441,7 +437,6 @@ describe('GCS client', function () {
         },
         HTTPS_PROXY: 'https://abc:[email protected]:2345',
         isOverwriteEnvProxy: false,
-        proxyString: null,
       },
       {
         name: 'when no HTTPS_PROXY, but the connection Proxy exists.',
@@ -457,7 +452,6 @@ describe('GCS client', function () {
         },
         HTTPS_PROXY: null,
         isOverwriteEnvProxy: true,
-        proxyString: 'https://user:[email protected]:1234',
       },
       {
         name: 'when the connectionProxy exists, but the noProxy is set with google storage destination.',
@@ -473,17 +467,15 @@ describe('GCS client', function () {
         },
         HTTPS_PROXY: null,
         isOverwriteEnvProxy: false,
-        proxyString: null,
       }, 
     ];
 
-    testCases.forEach(({ name, connectionConfig, HTTPS_PROXY, isOverwriteEnvProxy, proxyString }) => {
+    testCases.forEach(({ name, connectionConfig, HTTPS_PROXY, isOverwriteEnvProxy }) => {
       it(name, () => {
         HTTPS_PROXY !== null ? process.env.HTTPS_PROXY = HTTPS_PROXY : delete process.env.HTTPS_PROXY; 
         const GCS = new GCSUtil(connectionConfig);
         GCS.createClient(meta.stageInfo);
         assert.strictEqual(isOverwriteEnvProxy, GCS.getIsEnvProxyOverridden());
-        assert.strictEqual(proxyString, GCS.getProxyString());
       });
     });
   });