From 25cb359e69966c549eb505956c2aeee809819245 Mon Sep 17 00:00:00 2001 From: Alexander Forsyth Date: Tue, 5 Jan 2021 13:01:48 -0500 Subject: [PATCH] fix(util-waiter): expose minDelay and maxDelay for waiters (#1839) * 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 --- packages/util-waiter/src/createWaiter.spec.ts | 4 +- packages/util-waiter/src/poller.spec.ts | 4 +- packages/util-waiter/src/poller.ts | 4 +- .../util-waiter/src/utils/validate.spec.ts | 44 +++++++++++++++---- packages/util-waiter/src/utils/validate.ts | 18 ++++++-- packages/util-waiter/src/waiter.ts | 18 ++++---- 6 files changed, 64 insertions(+), 28 deletions(-) diff --git a/packages/util-waiter/src/createWaiter.spec.ts b/packages/util-waiter/src/createWaiter.spec.ts index 4ab3f26a12d4..309402c74d5b 100644 --- a/packages/util-waiter/src/createWaiter.spec.ts +++ b/packages/util-waiter/src/createWaiter.spec.ts @@ -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", () => ({ @@ -22,7 +22,7 @@ describe("createWaiter", () => { maxDelay: 120, maxWaitTime: 9999, client: "client", - } as ResolvedWaiterOptions; + } as WaiterOptions; const input = "input"; const abortedState = { diff --git a/packages/util-waiter/src/poller.spec.ts b/packages/util-waiter/src/poller.spec.ts index 1e4942342629..366536104583 100644 --- a/packages/util-waiter/src/poller.spec.ts +++ b/packages/util-waiter/src/poller.spec.ts @@ -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"); @@ -12,7 +12,7 @@ describe(runPolling.name, () => { maxDelay: 30, maxWaitTime: 99999, client: "mockClient", - } as ResolvedWaiterOptions; + } as WaiterOptions; const input = "mockInput"; const abortedState = { state: WaiterState.ABORTED, diff --git a/packages/util-waiter/src/poller.ts b/packages/util-waiter/src/poller.ts index 33b9e42e87b4..ed1997281bc6 100644 --- a/packages/util-waiter/src/poller.ts +++ b/packages/util-waiter/src/poller.ts @@ -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 @@ -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 ( - { minDelay, maxDelay, maxWaitTime, abortController, client }: ResolvedWaiterOptions, + { minDelay, maxDelay, maxWaitTime, abortController, client }: WaiterOptions, input: Input, acceptorChecks: (client: Client, input: Input) => Promise ): Promise => { diff --git a/packages/util-waiter/src/utils/validate.spec.ts b/packages/util-waiter/src/utils/validate.spec.ts index e9472785c2f7..bff31c85656b 100644 --- a/packages/util-waiter/src/utils/validate.spec.ts +++ b/packages/util-waiter/src/utils/validate.spec.ts @@ -1,18 +1,46 @@ -import { ResolvedWaiterOptions } from "../waiter"; +import { WaiterOptions } from "../waiter"; import { validateWaiterOptions } from "./validate"; describe(validateWaiterOptions.name, () => { - let waiterOptions: ResolvedWaiterOptions; + let waiterOptions: WaiterOptions; 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; @@ -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(); } }); @@ -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(); } @@ -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" ); } }); diff --git a/packages/util-waiter/src/utils/validate.ts b/packages/util-waiter/src/utils/validate.ts index 9db7c64abc9d..c186d67fe38c 100644 --- a/packages/util-waiter/src/utils/validate.ts +++ b/packages/util-waiter/src/utils/validate.ts @@ -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 = (options: ResolvedWaiterOptions): void => { +export const validateWaiterOptions = (options: WaiterOptions): 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` + ); } }; diff --git a/packages/util-waiter/src/waiter.ts b/packages/util-waiter/src/waiter.ts index 66575e35383e..7383ba1e12fb 100644 --- a/packages/util-waiter/src/waiter.ts +++ b/packages/util-waiter/src/waiter.ts @@ -15,20 +15,18 @@ export interface WaiterConfiguration { * Abort controller. Used for ending the waiter early. */ abortController?: AbortController; -} -export interface WaiterOptions extends WaiterConfiguration { /** - * 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; } @@ -44,8 +42,8 @@ export const waiterServiceDefaults = { /** * @private */ -export type ResolvedWaiterOptions = WaiterOptions & - Required, "minDelay" | "maxDelay">>; +export type WaiterOptions = WaiterConfiguration & + Required, "minDelay" | "maxDelay">>; export enum WaiterState { ABORTED = "ABORTED",