Skip to content

Commit

Permalink
[core-util] Update "delay" method in core-util with abort options (#2…
Browse files Browse the repository at this point in the history
…3019)

### Packages impacted by this PR
core-util

### Issues associated with this PR
#15930 

### Describe the problem that is addressed by this PR
- Update "delay" in core-util to support abort message
- Unduplicate "delay" function in core-http and core-amqp

### 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?
- Choose the implementation with "options" parameters to keep the flexibility in case we want to add extra functionality in the future 

### Are there test cases added in this PR? _(If not, why?)_


### Provide a list of related PRs _(if any)_
#15832

### 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)_
- [ ] Added a changelog (if necessary)
  • Loading branch information
minhanh-phan authored Sep 9, 2022
1 parent 8c9b021 commit a6e366d
Show file tree
Hide file tree
Showing 17 changed files with 117 additions and 130 deletions.
12 changes: 6 additions & 6 deletions sdk/core/core-amqp/src/retry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { MessagingError, translate } from "./errors";
import { AbortSignalLike } from "@azure/abort-controller";
import { Constants } from "./util/constants";
import { checkNetworkConnection } from "./util/checkNetworkConnection";
import { delay } from "./util/utils";
import { delay } from "@azure/core-util";
import { logger } from "./log";

/**
Expand Down Expand Up @@ -263,11 +263,11 @@ export async function retry<T>(config: RetryConfig<T>): Promise<T> {
targetDelayInMs,
updatedConfig.operationType
);
await delay(
targetDelayInMs,
updatedConfig.abortSignal,
`The retry operation has been cancelled by the user.`
);
await delay(targetDelayInMs, {
abortSignal: updatedConfig.abortSignal,
abortErrorMsg: `The retry operation has been cancelled by the user.`,
});

continue;
} else {
break;
Expand Down
47 changes: 9 additions & 38 deletions sdk/core/core-amqp/src/util/utils.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,10 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT license.

import { AbortError, AbortSignalLike } from "@azure/abort-controller";
import { CancellableAsyncLock, CancellableAsyncLockImpl } from "./lock";
import { StandardAbortMessage } from "./constants";
import { AbortSignalLike } from "@azure/abort-controller";
import { WebSocketImpl } from "rhea-promise";
import { isDefined } from "@azure/core-util";
import { delay as wrapperDelay } from "@azure/core-util";

/**
* @internal
Expand Down Expand Up @@ -176,47 +175,19 @@ export class Timeout {
* @param value - The value to be resolved with after a timeout of t milliseconds.
* @returns - Resolved promise
*/
export function delay<T>(
export async function delay<T>(
delayInMs: number,
abortSignal?: AbortSignalLike,
abortErrorMsg?: string,
value?: T
): Promise<T | void> {
return new Promise((resolve, reject) => {
let timer: ReturnType<typeof setTimeout> | undefined = undefined;
let onAborted: (() => void) | undefined = undefined;

const rejectOnAbort = (): void => {
return reject(new AbortError(abortErrorMsg ? abortErrorMsg : StandardAbortMessage));
};

const removeListeners = (): void => {
if (abortSignal && onAborted) {
abortSignal.removeEventListener("abort", onAborted);
}
};

onAborted = (): void => {
if (isDefined(timer)) {
clearTimeout(timer);
}
removeListeners();
return rejectOnAbort();
};

if (abortSignal && abortSignal.aborted) {
return rejectOnAbort();
}

timer = setTimeout(() => {
removeListeners();
resolve(value);
}, delayInMs);

if (abortSignal) {
abortSignal.addEventListener("abort", onAborted);
}
await wrapperDelay(delayInMs, {
abortSignal: abortSignal,
abortErrorMsg: abortErrorMsg,
});
if (value !== undefined) {
return value;
}
}

/**
Expand Down
7 changes: 2 additions & 5 deletions sdk/core/core-http/review/core-http.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { AbortSignalLike } from '@azure/abort-controller';
import { AccessToken } from '@azure/core-auth';
import { Context } from '@azure/core-tracing';
import { Debugger } from '@azure/logger';
import { delay } from '@azure/core-util';
import { GetTokenOptions } from '@azure/core-auth';
import { isTokenCredential } from '@azure/core-auth';
import { OperationTracingOptions } from '@azure/core-tracing';
Expand Down Expand Up @@ -181,11 +182,7 @@ export class DefaultHttpClient implements HttpClient {
sendRequest(httpRequest: WebResourceLike): Promise<HttpOperationResponse>;
}

// @public
export function delay<T>(delayInMs: number, value?: T, options?: {
abortSignal?: AbortSignalLike;
abortErrorMsg?: string;
}): Promise<T | void>;
export { delay }

// @public
export interface DeserializationContentTypes {
Expand Down
2 changes: 1 addition & 1 deletion sdk/core/core-http/src/coreHttp.ts
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ export {
} from "./util/utils";
export { URLBuilder, URLQuery } from "./url";
export { AbortSignalLike } from "@azure/abort-controller";
export { delay } from "./util/delay";
export { delay } from "@azure/core-util";
// legacy exports. Use core-tracing instead (and remove on next major version update of core-http).
export { createSpanFunction, SpanConfig } from "./createSpanLegacy";

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ import {
import { Constants } from "../util/constants";
import { HttpOperationResponse } from "../httpOperationResponse";
import { WebResourceLike } from "../webResource";
import { delay } from "../util/delay";
import { delay } from "@azure/core-util";

// #region Access Token Cycler

Expand Down
2 changes: 1 addition & 1 deletion sdk/core/core-http/src/policies/exponentialRetryPolicy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ import { Constants } from "../util/constants";
import { HttpOperationResponse } from "../httpOperationResponse";
import { RestError } from "../restError";
import { WebResourceLike } from "../webResource";
import { delay } from "../util/delay";
import { delay } from "@azure/core-util";
import { logger } from "../log";

/**
Expand Down
2 changes: 1 addition & 1 deletion sdk/core/core-http/src/policies/rpRegistrationPolicy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import {
} from "./requestPolicy";
import { HttpOperationResponse } from "../httpOperationResponse";
import { WebResourceLike } from "../webResource";
import { delay } from "../util/delay";
import { delay } from "@azure/core-util";

export function rpRegistrationPolicy(retryTimeout = 30): RequestPolicyFactory {
return {
Expand Down
2 changes: 1 addition & 1 deletion sdk/core/core-http/src/policies/systemErrorRetryPolicy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ import {
} from "../util/exponentialBackoffStrategy";
import { HttpOperationResponse } from "../httpOperationResponse";
import { WebResourceLike } from "../webResource";
import { delay } from "../util/delay";
import { delay } from "@azure/core-util";

/**
* A policy that retries when there's a system error, identified by the codes "ETIMEDOUT", "ESOCKETTIMEDOUT", "ECONNREFUSED", "ECONNRESET" or "ENOENT".
Expand Down
4 changes: 2 additions & 2 deletions sdk/core/core-http/src/policies/throttlingRetryPolicy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { Constants } from "../util/constants";
import { DEFAULT_CLIENT_MAX_RETRY_COUNT } from "../util/throttlingRetryStrategy";
import { HttpOperationResponse } from "../httpOperationResponse";
import { WebResourceLike } from "../webResource";
import { delay } from "../util/delay";
import { delay } from "@azure/core-util";

type ResponseHandler = (
httpRequest: WebResourceLike,
Expand Down Expand Up @@ -88,7 +88,7 @@ export class ThrottlingRetryPolicy extends BaseRequestPolicy {
if (delayInMs) {
this.numberOfRetries += 1;

await delay(delayInMs, undefined, {
await delay(delayInMs, {
abortSignal: httpRequest.abortSignal,
abortErrorMsg: StandardAbortMessage,
});
Expand Down
63 changes: 0 additions & 63 deletions sdk/core/core-http/src/util/delay.ts

This file was deleted.

2 changes: 2 additions & 0 deletions sdk/core/core-util/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,8 @@

### Features Added

- Update `delay` method to handle `abortSignal` functionality

### Breaking Changes

### Bugs Fixed
Expand Down
1 change: 1 addition & 0 deletions sdk/core/core-util/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -69,6 +69,7 @@
"sideEffects": false,
"prettier": "@azure/eslint-plugin-azure-sdk/prettier.json",
"dependencies": {
"@azure/abort-controller": "^1.0.0",
"tslib": "^2.2.0"
},
"devDependencies": {
Expand Down
10 changes: 9 additions & 1 deletion sdk/core/core-util/review/core-util.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,22 @@
```ts

import { AbortSignalLike } from '@azure/abort-controller';

// @public
export function computeSha256Hash(content: string, encoding: "base64" | "hex"): Promise<string>;

// @public
export function computeSha256Hmac(key: string, stringToSign: string, encoding: "base64" | "hex"): Promise<string>;

// @public
export function delay(timeInMs: number): Promise<void>;
export function delay(timeInMs: number, options?: DelayOptions): Promise<void>;

// @public
export interface DelayOptions {
abortErrorMsg?: string;
abortSignal?: AbortSignalLike;
}

// @public
export function getErrorMessage(e: unknown): string;
Expand Down
58 changes: 56 additions & 2 deletions sdk/core/core-util/src/delay.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,65 @@
// Copyright (c) Microsoft Corporation.
// Licensed under the MIT license.

import { AbortError, AbortSignalLike } from "@azure/abort-controller";
import { isDefined } from "./typeGuards";

const StandardAbortMessage = "The operation was aborted.";

/**
* Options for support abort functionality for the delay method
*/
export interface DelayOptions {
/**
* The abortSignal associated with containing operation.
*/
abortSignal?: AbortSignalLike;
/**
* The abort error message associated with containing operation.
*/
abortErrorMsg?: string;
}

/**
* A wrapper for setTimeout that resolves a promise after timeInMs milliseconds.
* @param timeInMs - The number of milliseconds to be delayed.
* @param options - The options for delay - currently abort options
* @returns Promise that is resolved after timeInMs
*/
export function delay(timeInMs: number): Promise<void> {
return new Promise((resolve) => setTimeout(() => resolve(), timeInMs));
export function delay(timeInMs: number, options?: DelayOptions): Promise<void> {
return new Promise((resolve, reject) => {
let timer: ReturnType<typeof setTimeout> | undefined = undefined;
let onAborted: (() => void) | undefined = undefined;

const rejectOnAbort = (): void => {
return reject(new AbortError(options?.abortErrorMsg ?? StandardAbortMessage));
};

const removeListeners = (): void => {
if (options?.abortSignal && onAborted) {
options.abortSignal.removeEventListener("abort", onAborted);
}
};

onAborted = (): void => {
if (isDefined(timer)) {
clearTimeout(timer);
}
removeListeners();
return rejectOnAbort();
};

if (options?.abortSignal && options.abortSignal.aborted) {
return rejectOnAbort();
}

timer = setTimeout(() => {
removeListeners();
resolve();
}, timeInMs);

if (options?.abortSignal) {
options.abortSignal.addEventListener("abort", onAborted);
}
});
}
2 changes: 1 addition & 1 deletion sdk/core/core-util/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@
// Licensed under the MIT license.

export { isNode } from "./isNode";
export { delay } from "./delay";
export { delay, DelayOptions } from "./delay";
export { getRandomIntegerInclusive } from "./random";
export { isObject, UnknownObject } from "./object";
export { isError, getErrorMessage } from "./error";
Expand Down
19 changes: 19 additions & 0 deletions sdk/core/core-util/test/public/delay.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// Licensed under the MIT license.

import * as sinon from "sinon";
import { AbortController } from "@azure/abort-controller";
import { assert } from "chai";
import { delay } from "../../src";

Expand All @@ -20,4 +21,22 @@ describe("delay", function () {
// should be resolved, so we can await it and it will resolve next tick
await delayPromise;
});

it("should return when the abort signal is called", async function () {
const delayTime = 2500;
const controller = new AbortController();
const StandardAbortMessage = "The operation was aborted.";
const delayPromise = delay(delayTime, {
abortSignal: controller.signal,
abortErrorMsg: StandardAbortMessage,
});
try {
controller.abort();
await delayPromise;
assert.fail();
} catch (err: any) {
assert.strictEqual(err.name, "AbortError");
assert.strictEqual(err.message, StandardAbortMessage);
}
});
});
Loading

0 comments on commit a6e366d

Please sign in to comment.