[core-http] Allow defining multiple error responses for Operations (#…

…11841) * Added support for isError in operationresponse * Release Updates * Add Missing condition * Extract Code into a seperate function * Port changes to core-client * Update sdk/core/core-http/src/policies/deserializationPolicy.ts Co-authored-by: Jeff Fisher <[email protected]> * Modifications based on PR Comments * Add one missing condition * Formatting changes * Code Reafctors * Format Co-authored-by: Jeff Fisher <[email protected]>
Azure · Oct 19, 2020 · c20d0b7 · c20d0b7
1 parent 92affe2
commit c20d0b7
Show file tree

Hide file tree

Showing 11 changed files with 500 additions and 142 deletions.
diff --git a/sdk/core/core-client/review/core-client.api.md b/sdk/core/core-client/review/core-client.api.md
@@ -230,6 +230,7 @@ export interface OperationResponse {
 export interface OperationResponseMap {
     bodyMapper?: Mapper;
     headersMapper?: Mapper;
+    isError?: boolean;
 }
 
 // @public

diff --git a/sdk/core/core-client/src/deserializationPolicy.ts b/sdk/core/core-client/src/deserializationPolicy.ts
@@ -8,7 +8,12 @@ import {
   PipelinePolicy,
   RestError
 } from "@azure/core-https";
-import { OperationRequest, OperationResponseMap, FullOperationResponse } from "./interfaces";
+import {
+  OperationRequest,
+  OperationResponseMap,
+  FullOperationResponse,
+  OperationSpec
+} from "./interfaces";
 import { MapperTypeNames } from "./serializer";
 import { isStreamOperation } from "./interfaceHelpers";
 
@@ -118,79 +123,15 @@ async function deserializeResponseBody(
   }
 
   const responseSpec = getOperationResponseMap(parsedResponse);
-  const expectedStatusCodes = Object.keys(operationSpec.responses);
-  const hasNoExpectedStatusCodes =
-    expectedStatusCodes.length === 0 ||
-    (expectedStatusCodes.length === 1 && expectedStatusCodes[0] === "default");
-  const isExpectedStatusCode: boolean = hasNoExpectedStatusCodes
-    ? 200 <= parsedResponse.status && parsedResponse.status < 300
-    : !!responseSpec;
-
-  // There is no operation response spec for current status code.
-  // So, treat it as an error case and use the default response spec to deserialize the response.
-  if (!isExpectedStatusCode) {
-    const defaultResponseSpec = operationSpec.responses.default;
-    if (!defaultResponseSpec) {
-      return parsedResponse;
-    }
-
-    const defaultBodyMapper = defaultResponseSpec.bodyMapper;
-    const defaultHeadersMapper = defaultResponseSpec.headersMapper;
-
-    const initialErrorMessage = isStreamOperation(operationSpec)
-      ? `Unexpected status code: ${parsedResponse.status}`
-      : (parsedResponse.bodyAsText as string);
-
-    const error = new RestError(initialErrorMessage, {
-      statusCode: parsedResponse.status,
-      request: parsedResponse.request,
-      response: parsedResponse
-    });
-
-    try {
-      // If error response has a body, try to extract error code & message from it
-      // Then try to deserialize it using default body mapper
-      if (parsedResponse.parsedBody) {
-        const parsedBody = parsedResponse.parsedBody;
-        const internalError: any = parsedBody.error || parsedBody;
-        error.code = internalError.code;
-        if (internalError.message) {
-          error.message = internalError.message;
-        }
-
-        if (defaultBodyMapper) {
-          let valueToDeserialize: any = parsedBody;
-          if (operationSpec.isXML && defaultBodyMapper.type.name === MapperTypeNames.Sequence) {
-            valueToDeserialize = [];
-            const elementName = defaultBodyMapper.xmlElementName;
-            if (typeof parsedBody === "object" && elementName) {
-              valueToDeserialize = parsedBody[elementName];
-            }
-          }
-          if (error.response) {
-            const errorResponse: FullOperationResponse = error.response;
-            errorResponse.parsedBody = operationSpec.serializer.deserialize(
-              defaultBodyMapper,
-              valueToDeserialize,
-              "error.response.parsedBody"
-            );
-          }
-        }
-      }
-
-      // If error response has headers, try to deserialize it using default header mapper
-      if (parsedResponse.headers && defaultHeadersMapper && error.response) {
-        const errorResponse: FullOperationResponse = error.response;
-        errorResponse.parsedHeaders = operationSpec.serializer.deserialize(
-          defaultHeadersMapper,
-          parsedResponse.headers.toJSON(),
-          "operationRes.parsedHeaders"
-        );
-      }
-    } catch (defaultError) {
-      error.message = `Error "${defaultError.message}" occurred in deserializing the responseBody - "${parsedResponse.bodyAsText}" for the default response.`;
-    }
+  const { error, shouldReturnResponse } = handleErrorResponse(
+    parsedResponse,
+    operationSpec,
+    responseSpec
+  );
+  if (error) {
     throw error;
+  } else if (shouldReturnResponse) {
+    return parsedResponse;
   }
 
   // An operation response spec does exist for current status code, so
@@ -238,6 +179,100 @@ async function deserializeResponseBody(
   return parsedResponse;
 }
 
+function isOperationSpecEmpty(operationSpec: OperationSpec): boolean {
+  const expectedStatusCodes = Object.keys(operationSpec.responses);
+  return (
+    expectedStatusCodes.length === 0 ||
+    (expectedStatusCodes.length === 1 && expectedStatusCodes[0] === "default")
+  );
+}
+
+function handleErrorResponse(
+  parsedResponse: FullOperationResponse,
+  operationSpec: OperationSpec,
+  responseSpec: OperationResponseMap | undefined
+): { error: RestError | null; shouldReturnResponse: boolean } {
+  const isSuccessByStatus = 200 <= parsedResponse.status && parsedResponse.status < 300;
+  const isExpectedStatusCode: boolean = isOperationSpecEmpty(operationSpec)
+    ? isSuccessByStatus
+    : !!responseSpec;
+
+  if (isExpectedStatusCode) {
+    if (responseSpec) {
+      if (!responseSpec.isError) {
+        return { error: null, shouldReturnResponse: false };
+      }
+    } else {
+      return { error: null, shouldReturnResponse: false };
+    }
+  }
+
+  const errorResponseSpec = responseSpec ?? operationSpec.responses.default;
+
+  if (!errorResponseSpec) {
+    return { error: null, shouldReturnResponse: true };
+  }
+
+  const defaultBodyMapper = errorResponseSpec.bodyMapper;
+  const defaultHeadersMapper = errorResponseSpec.headersMapper;
+
+  const initialErrorMessage = isStreamOperation(operationSpec)
+    ? `Unexpected status code: ${parsedResponse.status}`
+    : (parsedResponse.bodyAsText as string);
+
+  const error = new RestError(initialErrorMessage, {
+    statusCode: parsedResponse.status,
+    request: parsedResponse.request,
+    response: parsedResponse
+  });
+
+  try {
+    // If error response has a body, try to extract error code & message from it
+    // Then try to deserialize it using default body mapper
+    if (parsedResponse.parsedBody) {
+      const parsedBody = parsedResponse.parsedBody;
+      const internalError: any = parsedBody.error || parsedBody;
+      error.code = internalError.code;
+      if (internalError.message) {
+        error.message = internalError.message;
+      }
+
+      if (defaultBodyMapper) {
+        let valueToDeserialize: any = parsedBody;
+        if (operationSpec.isXML && defaultBodyMapper.type.name === MapperTypeNames.Sequence) {
+          valueToDeserialize = [];
+          const elementName = defaultBodyMapper.xmlElementName;
+          if (typeof parsedBody === "object" && elementName) {
+            valueToDeserialize = parsedBody[elementName];
+          }
+        }
+        if (error.response) {
+          const errorResponse: FullOperationResponse = error.response;
+          errorResponse.parsedBody = operationSpec.serializer.deserialize(
+            defaultBodyMapper,
+            valueToDeserialize,
+            "error.response.parsedBody"
+          );
+        }
+      }
+    }
+
+    // If error response has headers, try to deserialize it using default header mapper
+    if (parsedResponse.headers && defaultHeadersMapper && error.response) {
+      const errorResponse: FullOperationResponse = error.response;
+      errorResponse.parsedHeaders = operationSpec.serializer.deserialize(
+        defaultHeadersMapper,
+        parsedResponse.headers.toJSON(),
+        "operationRes.parsedHeaders"
+      );
+    }
+  } catch (defaultError) {
+    error.message = `Error "${defaultError.message}" occurred in deserializing the responseBody - "${parsedResponse.bodyAsText}" for the default response.`;
+  }
+
+  return { error, shouldReturnResponse: false };
+}
+
 async function parse(
   jsonContentTypes: string[],
   xmlContentTypes: string[],

diff --git a/sdk/core/core-client/src/interfaces.ts b/sdk/core/core-client/src/interfaces.ts
@@ -171,6 +171,11 @@ export interface OperationResponseMap {
    * The mapper that will be used to deserialize the response body.
    */
   bodyMapper?: Mapper;
+
+  /**
+   * Indicates if this is an error response
+   */
+  isError?: boolean;
 }
 
 /**

diff --git a/sdk/core/core-client/test/deserializationPolicy.spec.ts b/sdk/core/core-client/test/deserializationPolicy.spec.ts
@@ -431,6 +431,141 @@ describe("deserializationPolicy", function() {
         assert.strictEqual(e.response.parsedBody.message, "InvalidResourceNameBody");
       }
     });
+
+    it(`with non default error response headers`, async function() {
+      const BodyMapper: CompositeMapper = {
+        serializedName: "getproperties-body",
+        type: {
+          name: "Composite",
+          className: "PropertiesBody",
+          modelProperties: {
+            message: {
+              type: {
+                name: "String"
+              }
+            }
+          }
+        }
+      };
+
+      const HeadersMapper: CompositeMapper = {
+        serializedName: "getproperties-headers",
+        type: {
+          name: "Composite",
+          className: "PropertiesHeaders",
+          modelProperties: {
+            errorCode: {
+              serializedName: "x-ms-error-code",
+              type: {
+                name: "String"
+              }
+            }
+          }
+        }
+      };
+
+      const serializer = createSerializer(HeadersMapper, true);
+
+      const operationSpec: OperationSpec = {
+        httpMethod: "GET",
+        responses: {
+          500: {
+            headersMapper: HeadersMapper,
+            bodyMapper: BodyMapper,
+            isError: true
+          }
+        },
+        serializer
+      };
+
+      try {
+        await getDeserializedResponse({
+          operationSpec,
+          headers: { "x-ms-error-code": "InvalidResourceNameHeader" },
+          bodyAsText: '{"message": "InvalidResourceNameBody"}',
+          status: 500
+        });
+        assert.fail();
+      } catch (e) {
+        assert.exists(e);
+        assert.strictEqual(e.response.parsedHeaders.errorCode, "InvalidResourceNameHeader");
+        assert.strictEqual(e.response.parsedBody.message, "InvalidResourceNameBody");
+      }
+    });
+
+    it(`with non default complex error response`, async function() {
+      const BodyMapper: CompositeMapper = {
+        serializedName: "getproperties-body",
+        type: {
+          name: "Composite",
+          className: "PropertiesBody",
+          modelProperties: {
+            message1: {
+              type: {
+                name: "String"
+              }
+            },
+            message2: {
+              type: {
+                name: "String"
+              }
+            },
+            message3: {
+              type: {
+                name: "String"
+              }
+            }
+          }
+        }
+      };
+
+      const HeadersMapper: CompositeMapper = {
+        serializedName: "getproperties-headers",
+        type: {
+          name: "Composite",
+          className: "PropertiesHeaders",
+          modelProperties: {
+            errorCode: {
+              serializedName: "x-ms-error-code",
+              type: {
+                name: "String"
+              }
+            }
+          }
+        }
+      };
+
+      const serializer = createSerializer(HeadersMapper, true);
+
+      const operationSpec: OperationSpec = {
+        httpMethod: "GET",
+        responses: {
+          503: {
+            headersMapper: HeadersMapper,
+            bodyMapper: BodyMapper,
+            isError: true
+          }
+        },
+        serializer
+      };
+
+      try {
+        await getDeserializedResponse({
+          operationSpec,
+          headers: { "x-ms-error-code": "InvalidResourceNameHeader" },
+          bodyAsText:
+            '{"message1": "InvalidResourceNameBody1", "message2": "InvalidResourceNameBody2", "message3": "InvalidResourceNameBody3"}',
+          status: 503
+        });
+        assert.fail();
+      } catch (e) {
+        assert.exists(e);
+        assert.strictEqual(e.response.parsedHeaders.errorCode, "InvalidResourceNameHeader");
+        assert.strictEqual(e.response.parsedBody.message1, "InvalidResourceNameBody1");
+        assert.strictEqual(e.response.parsedBody.message2, "InvalidResourceNameBody2");
+        assert.strictEqual(e.response.parsedBody.message3, "InvalidResourceNameBody3");
+      }
+    });
   });
 });
 

diff --git a/sdk/core/core-http/CHANGELOG.md b/sdk/core/core-http/CHANGELOG.md
@@ -1,8 +1,9 @@
 # Release History
 
-## 1.1.10 (Unreleased)
+## 1.2.0 (2020-10-19)
 
 - Explicitly set `manual` redirect handling for node fetch. And fixing redirectPipeline [PR 11863](https://github.com/Azure/azure-sdk-for-js/pull/11863)
+- Add support for multiple error response codes.[PR 11841](https://github.com/Azure/azure-sdk-for-js/)
 
 ## 1.1.9 (2020-09-30)
 

diff --git a/sdk/core/core-http/package.json b/sdk/core/core-http/package.json
@@ -2,7 +2,7 @@
   "name": "@azure/core-http",
   "sdk-type": "client",
   "author": "Microsoft Corporation",
-  "version": "1.1.10",
+  "version": "1.2.0",
   "description": "Isomorphic client Runtime for Typescript/node.js/browser javascript client libraries generated using AutoRest",
   "tags": [
     "isomorphic",

diff --git a/sdk/core/core-http/review/core-http.api.md b/sdk/core/core-http/review/core-http.api.md
@@ -471,6 +471,7 @@ export interface OperationRequestOptions {
 export interface OperationResponse {
     bodyMapper?: Mapper;
     headersMapper?: Mapper;
+    isError?: boolean;
 }
 
 // @public