From 91610a5ffa937acb998dda45fae516a0f418c62c Mon Sep 17 00:00:00 2001 From: Dan Schulte Date: Thu, 21 Jun 2018 10:45:51 -0700 Subject: [PATCH 1/3] Move error deserialization into serializationPolicy --- lib/operationResponse.ts | 24 +++++++ lib/operationSpec.ts | 7 ++ lib/policies/exponentialRetryPolicy.ts | 32 +++++---- lib/policies/redirectPolicy.ts | 8 ++- lib/policies/serializationPolicy.ts | 91 +++++++++++++++++++++++++- 5 files changed, 144 insertions(+), 18 deletions(-) create mode 100644 lib/operationResponse.ts diff --git a/lib/operationResponse.ts b/lib/operationResponse.ts new file mode 100644 index 000000000000..d1106616354b --- /dev/null +++ b/lib/operationResponse.ts @@ -0,0 +1,24 @@ +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; + + /** + * Whether or not the response body will be a cloud error. + */ + isCloudError?: boolean; +} \ No newline at end of file diff --git a/lib/operationSpec.ts b/lib/operationSpec.ts index a714d497e993..b0c599c3812c 100644 --- 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 }; } \ No newline at end of file diff --git a/lib/policies/exponentialRetryPolicy.ts b/lib/policies/exponentialRetryPolicy.ts index c7e643aacd0d..abcace840b35 100644 --- 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 { - 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 { + 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 { - 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 index 169b6612f29c..76129ac1d035 100644 --- 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 { - 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 index c86cb3a8f21f..6a62f2984c35 100644 --- 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 { + public async sendRequest(request: WebResource): Promise { let result: Promise; 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 { + 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) { + if (defaultResponseSpec.isCloudError) { + 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; + } + } + + const defaultResponseBodyMapper: Mapper | undefined = defaultResponseSpec.bodyMapper; + 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; +} \ No newline at end of file From 404431c92387d797aea3ff8743eb83c1c7a20f54 Mon Sep 17 00:00:00 2001 From: Dan Schulte Date: Thu, 21 Jun 2018 11:44:49 -0700 Subject: [PATCH 2/3] Remove OperationResponse.isCloudError property --- lib/operationResponse.ts | 5 ----- lib/policies/serializationPolicy.ts | 4 ++-- package.json | 2 +- 3 files changed, 3 insertions(+), 8 deletions(-) diff --git a/lib/operationResponse.ts b/lib/operationResponse.ts index d1106616354b..c00e07f9ff2e 100644 --- a/lib/operationResponse.ts +++ b/lib/operationResponse.ts @@ -16,9 +16,4 @@ export interface OperationResponse { * The mapper that will be used to deserialize the response body. */ bodyMapper?: Mapper; - - /** - * Whether or not the response body will be a cloud error. - */ - isCloudError?: boolean; } \ No newline at end of file diff --git a/lib/policies/serializationPolicy.ts b/lib/policies/serializationPolicy.ts index 6a62f2984c35..0827bc224bc1 100644 --- a/lib/policies/serializationPolicy.ts +++ b/lib/policies/serializationPolicy.ts @@ -100,7 +100,8 @@ export class SerializationPolicy extends BaseRequestPolicy { let parsedErrorResponse: { [key: string]: any } = response.parsedBody; try { if (parsedErrorResponse) { - if (defaultResponseSpec.isCloudError) { + const defaultResponseBodyMapper: Mapper | undefined = defaultResponseSpec.bodyMapper; + if (defaultResponseBodyMapper && defaultResponseBodyMapper.serializedName === "CloudError") { if (parsedErrorResponse.error) { parsedErrorResponse = parsedErrorResponse.error; } @@ -122,7 +123,6 @@ export class SerializationPolicy extends BaseRequestPolicy { } } - const defaultResponseBodyMapper: Mapper | undefined = defaultResponseSpec.bodyMapper; if (defaultResponseBodyMapper) { let valueToDeserialize: any = parsedErrorResponse; if (operationSpec.isXML && defaultResponseBodyMapper.type.name === MapperType.Sequence) { diff --git a/package.json b/package.json index 54f8f5651872..6b05e71fa198 100644 --- 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", From d47e2a127f7d0f13f8d2fcdb6aee2afa6145184e Mon Sep 17 00:00:00 2001 From: Dan Schulte Date: Thu, 21 Jun 2018 11:50:53 -0700 Subject: [PATCH 3/3] Fix broken test --- test/shared/serviceClientTests.ts | 76 ++++++++++++++++--------------- 1 file changed, 40 insertions(+), 36 deletions(-) diff --git a/test/shared/serviceClientTests.ts b/test/shared/serviceClientTests.ts index 76541d6e897d..fca56fe7cf84 100644 --- 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); }); -}) \ No newline at end of file +}); \ No newline at end of file