resolved review comments #1

microsoft · Jan 4, 2018 · 65f79b8 · 65f79b8
1 parent d0f0604
commit 65f79b8
Show file tree

Hide file tree

Showing 4 changed files with 61 additions and 62 deletions.
diff --git a/Tasks/Common/azure-arm-rest/AzureServiceClient.ts b/Tasks/Common/azure-arm-rest/AzureServiceClient.ts
@@ -204,13 +204,7 @@ export class ServiceClient {
         }
     }
 
-    private sleepFor(sleepDurationInSeconds): Promise<any> {
-        return new Promise((resolve, reeject) => {
-            setTimeout(resolve, sleepDurationInSeconds * 1000);
-        });
-    }
-
-    public getFormattedError(error: any) {
+    public getFormattedError(error: any): string {
         if(error && error.message) {
             if(error.statusCode) {
                 var errorMessage = typeof error.message.valueOf() == 'string' ? error.message 
@@ -223,4 +217,10 @@ export class ServiceClient {
 
         return error;
     }
+
+    private sleepFor(sleepDurationInSeconds): Promise<any> {
+        return new Promise((resolve, reeject) => {
+            setTimeout(resolve, sleepDurationInSeconds * 1000);
+        });
+    }
 }
diff --git a/Tasks/Common/azure-arm-rest/azure-arm-appinsights.ts b/Tasks/Common/azure-arm-rest/azure-arm-appinsights.ts
@@ -46,7 +46,7 @@ export class AzureApplicationInsights {
         }
     }
 
-    public async update(insightProperties: any) {
+    public async update(insightProperties: any): Promise<ApplicationInsights> {
         var httpRequest = new webClient.WebRequest();
         httpRequest.method = 'PUT';
         httpRequest.body = JSON.stringify(insightProperties);

diff --git a/Tasks/Common/azure-arm-rest/azure-arm-endpoint.ts b/Tasks/Common/azure-arm-rest/azure-arm-endpoint.ts
@@ -16,8 +16,7 @@ export class AzureRMEndpoint {
         this.endpoint = null;
     }
 
-    public async getEndpoint() {
-        let dataDeferred = Q.defer<AzureEndpoint>();
+    public async getEndpoint(): Promise<AzureEndpoint> {
         if(!!this.endpoint) {
             return this.endpoint;
         }
@@ -35,7 +34,7 @@ export class AzureRMEndpoint {
             } as AzureEndpoint;
 
             if(this.endpoint.environment != null && this.endpoint.environment.toLowerCase() == this._environments.AzureStack && ( !this.endpoint.environmentAuthorityUrl || !this.endpoint.activeDirectoryResourceID)) {
-                return await this._getAzureStackData(this.endpoint);
+                this.endpoint = await this._updateAzureStackData(this.endpoint);
             }
             else {
                 this.endpoint.environmentAuthorityUrl = (!!this.endpoint.environmentAuthorityUrl) ? this.endpoint.environmentAuthorityUrl : "https://login.windows.net/";
@@ -46,7 +45,7 @@ export class AzureRMEndpoint {
         return this.endpoint;
     }
 
-    private _getAzureStackData(endpoint: AzureEndpoint) {
+    private async _updateAzureStackData(endpoint: AzureEndpoint): Promise<AzureEndpoint> {
         let dataDeferred = Q.defer<AzureEndpoint>();
         let webRequest = new webClient.WebRequest();
         webRequest.uri = `${endpoint.url}metadata/endpoints?api-version=2015-01-01`;
@@ -55,55 +54,55 @@ export class AzureRMEndpoint {
             'Content-Type': 'application/json'
         }
 
-        webClient.sendRequest(webRequest).then((response: webClient.WebResponse) => {
-            if(response.statusCode == 200) {
-                let result = response.body;
-                endpoint.graphEndpoint = result.graphEndpoint;
-                endpoint.galleryUrl = result.galleryUrl;
-                endpoint.portalEndpoint = result.portalEndpoint;
-                var authenticationData = result.authentication;
-                if(!!authenticationData) {
-                    var loginEndpoint = authenticationData.loginEndpoint;
-                    if(!!loginEndpoint) {
-                        loginEndpoint += (loginEndpoint[loginEndpoint.length - 1] == "/") ? "" : "/";
-                        endpoint.activeDirectoryAuthority = loginEndpoint;
-                        endpoint.environmentAuthorityUrl = loginEndpoint;
-                    }
-                    else {
-                        // change to login endpoint
-                        dataDeferred.reject(tl.loc('UnableToFetchAuthorityURL'));
-                    }
+        let azureStackResult;
+        try {
+            let response: webClient.WebResponse = await webClient.sendRequest(webRequest);
+            if(response.statusCode != 200) {
+                tl.debug("Action: _updateAzureStackData, Response: " + JSON.stringify(response));
+                throw new Error(response.statusCode + ' ' + response.statusMessage)
+            }
 
-                    var audiences = authenticationData.audiences;
-                    if(audiences && audiences.length > 0) {
-                        endpoint.activeDirectoryResourceID = audiences[0];
-                    }
+            azureStackResult = response.body;
+        }
+        catch(error) {
+            throw new Error(tl.loc("FailedToFetchAzureStackDependencyData", error.toString()));
+        }
 
-                    try {
-                        var endpointUrl =  endpoint.url;
-                        endpointUrl += (endpointUrl[endpointUrl.length-1] == "/") ? "" : "/";
-                        var index = endpointUrl.indexOf('.');
-                        var domain = endpointUrl.substring(index+1);
-                        domain = (domain.lastIndexOf("/") == domain.length-1) ? domain.substring(0, domain.length-1): domain;
-                        endpoint.AzureKeyVaultDnsSuffix = ("vault" + domain).toLowerCase();
-                        endpoint.AzureKeyVaultServiceEndpointResourceId = ("https://vault." + domain).toLowerCase();
-                    }
-                    catch(error) {
-                        dataDeferred.reject(tl.loc("SpecifiedAzureRmEndpointIsInvalid", endpointUrl));
-                    }
+        endpoint.graphEndpoint = azureStackResult.graphEndpoint;
+        endpoint.galleryUrl = azureStackResult.galleryUrl;
+        endpoint.portalEndpoint = azureStackResult.portalEndpoint;
+        var authenticationData = azureStackResult.authentication;
+        if(!!authenticationData) {
+            var loginEndpoint = authenticationData.loginEndpoint;
+            if(!!loginEndpoint) {
+                loginEndpoint += (loginEndpoint[loginEndpoint.length - 1] == "/") ? "" : "/";
+                endpoint.activeDirectoryAuthority = loginEndpoint;
+                endpoint.environmentAuthorityUrl = loginEndpoint;
+            }
+            else {
+                // change to login endpoint
+                throw new Error(tl.loc('UnableToFetchAuthorityURL'));
+            }
 
-                    dataDeferred.resolve(endpoint);
-                }
+            var audiences = authenticationData.audiences;
+            if(audiences && audiences.length > 0) {
+                endpoint.activeDirectoryResourceID = audiences[0];
+            }
 
+            try {
+                var endpointUrl =  endpoint.url;
+                endpointUrl += (endpointUrl[endpointUrl.length-1] == "/") ? "" : "/";
+                var index = endpointUrl.indexOf('.');
+                var domain = endpointUrl.substring(index+1);
+                domain = (domain.lastIndexOf("/") == domain.length-1) ? domain.substring(0, domain.length-1): domain;
+                endpoint.AzureKeyVaultDnsSuffix = ("vault" + domain).toLowerCase();
+                endpoint.AzureKeyVaultServiceEndpointResourceId = ("https://vault." + domain).toLowerCase();
             }
-            else {
-                tl.debug("Action: initializeAzureStackData, Response: " + JSON.stringify(response));
-                dataDeferred.reject(tl.loc("FailedToFetchAzureStackDependencyData", response.statusCode));
+            catch(error) {
+                throw new Error(tl.loc("SpecifiedAzureRmEndpointIsInvalid", endpointUrl));
             }
-        }, (error) => {
-            dataDeferred.reject(error);
-        });
-
-        return dataDeferred.promise;
+        }
+
+        return endpoint;
     }
 }
diff --git a/Tasks/Common/azure-arm-rest/webClient.ts b/Tasks/Common/azure-arm-rest/webClient.ts
@@ -70,6 +70,12 @@ export async function sendRequest(request: WebRequest, options?: WebRequestOptio
     }
 }
 
+export function sleepFor(sleepDurationInSeconds): Promise<any> {
+    return new Promise((resolve, reject) => {
+        setTimeout(resolve, sleepDurationInSeconds * 1000);
+    });
+}
+
 async function sendRequestInternal(request: WebRequest): Promise<WebResponse> {
     tl.debug(util.format("[%s]%s", request.method, request.uri));
     var response: httpClient.HttpClientResponse = await httpCallbackClient.request(request.method, request.uri, request.body, request.headers);
@@ -95,9 +101,3 @@ async function toWebResponse(response: httpClient.HttpClientResponse): Promise<W
 
     return res;
 }
-
-export function sleepFor(sleepDurationInSeconds): Promise<any> {
-    return new Promise((resolve, reject) => {
-        setTimeout(resolve, sleepDurationInSeconds * 1000);
-    });
-}