addressing review comments

Author: ssreerama

extensions/mssql/src/http/httpClientCore.ts

@@ -12,6 +12,7 @@ import { Readable } from "stream";
 import { ILogger } from "../models/interfaces";
 
 const UnableToGetProxyAgentOptionsMessage = "Unable to read proxy agent options to get tenants.";
+const DefaultRequestTimeoutMs = 20000; // 20 seconds
 
 export interface IHttpClientMessages {
     missingProtocolWarning(proxy: string): string;
@@ -25,6 +26,7 @@ export interface IHttpClientDependencies {
     parseUriScheme?: (value: string) => string | undefined;
     showWarningMessage?: (message: string) => void;
     getErrorMessage?: (error: unknown) => string;
+    getRequestTimeout?: () => number;
     messages?: IHttpClientMessages;
 }
 
@@ -168,12 +170,12 @@ export class HttpClientCore {
                 : new URL(proxy).protocol;
 
             if (!scheme) {
-                message = `Proxy settings found, but without a protocol (e.g. http://): '${proxy}'.  You may encounter connection issues while using the MSSQL extension.`;
+                message = `Proxy settings found, but without a protocol (e.g. http://): '${proxy}'.  You may encounter connection issues while using this extension.`;
                 localizedMessage = this.dependencies.messages?.missingProtocolWarning(proxy);
             }
         } catch (err) {
             const errorMessage = this.getErrorMessage(err);
-            message = `Proxy settings found, but encountered an error while parsing the URL: '${proxy}'.  You may encounter connection issues while using the MSSQL extension.  Error: ${errorMessage}`;
+            message = `Proxy settings found, but encountered an error while parsing the URL: '${proxy}'.  You may encounter connection issues while using this extension.  Error: ${errorMessage}`;
             localizedMessage = this.dependencies.messages?.unparseableWarning(proxy, errorMessage);
         }
 
@@ -225,6 +227,7 @@ export class HttpClientCore {
         const config: AxiosRequestConfig = {
             headers,
             validateStatus: () => true, // Never throw
+            timeout: this.dependencies.getRequestTimeout?.() ?? DefaultRequestTimeoutMs,
         };
 
         const proxy = this.loadProxyConfig();
@@ -410,7 +413,11 @@ export class HttpClientCore {
 
         return {
             host: proxyEndpoint.hostname,
-            port: Number(proxyEndpoint.port),
+            port: proxyEndpoint.port
+                ? Number(proxyEndpoint.port)
+                : proxyEndpoint.protocol === "https:"
+                  ? 443
+                  : 80,
             auth,
             rejectUnauthorized: typeof strictSSL === "boolean",
         };

extensions/sql-database-projects/src/controllers/mainController.ts

@@ -21,6 +21,7 @@ import * as constants from "../common/constants";
 import { SqlDatabaseProjectProvider } from "../projectProvider/projectProvider";
 import { GenerateProjectFromOpenApiSpecOptions, ItemType } from "sqldbproj";
 import { FileNode } from "../models/tree/fileFolderTreeItem";
+import { HttpClient } from "../http/httpClient";
 
 /**
  * The main controller class that initializes the extension
@@ -61,6 +62,9 @@ export default class MainController implements vscode.Disposable {
                 .update(DotnetInstallLocationKey, oldNetCoreInstallSetting, true);
         }
 
+        // Warn about invalid proxy settings early during activation
+        new HttpClient().warnOnInvalidProxySettings();
+
         await this.initializeDatabaseProjects();
         return new SqlDatabaseProjectProvider(this.projectsController);
     }

extensions/sql-database-projects/src/http/httpClient.ts

@@ -8,12 +8,13 @@ import * as fs from "fs";
 import {
     HttpClientCore,
     IHttpClientDependencies,
-    ILogger,
     HttpDownloadError,
     IDownloadFileOptions,
     IDownloadFileResult,
 } from "./httpClientCore";
+import { ILogger } from "../common/logger";
 import * as constants from "../common/constants";
+import { getErrorMessage } from "../common/utils";
 
 export class HttpClient extends HttpClientCore {
     constructor(logger?: ILogger) {
@@ -27,6 +28,7 @@ export class HttpClient extends HttpClientCore {
             showWarningMessage: (message: string) => {
                 void vscode.window.showWarningMessage(message);
             },
+            getErrorMessage,
             // Provide a messages implementation so that HttpClientCore can surface
             // proxy warnings via showWarningMessage. Without this, warnOnInvalidProxySettings
             // would silently skip the showWarningMessage call even though it is wired up above.
@@ -82,8 +84,7 @@ export class HttpClient extends HttpClientCore {
         try {
             result = await this.downloadFile(downloadUrl, fd, options);
         } catch (e) {
-            const message = e instanceof Error ? e.message : String(e);
-            outputChannel?.appendLine(`${constants.downloadError}: ${message}`);
+            outputChannel?.appendLine(`${constants.downloadError}: ${getErrorMessage(e)}`);
             throw e;
         } finally {
             fs.closeSync(fd);

extensions/sql-database-projects/src/http/httpClientCore.ts

@@ -14,9 +14,8 @@ import axios, { AxiosRequestConfig, AxiosResponse, RawAxiosResponseHeaders } fro
 import { Readable } from "stream";
 import { ILogger } from "../common/logger";
 
-export type { ILogger };
-
-const UnableToGetProxyAgentOptionsMessage = "Unable to read proxy agent options to get tenants.";
+const UnableToGetProxyAgentOptionsMessage = "Unable to read proxy agent options.";
+const DefaultRequestTimeoutMs = 20000; // 20 seconds
 
 export interface IHttpClientMessages {
     missingProtocolWarning(proxy: string): string;
@@ -30,6 +29,7 @@ export interface IHttpClientDependencies {
     parseUriScheme?: (value: string) => string | undefined;
     showWarningMessage?: (message: string) => void;
     getErrorMessage?: (error: unknown) => string;
+    getRequestTimeout?: () => number;
     messages?: IHttpClientMessages;
 }
 
@@ -179,12 +179,12 @@ export class HttpClientCore {
                 : new URL(proxy).protocol;
 
             if (!scheme) {
-                message = `Proxy settings found, but without a protocol (e.g. http://): '${proxy}'.  You may encounter connection issues while using the MSSQL extension.`;
+                message = `Proxy settings found, but without a protocol (e.g. http://): '${proxy}'.  You may encounter connection issues while using this extension.`;
                 localizedMessage = this.dependencies.messages?.missingProtocolWarning(proxy);
             }
         } catch (err) {
             const errorMessage = this.getErrorMessage(err);
-            message = `Proxy settings found, but encountered an error while parsing the URL: '${proxy}'.  You may encounter connection issues while using the MSSQL extension.  Error: ${errorMessage}`;
+            message = `Proxy settings found, but encountered an error while parsing the URL: '${proxy}'.  You may encounter connection issues while using this extension.  Error: ${errorMessage}`;
             localizedMessage = this.dependencies.messages?.unparseableWarning(proxy, errorMessage);
         }
 
@@ -236,6 +236,7 @@ export class HttpClientCore {
         const config: AxiosRequestConfig = {
             headers,
             validateStatus: () => true, // Never throw
+            timeout: this.dependencies.getRequestTimeout?.() ?? DefaultRequestTimeoutMs,
         };
 
         const proxy = this.loadProxyConfig();
@@ -427,7 +428,11 @@ export class HttpClientCore {
 
         return {
             host: proxyEndpoint.hostname,
-            port: Number(proxyEndpoint.port),
+            port: proxyEndpoint.port
+                ? Number(proxyEndpoint.port)
+                : proxyEndpoint.protocol === "https:"
+                  ? 443
+                  : 80,
             auth,
             // Default to rejecting unauthorized certs unless the user explicitly disables strict SSL.
             rejectUnauthorized: strictSSL !== false,