Merge pull request Azure#141 from Azure/daschult/deserializeResponses

Move error deserialization into serializationPolicy
bterlson · Jun 21, 2018 · 2bea9e2 · 2bea9e2
2 parents fe21b60 + d47e2a1
commit 2bea9e2
Show file tree

Hide file tree

Showing 7 changed files with 180 additions and 55 deletions.
diff --git a/lib/operationResponse.ts b/lib/operationResponse.ts
@@ -0,0 +1,19 @@
+import { Mapper } from "./serializer";
+
+// Copyright (c) Microsoft Corporation. All rights reserved.
+// Licensed under the MIT License. See License.txt in the project root for license information.
+
+/**
+ * An OperationResponse that can be returned from an operation request for a single status code.
+ */
+export interface OperationResponse {
+  /**
+   * The mapper that will be used to deserialize the response headers.
+   */
+  headersMapper?: Mapper;
+
+  /**
+   * The mapper that will be used to deserialize the response body.
+   */
+  bodyMapper?: Mapper;
+}
diff --git a/lib/operationSpec.ts b/lib/operationSpec.ts
@@ -4,6 +4,7 @@
 import { OperationParameter, OperationQueryParameter, OperationURLParameter } from "./operationParameter";
 import { Serializer } from "./serializer";
 import { HttpMethods } from "./webResource";
+import { OperationResponse } from "./operationResponse";
 
 /**
  * A specification that defines an operation.
@@ -68,4 +69,10 @@ export interface OperationSpec {
    * operation's HTTP request.
    */
   formDataParameters?: OperationParameter[];
+
+  /**
+   * The different types of responses that this operation can return based on what status code is
+   * returned.
+   */
+  responses: { [responseCode: string]: OperationResponse };
 }
diff --git a/lib/policies/exponentialRetryPolicy.ts b/lib/policies/exponentialRetryPolicy.ts
@@ -111,29 +111,33 @@ export class ExponentialRetryPolicy extends BaseRequestPolicy {
     return retryData;
   }
 
-  async retry(request: WebResource, response: HttpOperationResponse, retryData?: RetryData, err?: RetryError): Promise<HttpOperationResponse> {
-    const self = this;
-    retryData = self.updateRetryData(retryData, err);
-    if (self.shouldRetry(response.status, retryData)) {
+  async retry(request: WebResource, response: HttpOperationResponse, retryData?: RetryData, requestError?: RetryError): Promise<HttpOperationResponse> {
+    retryData = this.updateRetryData(retryData, requestError);
+    const isAborted: boolean | undefined = request.abortSignal && request.abortSignal.aborted;
+    if (!isAborted && this.shouldRetry(response.status, retryData)) {
       try {
         await utils.delay(retryData.retryInterval);
-        const res: HttpOperationResponse = await this._nextPolicy.sendRequest(request.clone());
-        return self.retry(request, res, retryData, err);
+        response = await this._nextPolicy.sendRequest(request.clone());
+        requestError = undefined;
       } catch (err) {
-        return self.retry(request, response, retryData, err);
+        requestError = err;
       }
+      return this.retry(request, response, retryData, requestError);
+    } else if (isAborted || !utils.objectIsNull(requestError)) {
+      // If the operation failed in the end, return all errors instead of just the last one
+      requestError = retryData.error;
+      return Promise.reject(requestError);
     } else {
-      if (!utils.objectIsNull(err)) {
-        // If the operation failed in the end, return all errors instead of just the last one
-        err = retryData.error;
-        return Promise.reject(err);
-      }
       return Promise.resolve(response);
     }
   }
 
   public async sendRequest(request: WebResource): Promise<HttpOperationResponse> {
-    const response: HttpOperationResponse = await this._nextPolicy.sendRequest(request.clone());
-    return this.retry(request, response);
+    try {
+      const response: HttpOperationResponse = await this._nextPolicy.sendRequest(request.clone());
+      return this.retry(request, response);
+    } catch (error) {
+      return this.retry(request, error.response, undefined, error);
+    }
   }
 }
diff --git a/lib/policies/redirectPolicy.ts b/lib/policies/redirectPolicy.ts
@@ -49,7 +49,11 @@ export class RedirectPolicy extends BaseRequestPolicy {
   }
 
   public async sendRequest(request: WebResource): Promise<HttpOperationResponse> {
-    const response: HttpOperationResponse = await this._nextPolicy.sendRequest(request);
-    return this.handleRedirect(response, 0);
+    try {
+      const response: HttpOperationResponse = await this._nextPolicy.sendRequest(request);
+      return this.handleRedirect(response, 0);
+    } catch (error) {
+      return Promise.reject(error);
+    }
   }
 }
diff --git a/lib/policies/serializationPolicy.ts b/lib/policies/serializationPolicy.ts
@@ -3,7 +3,9 @@
 
 import { HttpOperationResponse } from "../httpOperationResponse";
 import { getPathStringFromParameter } from "../operationParameter";
+import { OperationResponse } from "../operationResponse";
 import { OperationSpec } from "../operationSpec";
+import { RestError } from "../restError";
 import { Mapper, MapperType } from "../serializer";
 import * as utils from "../util/utils";
 import { WebResource } from "../webResource";
@@ -27,11 +29,12 @@ export class SerializationPolicy extends BaseRequestPolicy {
     super(nextPolicy, options);
   }
 
-  public sendRequest(request: WebResource): Promise<HttpOperationResponse> {
+  public async sendRequest(request: WebResource): Promise<HttpOperationResponse> {
     let result: Promise<HttpOperationResponse>;
     try {
       this.serializeRequestBody(request);
-      result = this._nextPolicy.sendRequest(request);
+      const operationResponse: HttpOperationResponse = await this._nextPolicy.sendRequest(request);
+      result = this.deserializeResponseBody(operationResponse);
     } catch (error) {
       result = Promise.reject(error);
     }
@@ -69,4 +72,88 @@ export class SerializationPolicy extends BaseRequestPolicy {
       }
     }
   }
+
+  public deserializeResponseBody(response: HttpOperationResponse): Promise<HttpOperationResponse> {
+    const operationSpec: OperationSpec | undefined = response.request.operationSpec;
+    if (operationSpec && operationSpec.responses) {
+      const statusCode: number = response.status;
+
+      const expectedStatusCodes: string[] = Object.keys(operationSpec.responses);
+
+      const hasNoExpectedStatusCodes: boolean = (expectedStatusCodes.length === 0 || (expectedStatusCodes.length === 1 && expectedStatusCodes[0] === "default"));
+
+      const responseSpec: OperationResponse = operationSpec.responses[statusCode];
+
+      const isExpectedStatusCode: boolean = hasNoExpectedStatusCodes ? (200 <= statusCode && statusCode < 300) : !!responseSpec;
+      if (!isExpectedStatusCode) {
+        const defaultResponseSpec: OperationResponse = operationSpec.responses.default;
+        if (defaultResponseSpec) {
+          const initialErrorMessage: string = isStreamOperation(operationSpec.responses)
+            ? `Unexpected status code: ${statusCode}`
+            : response.bodyAsText as string;
+
+          const error = new RestError(initialErrorMessage);
+          error.statusCode = statusCode;
+          error.request = utils.stripRequest(response.request);
+          error.response = utils.stripResponse(response);
+
+          let parsedErrorResponse: { [key: string]: any } = response.parsedBody;
+          try {
+            if (parsedErrorResponse) {
+              const defaultResponseBodyMapper: Mapper | undefined = defaultResponseSpec.bodyMapper;
+              if (defaultResponseBodyMapper && defaultResponseBodyMapper.serializedName === "CloudError") {
+                if (parsedErrorResponse.error) {
+                  parsedErrorResponse = parsedErrorResponse.error;
+                }
+                if (parsedErrorResponse.code) {
+                  error.code = parsedErrorResponse.code;
+                }
+                if (parsedErrorResponse.message) {
+                  error.message = parsedErrorResponse.message;
+                }
+              } else {
+                let internalError: any = parsedErrorResponse;
+                if (parsedErrorResponse.error) {
+                  internalError = parsedErrorResponse.error;
+                }
+
+                error.code = internalError.code;
+                if (internalError.message) {
+                  error.message = internalError.message;
+                }
+              }
+
+              if (defaultResponseBodyMapper) {
+                let valueToDeserialize: any = parsedErrorResponse;
+                if (operationSpec.isXML && defaultResponseBodyMapper.type.name === MapperType.Sequence) {
+                  valueToDeserialize = typeof parsedErrorResponse === "object"
+                    ? parsedErrorResponse[defaultResponseBodyMapper.xmlElementName!]
+                    : [];
+                }
+                error.body = operationSpec.serializer.deserialize(defaultResponseBodyMapper, valueToDeserialize, "error.body");
+              }
+            }
+          } catch (defaultError) {
+            error.message = `Error \"${defaultError.message}\" occurred in deserializing the responseBody - \"${response.bodyAsText}\" for the default response.`;
+          }
+          return Promise.reject(error);
+        }
+      } else {
+
+      }
+    }
+    return Promise.resolve(response);
+  }
 }
+
+function isStreamOperation(responseSpecs: { [statusCode: string]: OperationResponse }): boolean {
+  let result = false;
+  for (const statusCode in responseSpecs) {
+    const operationResponse: OperationResponse = responseSpecs[statusCode];
+    if (operationResponse.bodyMapper && operationResponse.bodyMapper.type.name === MapperType.Stream) {
+      result = true;
+      break;
+    }
+  }
+  return result;
+}
diff --git a/package.json b/package.json
@@ -25,7 +25,7 @@
     "autorest",
     "clientruntime"
   ],
-  "main": "./dist/lib/msRest.js",
+  "main": "./lib/msRest.ts",
   "types": "./typings/lib/msRest.d.ts",
   "browser": {
     "./dist/lib/msRest.js": "./es/lib/msRest.js",

diff --git a/test/shared/serviceClientTests.ts b/test/shared/serviceClientTests.ts
@@ -1,8 +1,8 @@
 import { ServiceClient, WebResource, Serializer, HttpOperationResponse, DictionaryMapper } from "../../lib/msRest";
 import * as assert from "assert";
 
-describe("ServiceClient", function() {
-  it("should serialize headerCollectionPrefix", async function() {
+describe("ServiceClient", function () {
+  it("should serialize headerCollectionPrefix", async function () {
     const expected = {
       "foo-bar-alpha": "hello",
       "foo-bar-beta": "world",
@@ -21,45 +21,49 @@ describe("ServiceClient", function() {
       requestPolicyCreators: []
     });
 
-    await client.sendOperationRequest(new WebResource(), {
-      arguments: {
-        metadata: {
-          "alpha": "hello",
-          "beta": "world"
-        },
-        unrelated: 42
-      }
-    }, {
-      httpMethod: "GET",
-      baseUrl: "httpbin.org",
-      serializer: new Serializer(),
-      headerParameters: [{
-        parameterPath: "metadata",
-        mapper: {
-          serializedName: "metadata",
-          type: {
-            name: "Dictionary",
-            value: {
-              type: {
-                name: "String"
+    await client.sendOperationRequest(
+      new WebResource(), {
+        arguments: {
+          metadata: {
+            "alpha": "hello",
+            "beta": "world"
+          },
+          unrelated: 42
+        }
+      },
+      {
+        httpMethod: "GET",
+        baseUrl: "httpbin.org",
+        serializer: new Serializer(),
+        headerParameters: [{
+          parameterPath: "metadata",
+          mapper: {
+            serializedName: "metadata",
+            type: {
+              name: "Dictionary",
+              value: {
+                type: {
+                  name: "String"
+                }
               }
+            },
+            headerCollectionPrefix: "foo-bar-"
+          } as DictionaryMapper
+        }, {
+          parameterPath: "unrelated",
+          mapper: {
+            serializedName: "unrelated",
+            type: {
+              name: "Number"
             }
-          },
-          headerCollectionPrefix: "foo-bar-"
-        } as DictionaryMapper
-      }, {
-        parameterPath: "unrelated",
-        mapper: {
-          serializedName: "unrelated",
-          type: {
-            name: "Number"
           }
+        }],
+        responses: {
+          200: {}
         }
-      }]
-    });
-
+      });
 
     assert(request!);
     assert.deepStrictEqual(request!.headers.toJson(), expected);
   });
-})
+});