Skip to content

Commit

Permalink
[core-lro] Include errors (#25500)
Browse files Browse the repository at this point in the history
### Packages impacted by this PR
@azure/core-lro

### Issues associated with this PR
#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)
  • Loading branch information
deyaaeldeen authored Apr 11, 2023
1 parent 378751f commit f5d77f1
Show file tree
Hide file tree
Showing 8 changed files with 128 additions and 3 deletions.
2 changes: 2 additions & 0 deletions sdk/core/core-lro/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@

### Other Changes

- Include detailed error information in failed polling error messages.

## 2.5.2 (2023-04-06)

### Bugs Fixed
Expand Down
3 changes: 3 additions & 0 deletions sdk/core/core-lro/src/http/models.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// Licensed under the MIT license.

import { AbortSignalLike } from "@azure/abort-controller";
import { LroError } from "../poller/models";

// TODO: rename to ResourceLocationConfig
/**
Expand All @@ -20,6 +21,8 @@ export interface ResponseBody extends Record<string, unknown> {
provisioningState?: unknown;
/** The properties of the provisioning process */
properties?: { provisioningState?: unknown } & Record<string, unknown>;
/** The error if the operation failed */
error?: Partial<LroError>;
}

/**
Expand Down
19 changes: 19 additions & 0 deletions sdk/core/core-lro/src/http/operation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import {
ResponseBody,
} from "./models";
import {
LroError,
OperationConfig,
OperationStatus,
RestorableOperationState,
Expand Down Expand Up @@ -171,6 +172,23 @@ export function parseRetryAfter<T>({ rawResponse }: LroResponse<T>): number | un
return undefined;
}

export function getErrorFromResponse<T>(response: LroResponse<T>): 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();
Expand Down Expand Up @@ -325,6 +343,7 @@ export async function pollHttpOperation<TState, TResult>(inputs: {
processResult: processResult
? ({ flatResponse }, inputState) => processResult(flatResponse, inputState)
: ({ flatResponse }) => flatResponse as TResult,
getError: getErrorFromResponse,
updateState,
getPollingInterval: parseRetryAfter,
getOperationLocation,
Expand Down
2 changes: 2 additions & 0 deletions sdk/core/core-lro/src/http/poller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
import { LongRunningOperation, LroResponse } from "./models";
import { OperationState, SimplePollerLike } from "../poller/models";
import {
getErrorFromResponse,
getOperationLocation,
getOperationStatus,
getResourceLocation,
Expand Down Expand Up @@ -41,6 +42,7 @@ export async function createHttpPoller<TResult, TState extends OperationState<TR
getOperationLocation,
getResourceLocation,
getPollingInterval: parseRetryAfter,
getError: getErrorFromResponse,
resolveOnUnsuccessful,
})(
{
Expand Down
16 changes: 16 additions & 0 deletions sdk/core/core-lro/src/poller/models.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,18 @@ export interface CreatePollerOptions<TResponse, TResult, TState> {
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`.
*/
Expand Down Expand Up @@ -115,6 +127,10 @@ export interface BuildCreatePollerOptions<TResponse, TState> {
* 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.
*/
Expand Down
52 changes: 49 additions & 3 deletions sdk/core/core-lro/src/poller/operation.ts
Original file line number Diff line number Diff line change
@@ -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";

Expand Down Expand Up @@ -33,24 +40,60 @@ function setStateError<TState, TResult>(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<TState, TResult, TResponse>(result: {
status: OperationStatus;
response: TResponse;
state: RestorableOperationState<TState>;
stateProxy: StateProxy<TState, TResult>;
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": {
Expand Down Expand Up @@ -200,6 +243,7 @@ export async function pollOperation<TResponse, TState, TResult, TOptions>(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;
Expand All @@ -217,6 +261,7 @@ export async function pollOperation<TResponse, TState, TResult, TOptions>(inputs
withOperationLocation,
getPollingInterval,
processResult,
getError,
updateState,
setDelay,
isDone,
Expand All @@ -241,6 +286,7 @@ export async function pollOperation<TResponse, TState, TResult, TOptions>(inputs
stateProxy,
isDone,
processResult,
getError,
setErrorAsResult,
});

Expand Down
2 changes: 2 additions & 0 deletions sdk/core/core-lro/src/poller/poller.ts
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ export function buildCreatePoller<TResponse, TResult, TState extends OperationSt
isOperationError,
getResourceLocation,
getPollingInterval,
getError,
resolveOnUnsuccessful,
} = inputs;
return async (
Expand Down Expand Up @@ -171,6 +172,7 @@ export function buildCreatePoller<TResponse, TResult, TState extends OperationSt
getOperationStatus: getStatusFromPollResponse,
getResourceLocation,
processResult,
getError,
updateState,
options: pollOptions,
setDelay: (pollIntervalInMs) => {
Expand Down
35 changes: 35 additions & 0 deletions sdk/core/core-lro/test/lro.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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 },
},
});
});
});
});
}
Expand Down

0 comments on commit f5d77f1

Please sign in to comment.