Skip to content

Commit

Permalink
Add RestResponse type and more runtime deserialization features (Azur…
Browse files Browse the repository at this point in the history
…e#205)

* Add RestResponse type and more runtime deserialization features

* _response should always win in name conflicts

* Fix linter errors

* Make blobBody just a Promise, not a function. Resolves Azure#192

* Fix tests

* Remove utils.responseToBody

* Update RestResponse doc

* Add default error to retry policy. Move error codes to RestError static properties.

* Bump to v0.21
  • Loading branch information
RikkiGibson authored Aug 29, 2018
1 parent da6cd30 commit 16f681c
Show file tree
Hide file tree
Showing 13 changed files with 129 additions and 127 deletions.
6 changes: 3 additions & 3 deletions lib/axiosHttpClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ export class AxiosHttpClient implements HttpClient {

const abortSignal = httpRequest.abortSignal;
if (abortSignal && abortSignal.aborted) {
throw new RestError("The request was aborted", "REQUEST_ABORTED_ERROR", undefined, httpRequest);
throw new RestError("The request was aborted", RestError.REQUEST_ABORTED_ERROR, undefined, httpRequest);
}

let abortListener: (() => void) | undefined;
Expand Down Expand Up @@ -136,10 +136,10 @@ export class AxiosHttpClient implements HttpClient {
res = await axiosClient(config);
} catch (err) {
if (err instanceof axios.Cancel) {
throw new RestError(err.message, "REQUEST_ABORTED_ERROR", undefined, httpRequest);
throw new RestError(err.message, RestError.REQUEST_SEND_ERROR, undefined, httpRequest);
} else {
const axiosErr = err as AxiosError;
throw new RestError(axiosErr.message, "REQUEST_SEND_ERROR", undefined, httpRequest);
throw new RestError(axiosErr.message, RestError.REQUEST_SEND_ERROR, undefined, httpRequest);
}
} finally {
if (abortSignal && abortListener) {
Expand Down
16 changes: 15 additions & 1 deletion lib/httpOperationResponse.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ export interface HttpOperationResponse extends HttpResponse {
* The response body as a browser Blob.
* Always undefined in node.js.
*/
blobBody?: () => Promise<Blob>;
blobBody?: Promise<Blob>;

/**
* NODEJS ONLY
Expand All @@ -68,3 +68,17 @@ export interface HttpOperationResponse extends HttpResponse {
*/
readableStreamBody?: NodeJS.ReadableStream;
}

/**
* The flattened response to a REST call.
* Contains the underlying HttpOperationResponse as well as
* the merged properties of the parsedBody, parsedHeaders, etc.
*/
export interface RestResponse {
/**
* The underlying HTTP response containing both raw and deserialized response data.
*/
_response: HttpOperationResponse;

[key: string]: any;
}
6 changes: 3 additions & 3 deletions lib/msRest.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,15 @@ export { WebResource, HttpRequestBody, RequestPrepareOptions, HttpMethods, Param
export { DefaultHttpClient } from "./defaultHttpClient";
export { HttpClient } from "./httpClient";
export { HttpHeaders } from "./httpHeaders";
export { HttpOperationResponse, HttpResponse } from "./httpOperationResponse";
export { HttpOperationResponse, HttpResponse, RestResponse } from "./httpOperationResponse";
export { HttpPipelineLogger } from "./httpPipelineLogger";
export { HttpPipelineLogLevel } from "./httpPipelineLogLevel";
export { RestError } from "./restError";
export { OperationArguments } from "./operationArguments";
export { OperationParameter, OperationQueryParameter, OperationURLParameter } from "./operationParameter";
export { OperationResponse } from "./operationResponse";
export { OperationSpec } from "./operationSpec";
export { ServiceClient, ServiceClientOptions } from "./serviceClient";
export { ServiceClient, ServiceClientOptions, flattenResponse } from "./serviceClient";
export { QueryCollectionFormat } from "./queryCollectionFormat";
export { Constants } from "./util/constants";
export { logPolicy } from "./policies/logPolicy";
Expand All @@ -33,7 +33,7 @@ export {
export {
stripRequest, stripResponse, delay,
executePromisesSequentially, generateUuid, encodeUri, ServiceCallback,
promiseToCallback, responseToBody, promiseToServiceCallback, isValidUuid,
promiseToCallback, promiseToServiceCallback, isValidUuid,
applyMixins, isNode, isDuration
} from "./util/utils";
export { URLBuilder, URLQuery } from "./url";
Expand Down
7 changes: 5 additions & 2 deletions lib/policies/deserializationPolicy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,9 @@ export function deserializeResponseBody(response: HttpOperationResponse): Promis
restError.response = utils.stripResponse(parsedResponse);
return Promise.reject(restError);
}
} else if (operationSpec.httpMethod === "HEAD") {
// head methods never have a body, but we return a boolean to indicate presence/absence of the resource
parsedResponse.parsedBody = response.status >= 200 && response.status < 300;
}

if (responseSpec.headersMapper) {
Expand All @@ -160,9 +163,9 @@ export function deserializeResponseBody(response: HttpOperationResponse): Promis
}

function parse(operationResponse: HttpOperationResponse): Promise<HttpOperationResponse> {
const errorHandler = (err: any) => {
const errorHandler = (err: Error & { code: string }) => {
const msg = `Error "${err}" occurred while parsing the response body - ${operationResponse.bodyAsText}.`;
const errCode = err.code || "PARSE_ERROR";
const errCode = err.code || RestError.PARSE_ERROR;
const e = new RestError(msg, errCode, operationResponse.status, operationResponse.request, operationResponse, operationResponse.bodyAsText);
return Promise.reject(e);
};
Expand Down
21 changes: 14 additions & 7 deletions lib/policies/exponentialRetryPolicy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import { HttpOperationResponse } from "../httpOperationResponse";
import * as utils from "../util/utils";
import { WebResource } from "../webResource";
import { BaseRequestPolicy, RequestPolicy, RequestPolicyFactory, RequestPolicyOptions } from "./requestPolicy";
import { RestError } from "../restError";

export interface RetryData {
retryCount: number;
Expand Down Expand Up @@ -86,8 +87,8 @@ export class ExponentialRetryPolicy extends BaseRequestPolicy {
* @param {RetryData} retryData The retry data.
* @return {boolean} True if the operation qualifies for a retry; false otherwise.
*/
function shouldRetry(policy: ExponentialRetryPolicy, statusCode: number, retryData: RetryData): boolean {
if ((statusCode < 500 && statusCode !== 408) || statusCode === 501 || statusCode === 505) {
function shouldRetry(policy: ExponentialRetryPolicy, statusCode: number | undefined, retryData: RetryData): boolean {
if (statusCode == undefined || (statusCode < 500 && statusCode !== 408) || statusCode === 501 || statusCode === 505) {
return false;
}

Expand Down Expand Up @@ -138,18 +139,24 @@ function updateRetryData(policy: ExponentialRetryPolicy, retryData?: RetryData,
return retryData;
}

function retry(policy: ExponentialRetryPolicy, request: WebResource, response: HttpOperationResponse, retryData?: RetryData, requestError?: RetryError): Promise<HttpOperationResponse> {
function retry(policy: ExponentialRetryPolicy, request: WebResource, response?: HttpOperationResponse, retryData?: RetryData, requestError?: RetryError): Promise<HttpOperationResponse> {
retryData = updateRetryData(policy, retryData, requestError);
const isAborted: boolean | undefined = request.abortSignal && request.abortSignal.aborted;
if (!isAborted && shouldRetry(policy, response.status, retryData)) {
if (!isAborted && shouldRetry(policy, response && response.status, retryData)) {
return utils.delay(retryData.retryInterval)
.then(() => policy._nextPolicy.sendRequest(request.clone()))
.then(res => retry(policy, request, res, retryData, undefined))
.catch(err => retry(policy, request, response, retryData, err));
} else if (isAborted || requestError != undefined) {
} else if (isAborted || requestError || !response) {
// If the operation failed in the end, return all errors instead of just the last one
requestError = retryData.error;
return Promise.reject(requestError);
const err = retryData.error ||
new RestError(
"Failed to send the request.",
RestError.REQUEST_SEND_ERROR,
response && response.status,
response && response.request,
response);
return Promise.reject(err);
} else {
return Promise.resolve(response);
}
Expand Down
4 changes: 4 additions & 0 deletions lib/restError.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@ import { HttpOperationResponse } from "./httpOperationResponse";
import { WebResource } from "./webResource";

export class RestError extends Error {
static readonly REQUEST_SEND_ERROR = "REQUEST_SEND_ERROR";
static readonly REQUEST_ABORTED_ERROR = "REQUEST_ABORTED_ERROR";
static readonly PARSE_ERROR = "PARSE_ERROR";

code?: string;
statusCode?: number;
request?: WebResource;
Expand Down
76 changes: 72 additions & 4 deletions lib/serviceClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@
import { ServiceClientCredentials } from "./credentials/serviceClientCredentials";
import { DefaultHttpClient } from "./defaultHttpClient";
import { HttpClient } from "./httpClient";
import { HttpOperationResponse } from "./httpOperationResponse";
import { HttpOperationResponse, RestResponse } from "./httpOperationResponse";
import { HttpPipelineLogger } from "./httpPipelineLogger";
import { OperationArguments } from "./operationArguments";
import { getPathStringFromParameter, getPathStringFromParameterPath, OperationParameter, ParameterPath } from "./operationParameter";
Expand All @@ -25,6 +25,8 @@ import { Constants } from "./util/constants";
import * as utils from "./util/utils";
import { stringifyXML } from "./util/xml";
import { RequestOptionsBase, RequestPrepareOptions, WebResource } from "./webResource";
import { OperationResponse } from "./operationResponse";
import { ServiceCallback } from "./util/utils";

/**
* Options to be provided while creating the client.
Expand Down Expand Up @@ -176,11 +178,17 @@ export class ServiceClient {
* Send an HTTP request that is populated using the provided OperationSpec.
* @param {OperationArguments} operationArguments The arguments that the HTTP request's templated values will be populated from.
* @param {OperationSpec} operationSpec The OperationSpec to use to populate the httpRequest.
* @param {ServiceCallback} callback The callback to call when the response is received.
*/
sendOperationRequest(operationArguments: OperationArguments, operationSpec: OperationSpec): Promise<HttpOperationResponse> {
sendOperationRequest(operationArguments: OperationArguments, operationSpec: OperationSpec, callback?: ServiceCallback<any>): Promise<RestResponse> {
if (typeof operationArguments.options === "function") {
callback = operationArguments.options;
operationArguments.options = undefined;
}

const httpRequest = new WebResource();

let result: Promise<HttpOperationResponse>;
let result: Promise<RestResponse>;
try {
const baseUri: string | undefined = operationSpec.baseUrl || this.baseUri;
if (!baseUri) {
Expand Down Expand Up @@ -294,10 +302,20 @@ export class ServiceClient {
httpRequest.streamResponseBody = isStreamOperation(operationSpec);
}

result = this.sendRequest(httpRequest);
result = this.sendRequest(httpRequest)
.then(res => flattenResponse(res, operationSpec.responses[res.status]));
} catch (error) {
result = Promise.reject(error);
}

const cb = callback;
if (cb) {
result
// tslint:disable-next-line:no-null-keyword
.then(res => cb(null, res._response.parsedBody, res._response.request, res._response))
.catch(err => cb(err));
}

return result;
}
}
Expand Down Expand Up @@ -460,3 +478,53 @@ function getPropertyFromParameterPath(parent: { [parameterName: string]: any },
}
return result;
}

export function flattenResponse(_response: HttpOperationResponse, responseSpec: OperationResponse | undefined): RestResponse {
const parsedHeaders = _response.parsedHeaders;
const bodyMapper = responseSpec && responseSpec.bodyMapper;
if (bodyMapper) {
const typeName = bodyMapper.type.name;
if (typeName === "Stream") {
return {
...parsedHeaders,
blobBody: _response.blobBody,
readableStreamBody: _response.readableStreamBody,
_response
};
}

if (typeName === "Sequence") {
const arrayResponse = [...(_response.parsedBody) || []] as RestResponse & any[];
if (parsedHeaders) {
for (const key of Object.keys(parsedHeaders)) {
arrayResponse[key] = parsedHeaders[key];
}
}
arrayResponse._response = _response;
return arrayResponse;
}

if (typeName === "Composite" || typeName === "Dictionary") {
return {
...parsedHeaders,
..._response.parsedBody,
_response
};
}
}

if (bodyMapper || _response.request.method === "HEAD") {
// primitive body types and HEAD booleans
return {
...parsedHeaders,
body: _response.parsedBody,
_response
};
}

return {
...parsedHeaders,
..._response.parsedBody,
_response
};
}
27 changes: 2 additions & 25 deletions lib/util/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,29 +12,6 @@ import { Constants } from "./constants";
*/
export const isNode = typeof navigator === "undefined" && typeof process !== "undefined";

/**
* Adapts a WithHttpOperationResponse method to unwrap the body and accept an optional callback.
* @param httpOperationResponseMethod the method to call and apply a callback to
* @param args the arguments to the method, optionally including a trailing callback.
*/
export function responseToBody(httpOperationResponseMethod: (...args: any[]) => Promise<HttpOperationResponse>, ...args: any[]): Promise<any> | undefined {
// The generated code will always pass (options, callback) as the last two args.
// But `options` could actually be the callback for the sake of user convenience.
let callback = args.pop();
if (typeof callback !== "function" && typeof args[args.length - 1] === "function") {
callback = args.pop();
}
if (typeof callback === "function") {
httpOperationResponseMethod(...args)
// tslint:disable-next-line:no-null-keyword
.then(res => callback(null, res.parsedBody, res.request, res))
.catch(err => callback(err));
} else {
return httpOperationResponseMethod(...args).then((res: HttpOperationResponse) => res.parsedBody);
}
return undefined; // optimized out
}

/**
* Checks if a parsed URL is HTTPS
*
Expand Down Expand Up @@ -194,12 +171,12 @@ export function delay<T>(t: number, value?: T): Promise<T> {
export interface ServiceCallback<TResult> {
/**
* A method that will be invoked as a callback to a service function.
* @param {Error | RestError} err The error occurred if any, while executing the request; otherwise null.
* @param {Error | RestError | null} err The error occurred if any, while executing the request; otherwise null.
* @param {TResult} [result] The deserialized response body if an error did not occur.
* @param {WebResource} [request] The raw/actual request sent to the server if an error did not occur.
* @param {HttpOperationResponse} [response] The raw/actual response from the server if an error did not occur.
*/
(err: Error | RestError, result?: TResult, request?: WebResource, response?: HttpOperationResponse): void;
(err: Error | RestError | null, result?: TResult, request?: WebResource, response?: HttpOperationResponse): void;
}

/**
Expand Down
10 changes: 5 additions & 5 deletions lib/xhrHttpClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,7 @@ export class XhrHttpClient implements HttpClient {
xhr.addEventListener("readystatechange", () => {
// Resolve as soon as headers are loaded
if (xhr.readyState === XMLHttpRequest.HEADERS_RECEIVED) {
const bodyPromise = new Promise<Blob>((resolve, reject) => {
const blobBody = new Promise<Blob>((resolve, reject) => {
xhr.addEventListener("load", () => {
resolve(xhr.response);
});
Expand All @@ -84,7 +84,7 @@ export class XhrHttpClient implements HttpClient {
request,
status: xhr.status,
headers: parseHeaders(xhr),
blobBody: () => bodyPromise
blobBody
});
}
});
Expand Down Expand Up @@ -127,7 +127,7 @@ export function parseHeaders(xhr: XMLHttpRequest) {
}

function rejectOnTerminalEvent(request: WebResource, xhr: XMLHttpRequest, reject: (err: any) => void) {
xhr.addEventListener("error", () => reject(new RestError(`Failed to send request to ${request.url}`, "REQUEST_SEND_ERROR", undefined, request)));
xhr.addEventListener("abort", () => reject(new RestError("The request was aborted", "REQUEST_ABORTED_ERROR", undefined, request)));
xhr.addEventListener("timeout", () => reject(new RestError(`timeout of ${xhr.timeout}ms exceeded`, "REQUEST_SEND_ERROR", undefined, request)));
xhr.addEventListener("error", () => reject(new RestError(`Failed to send request to ${request.url}`, RestError.REQUEST_SEND_ERROR, undefined, request)));
xhr.addEventListener("abort", () => reject(new RestError("The request was aborted", RestError.REQUEST_ABORTED_ERROR, undefined, request)));
xhr.addEventListener("timeout", () => reject(new RestError(`timeout of ${xhr.timeout}ms exceeded`, RestError.REQUEST_SEND_ERROR, undefined, request)));
}
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
"email": "azsdkteam@microsoft.com",
"url": "https://github.com/Azure/ms-rest-js"
},
"version": "0.20.0",
"version": "0.21.0",
"description": "Isomorphic client Runtime for Typescript/node.js/browser javascript client libraries generated using AutoRest",
"tags": [
"isomorphic",
Expand Down
2 changes: 1 addition & 1 deletion test/shared/defaultHttpClientTests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,7 @@ describe("defaultHttpClient", () => {
const response = await client.sendRequest(request);
const streamBody = response.readableStreamBody;
if (response.blobBody) {
await response.blobBody();
await response.blobBody;
} else if (streamBody) {
streamBody.on('data', () => {});
await new Promise((resolve, reject) => {
Expand Down
Loading

0 comments on commit 16f681c

Please sign in to comment.