From f5d77f13e48c7de7a704510c52c208f654895226 Mon Sep 17 00:00:00 2001 From: Deyaaeldeen Almahallawi Date: Tue, 11 Apr 2023 14:38:49 -0700 Subject: [PATCH] [core-lro] Include errors (#25500) ### Packages impacted by this PR @azure/core-lro ### Issues associated with this PR https://github.com/Azure/azure-sdk-for-js/issues/25374. Note that CI failures are unrelated. ### Describe the problem that is addressed by this PR Error messages when polling fails don't contain informaion on why the failure happened. ### What are the possible designs available to address the problem? If there are more than one possible design, why was the one in this PR chosen? N/A ### Are there test cases added in this PR? _(If not, why?)_ Yes ### Provide a list of related PRs _(if any)_ N/A ### Command used to generate this PR:**_(Applicable only to SDK release request PRs)_ ### Checklists - [x] Added impacted package name to the issue description - [ ] Does this PR needs any fixes in the SDK Generator?** _(If so, create an Issue in the [Autorest/typescript](https://github.com/Azure/autorest.typescript) repository and link it here)_ - [x] Added a changelog (if necessary) --- sdk/core/core-lro/CHANGELOG.md | 2 + sdk/core/core-lro/src/http/models.ts | 3 ++ sdk/core/core-lro/src/http/operation.ts | 19 +++++++++ sdk/core/core-lro/src/http/poller.ts | 2 + sdk/core/core-lro/src/poller/models.ts | 16 +++++++ sdk/core/core-lro/src/poller/operation.ts | 52 +++++++++++++++++++++-- sdk/core/core-lro/src/poller/poller.ts | 2 + sdk/core/core-lro/test/lro.spec.ts | 35 +++++++++++++++ 8 files changed, 128 insertions(+), 3 deletions(-) diff --git a/sdk/core/core-lro/CHANGELOG.md b/sdk/core/core-lro/CHANGELOG.md index 1f10f4f4daf4..4de074a97004 100644 --- a/sdk/core/core-lro/CHANGELOG.md +++ b/sdk/core/core-lro/CHANGELOG.md @@ -10,6 +10,8 @@ ### Other Changes +- Include detailed error information in failed polling error messages. + ## 2.5.2 (2023-04-06) ### Bugs Fixed diff --git a/sdk/core/core-lro/src/http/models.ts b/sdk/core/core-lro/src/http/models.ts index f3f2282d1950..2a3711fdef0c 100644 --- a/sdk/core/core-lro/src/http/models.ts +++ b/sdk/core/core-lro/src/http/models.ts @@ -2,6 +2,7 @@ // Licensed under the MIT license. import { AbortSignalLike } from "@azure/abort-controller"; +import { LroError } from "../poller/models"; // TODO: rename to ResourceLocationConfig /** @@ -20,6 +21,8 @@ export interface ResponseBody extends Record { provisioningState?: unknown; /** The properties of the provisioning process */ properties?: { provisioningState?: unknown } & Record; + /** The error if the operation failed */ + error?: Partial; } /** diff --git a/sdk/core/core-lro/src/http/operation.ts b/sdk/core/core-lro/src/http/operation.ts index b1964c4d2739..94287ae19f86 100644 --- a/sdk/core/core-lro/src/http/operation.ts +++ b/sdk/core/core-lro/src/http/operation.ts @@ -10,6 +10,7 @@ import { ResponseBody, } from "./models"; import { + LroError, OperationConfig, OperationStatus, RestorableOperationState, @@ -171,6 +172,23 @@ export function parseRetryAfter({ rawResponse }: LroResponse): number | un return undefined; } +export function getErrorFromResponse(response: LroResponse): LroError | undefined { + const error = (response.flatResponse as ResponseBody).error; + if (!error) { + logger.warning( + `The long-running operation failed but there is no error property in the response's body` + ); + return; + } + if (!error.code || !error.message) { + logger.warning( + `The long-running operation failed but the error property in the response's body doesn't contain code or message` + ); + return; + } + return error as LroError; +} + function calculatePollingIntervalFromDate(retryAfterDate: Date): number | undefined { const timeNow = Math.floor(new Date().getTime()); const retryAfterTime = retryAfterDate.getTime(); @@ -325,6 +343,7 @@ export async function pollHttpOperation(inputs: { processResult: processResult ? ({ flatResponse }, inputState) => processResult(flatResponse, inputState) : ({ flatResponse }) => flatResponse as TResult, + getError: getErrorFromResponse, updateState, getPollingInterval: parseRetryAfter, getOperationLocation, diff --git a/sdk/core/core-lro/src/http/poller.ts b/sdk/core/core-lro/src/http/poller.ts index c18b6bb28ad2..79d7360449f9 100644 --- a/sdk/core/core-lro/src/http/poller.ts +++ b/sdk/core/core-lro/src/http/poller.ts @@ -4,6 +4,7 @@ import { LongRunningOperation, LroResponse } from "./models"; import { OperationState, SimplePollerLike } from "../poller/models"; import { + getErrorFromResponse, getOperationLocation, getOperationStatus, getResourceLocation, @@ -41,6 +42,7 @@ export async function createHttpPoller { withOperationLocation?: (operationLocation: string) => void; } +export interface LroError { + code: string; + innererror?: InnerError; + message: string; +} + +export interface InnerError { + code: string; + message: string; + innererror?: InnerError; +} + /** * Options for `buildCreatePoller`. */ @@ -115,6 +127,10 @@ export interface BuildCreatePollerOptions { * wait before sending the next polling request. */ getPollingInterval?: (response: TResponse) => number | undefined; + /** + * Extracts an error model from a response. + */ + getError?: (response: TResponse) => LroError | undefined; /** * Control whether to throw an exception if the operation failed or was canceled. */ diff --git a/sdk/core/core-lro/src/poller/operation.ts b/sdk/core/core-lro/src/poller/operation.ts index 492e333d8663..3f9fe5a64448 100644 --- a/sdk/core/core-lro/src/poller/operation.ts +++ b/sdk/core/core-lro/src/poller/operation.ts @@ -1,7 +1,14 @@ // Copyright (c) Microsoft Corporation. // Licensed under the MIT license. -import { Operation, OperationStatus, RestorableOperationState, StateProxy } from "./models"; +import { + LroError, + InnerError, + Operation, + OperationStatus, + RestorableOperationState, + StateProxy, +} from "./models"; import { logger } from "../logger"; import { terminalStates } from "./constants"; @@ -33,24 +40,60 @@ function setStateError(inputs: { }; } +function appendReadableErrorMessage(currentMessage: string, innerMessage: string): string { + let message = currentMessage; + if (message.slice(-1) !== ".") { + message = message + "."; + } + return message + " " + innerMessage; +} + +function simplifyError(err: LroError): { + code: string; + message: string; +} { + let message = err.message; + let code = err.code; + let curErr = err as InnerError; + while (curErr.innererror) { + curErr = curErr.innererror; + code = curErr.code; + message = appendReadableErrorMessage(message, curErr.message); + } + return { + code, + message, + }; +} + function processOperationStatus(result: { status: OperationStatus; response: TResponse; state: RestorableOperationState; stateProxy: StateProxy; processResult?: (result: TResponse, state: TState) => TResult; + getError?: (response: TResponse) => LroError | undefined; isDone?: (lastResponse: TResponse, state: TState) => boolean; setErrorAsResult: boolean; }): void { - const { state, stateProxy, status, isDone, processResult, response, setErrorAsResult } = result; + const { state, stateProxy, status, isDone, processResult, getError, response, setErrorAsResult } = + result; switch (status) { case "succeeded": { stateProxy.setSucceeded(state); break; } case "failed": { - stateProxy.setError(state, new Error(`The long-running operation has failed`)); + const err = getError?.(response); + let postfix = ""; + if (err) { + const { code, message } = simplifyError(err); + postfix = `. ${code}. ${message}`; + } + const errStr = `The long-running operation has failed${postfix}`; + stateProxy.setError(state, new Error(errStr)); stateProxy.setFailed(state); + logger.warning(errStr); break; } case "canceled": { @@ -200,6 +243,7 @@ export async function pollOperation(inputs ) => string | undefined; withOperationLocation?: (operationLocation: string, isUpdated: boolean) => void; processResult?: (result: TResponse, state: TState) => TResult; + getError?: (response: TResponse) => LroError | undefined; updateState?: (state: TState, lastResponse: TResponse) => void; isDone?: (lastResponse: TResponse, state: TState) => boolean; setErrorAsResult: boolean; @@ -217,6 +261,7 @@ export async function pollOperation(inputs withOperationLocation, getPollingInterval, processResult, + getError, updateState, setDelay, isDone, @@ -241,6 +286,7 @@ export async function pollOperation(inputs stateProxy, isDone, processResult, + getError, setErrorAsResult, }); diff --git a/sdk/core/core-lro/src/poller/poller.ts b/sdk/core/core-lro/src/poller/poller.ts index 82ec1ab0af39..4dc4f1f042a1 100644 --- a/sdk/core/core-lro/src/poller/poller.ts +++ b/sdk/core/core-lro/src/poller/poller.ts @@ -56,6 +56,7 @@ export function buildCreatePoller { diff --git a/sdk/core/core-lro/test/lro.spec.ts b/sdk/core/core-lro/test/lro.spec.ts index 9ca7dc5cbf02..351092237f3c 100644 --- a/sdk/core/core-lro/test/lro.spec.ts +++ b/sdk/core/core-lro/test/lro.spec.ts @@ -2593,6 +2593,41 @@ matrix( }); assert.equal(poller.getResult()?.properties?.provisioningState, "Canceled"); }); + it("prints an error message based on the error in the status monitor", async () => { + const pollingPath = "/postlocation/retry/succeeded/operationResults/200/"; + const code = "InvalidRequest"; + const message = "Bad Request"; + const body = { status: "Failed", error: { code, message } }; + await assertDivergentBehavior({ + op: runLro({ + routes: [ + { + method: "POST", + status: 202, + headers: { + "Operation-Location": pollingPath, + }, + body: `{"status":"Running"}`, + }, + { + method: "GET", + path: pollingPath, + status: 200, + body: JSON.stringify(body), + }, + ], + }), + throwOnNon2xxResponse, + throwing: { + messagePattern: new RegExp( + `The long-running operation has failed. ${code}. ${message}` + ), + }, + notThrowing: { + result: { ...body, statusCode: 200 }, + }, + }); + }); }); }); }