Skip to content

Commit

Permalink
fix(util-waiter): expose minDelay and maxDelay for waiters (aws#1839)
Browse files Browse the repository at this point in the history
* fix: expose min delay and max to the client as optional on waiterconfiguration

* fix: validation error should reference exported WaiterConfiguration type

* fix: minor update to min/max delay description wording

* fix: added additional clarifying language

* fix: clarify comments around minDelay and maxDelay

* fix: waiter validation throws a proper error object
  • Loading branch information
alexforsyth authored Jan 5, 2021
1 parent 314d3b3 commit 25cb359
Show file tree
Hide file tree
Showing 6 changed files with 64 additions and 28 deletions.
4 changes: 2 additions & 2 deletions packages/util-waiter/src/createWaiter.spec.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { AbortController } from "@aws-sdk/abort-controller";

import { ResolvedWaiterOptions, WaiterState } from "./waiter";
import { WaiterOptions, WaiterState } from "./waiter";

const mockValidate = jest.fn();
jest.mock("./utils/validate", () => ({
Expand All @@ -22,7 +22,7 @@ describe("createWaiter", () => {
maxDelay: 120,
maxWaitTime: 9999,
client: "client",
} as ResolvedWaiterOptions<any>;
} as WaiterOptions<any>;
const input = "input";

const abortedState = {
Expand Down
4 changes: 2 additions & 2 deletions packages/util-waiter/src/poller.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ import { AbortController } from "@aws-sdk/abort-controller";

import { runPolling } from "./poller";
import { sleep } from "./utils/sleep";
import { ResolvedWaiterOptions, WaiterState } from "./waiter";
import { WaiterOptions, WaiterState } from "./waiter";

jest.mock("./utils/sleep");

Expand All @@ -12,7 +12,7 @@ describe(runPolling.name, () => {
maxDelay: 30,
maxWaitTime: 99999,
client: "mockClient",
} as ResolvedWaiterOptions<any>;
} as WaiterOptions<any>;
const input = "mockInput";
const abortedState = {
state: WaiterState.ABORTED,
Expand Down
4 changes: 2 additions & 2 deletions packages/util-waiter/src/poller.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import { sleep } from "./utils/sleep";
import { ResolvedWaiterOptions, WaiterResult, WaiterState } from "./waiter";
import { WaiterOptions, WaiterResult, WaiterState } from "./waiter";

/**
* Reference: https://awslabs.github.io/smithy/1.0/spec/waiters.html#waiter-retries
Expand All @@ -20,7 +20,7 @@ const randomInRange = (min: number, max: number) => min + Math.random() * (max -
* @param stateChecker function that checks the acceptor states on each poll.
*/
export const runPolling = async <Client, Input>(
{ minDelay, maxDelay, maxWaitTime, abortController, client }: ResolvedWaiterOptions<Client>,
{ minDelay, maxDelay, maxWaitTime, abortController, client }: WaiterOptions<Client>,
input: Input,
acceptorChecks: (client: Client, input: Input) => Promise<WaiterResult>
): Promise<WaiterResult> => {
Expand Down
44 changes: 36 additions & 8 deletions packages/util-waiter/src/utils/validate.spec.ts
Original file line number Diff line number Diff line change
@@ -1,18 +1,46 @@
import { ResolvedWaiterOptions } from "../waiter";
import { WaiterOptions } from "../waiter";
import { validateWaiterOptions } from "./validate";

describe(validateWaiterOptions.name, () => {
let waiterOptions: ResolvedWaiterOptions<any>;
let waiterOptions: WaiterOptions<any>;

beforeEach(() => {
waiterOptions = {
maxWaitTime: 120,
minDelay: 20,
maxDelay: 120,
maxDelay: 1200,
client: "client",
};
});

it("should not throw an error when maxDelay is proper", (done) => {
waiterOptions.maxDelay = 300;
waiterOptions.minDelay = 200;
waiterOptions.maxWaitTime = 250;
try {
validateWaiterOptions(waiterOptions);
expect(1).toBe(1);
done();
} catch (e) {
expect(e).toBe("SHOULD NOT ERROR HERE");
}
});

it("should not throw an error when maxDelay is less than minDelay", (done) => {
waiterOptions.maxDelay = 120;
waiterOptions.minDelay = 200;
waiterOptions.maxWaitTime = 250;
try {
validateWaiterOptions(waiterOptions);
expect(1).toBe("SHOULD NOT GET HERE");
} catch (e) {
expect(e.toString()).toBe(
"Error: WaiterConfiguration.maxDelay [120] must be greater than WaiterConfiguration.minDelay [200] for this waiter"
);
done();
}
});

it("should not throw an error when maxWaitTime is proper", (done) => {
waiterOptions.maxWaitTime = 300;
waiterOptions.minDelay = 200;
Expand All @@ -31,7 +59,7 @@ describe(validateWaiterOptions.name, () => {
try {
validateWaiterOptions(waiterOptions);
} catch (e) {
expect(e).toBe("WaiterOptions.maxWaitTime must be greater than 0");
expect(e.toString()).toBe("Error: WaiterConfiguration.maxWaitTime must be greater than 0");
done();
}
});
Expand All @@ -42,8 +70,8 @@ describe(validateWaiterOptions.name, () => {
try {
validateWaiterOptions(waiterOptions);
} catch (e) {
expect(e).toBe(
"WaiterOptions.maxWaitTime [150] must be greater than WaiterOptions.minDelay [200] for this waiter"
expect(e.toString()).toBe(
"Error: WaiterConfiguration.maxWaitTime [150] must be greater than WaiterConfiguration.minDelay [200] for this waiter"
);
done();
}
Expand All @@ -55,8 +83,8 @@ describe(validateWaiterOptions.name, () => {
try {
validateWaiterOptions(waiterOptions);
} catch (e) {
expect(e).toBe(
"WaiterOptions.maxWaitTime [200] must be greater than WaiterOptions.minDelay [200] for this waiter"
expect(e.toString()).toBe(
"Error: WaiterConfiguration.maxWaitTime [200] must be greater than WaiterConfiguration.minDelay [200] for this waiter"
);
}
});
Expand Down
18 changes: 14 additions & 4 deletions packages/util-waiter/src/utils/validate.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,23 @@
import { ResolvedWaiterOptions } from "../waiter";
import { WaiterOptions } from "../waiter";

/**
* Validates that waiter options are passed correctly
* @param options a waiter configuration object
*/
export const validateWaiterOptions = <Client>(options: ResolvedWaiterOptions<Client>): void => {
export const validateWaiterOptions = <Client>(options: WaiterOptions<Client>): void => {
if (options.maxWaitTime < 1) {
throw `WaiterOptions.maxWaitTime must be greater than 0`;
throw new Error(`WaiterConfiguration.maxWaitTime must be greater than 0`);
} else if (options.minDelay < 1) {
throw new Error(`WaiterConfiguration.minDelay must be greater than 0`);
} else if (options.maxDelay < 1) {
throw new Error(`WaiterConfiguration.maxDelay must be greater than 0`);
} else if (options.maxWaitTime <= options.minDelay) {
throw `WaiterOptions.maxWaitTime [${options.maxWaitTime}] must be greater than WaiterOptions.minDelay [${options.minDelay}] for this waiter`;
throw new Error(
`WaiterConfiguration.maxWaitTime [${options.maxWaitTime}] must be greater than WaiterConfiguration.minDelay [${options.minDelay}] for this waiter`
);
} else if (options.maxDelay < options.minDelay) {
throw new Error(
`WaiterConfiguration.maxDelay [${options.maxDelay}] must be greater than WaiterConfiguration.minDelay [${options.minDelay}] for this waiter`
);
}
};
18 changes: 8 additions & 10 deletions packages/util-waiter/src/waiter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,20 +15,18 @@ export interface WaiterConfiguration<Client> {
* Abort controller. Used for ending the waiter early.
*/
abortController?: AbortController;
}

export interface WaiterOptions<Client> extends WaiterConfiguration<Client> {
/**
* The minimum amount of time to delay between retries in seconds. This value defaults
* to 2 if not specified. If specified, this value MUST be greater than or equal to 1
* and less than or equal to maxDelay.
* The minimum amount of time to delay between retries in seconds. This is the
* floor of the exponential backoff. This value defaults to service default
* if not specified. This value MUST be less than or equal to maxDelay and greater than 0.
*/
minDelay?: number;

/**
* The maximum amount of time to delay between retries in seconds. The maximum amount
* of time in seconds to delay between each retry. This value defaults to 120 if not
* specified (2 minutes). If specified, this value MUST be greater than or equal to 1.
* The maximum amount of time to delay between retries in seconds. This is the
* ceiling of the exponential backoff. This value defaults to service default
* if not specified. If specified, this value MUST be greater than or equal to 1.
*/
maxDelay?: number;
}
Expand All @@ -44,8 +42,8 @@ export const waiterServiceDefaults = {
/**
* @private
*/
export type ResolvedWaiterOptions<Client> = WaiterOptions<Client> &
Required<Pick<WaiterOptions<Client>, "minDelay" | "maxDelay">>;
export type WaiterOptions<Client> = WaiterConfiguration<Client> &
Required<Pick<WaiterConfiguration<Client>, "minDelay" | "maxDelay">>;

export enum WaiterState {
ABORTED = "ABORTED",
Expand Down

0 comments on commit 25cb359

Please sign in to comment.