From 873c6b1bf05b9b5a5042233df9d1caf61678cfec Mon Sep 17 00:00:00 2001 From: Alex Forsyth Date: Wed, 23 Dec 2020 14:51:37 -0500 Subject: [PATCH 1/6] fix: expose min delay and max to the client as optional on waiterconfiguration --- packages/util-waiter/src/createWaiter.spec.ts | 4 ++-- packages/util-waiter/src/poller.spec.ts | 4 ++-- packages/util-waiter/src/poller.ts | 4 ++-- packages/util-waiter/src/utils/validate.spec.ts | 4 ++-- packages/util-waiter/src/utils/validate.ts | 6 ++++-- packages/util-waiter/src/waiter.ts | 6 ++---- 6 files changed, 14 insertions(+), 14 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..65384cc56ee6 100644 --- a/packages/util-waiter/src/utils/validate.spec.ts +++ b/packages/util-waiter/src/utils/validate.spec.ts @@ -1,8 +1,8 @@ -import { ResolvedWaiterOptions } from "../waiter"; +import { WaiterOptions } from "../waiter"; import { validateWaiterOptions } from "./validate"; describe(validateWaiterOptions.name, () => { - let waiterOptions: ResolvedWaiterOptions; + let waiterOptions: WaiterOptions; beforeEach(() => { waiterOptions = { diff --git a/packages/util-waiter/src/utils/validate.ts b/packages/util-waiter/src/utils/validate.ts index 9db7c64abc9d..e37159f29368 100644 --- a/packages/util-waiter/src/utils/validate.ts +++ b/packages/util-waiter/src/utils/validate.ts @@ -1,13 +1,15 @@ -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`; } else if (options.maxWaitTime <= options.minDelay) { throw `WaiterOptions.maxWaitTime [${options.maxWaitTime}] must be greater than WaiterOptions.minDelay [${options.minDelay}] for this waiter`; + } else if (options.maxDelay < options.minDelay) { + throw `WaiterOptions.maxDelay [${options.maxWaitTime}] must be greater than WaiterOptions.minDelay [${options.minDelay}] for this waiter`; } }; diff --git a/packages/util-waiter/src/waiter.ts b/packages/util-waiter/src/waiter.ts index 66575e35383e..c226965d91dc 100644 --- a/packages/util-waiter/src/waiter.ts +++ b/packages/util-waiter/src/waiter.ts @@ -15,9 +15,7 @@ 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 @@ -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", From 09d64c66d2d0e79e062222491351c59ae5171ecc Mon Sep 17 00:00:00 2001 From: Alex Forsyth Date: Wed, 23 Dec 2020 15:00:47 -0500 Subject: [PATCH 2/6] fix: validation error should reference exported WaiterConfiguration type --- .../util-waiter/src/utils/validate.spec.ts | 36 ++++++++++++++++--- packages/util-waiter/src/utils/validate.ts | 6 ++-- 2 files changed, 35 insertions(+), 7 deletions(-) diff --git a/packages/util-waiter/src/utils/validate.spec.ts b/packages/util-waiter/src/utils/validate.spec.ts index 65384cc56ee6..e8bd7e335447 100644 --- a/packages/util-waiter/src/utils/validate.spec.ts +++ b/packages/util-waiter/src/utils/validate.spec.ts @@ -8,11 +8,39 @@ describe(validateWaiterOptions.name, () => { 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).toBe( + "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).toBe("WaiterConfiguration.maxWaitTime must be greater than 0"); done(); } }); @@ -43,7 +71,7 @@ describe(validateWaiterOptions.name, () => { validateWaiterOptions(waiterOptions); } catch (e) { expect(e).toBe( - "WaiterOptions.maxWaitTime [150] must be greater than WaiterOptions.minDelay [200] for this waiter" + "WaiterConfiguration.maxWaitTime [150] must be greater than WaiterConfiguration.minDelay [200] for this waiter" ); done(); } @@ -56,7 +84,7 @@ describe(validateWaiterOptions.name, () => { validateWaiterOptions(waiterOptions); } catch (e) { expect(e).toBe( - "WaiterOptions.maxWaitTime [200] must be greater than WaiterOptions.minDelay [200] for this waiter" + "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 e37159f29368..cd9ad1d787f1 100644 --- a/packages/util-waiter/src/utils/validate.ts +++ b/packages/util-waiter/src/utils/validate.ts @@ -6,10 +6,10 @@ import { WaiterOptions } from "../waiter"; */ export const validateWaiterOptions = (options: WaiterOptions): void => { if (options.maxWaitTime < 1) { - throw `WaiterOptions.maxWaitTime must be greater than 0`; + throw `WaiterConfiguration.maxWaitTime 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 `WaiterConfiguration.maxWaitTime [${options.maxWaitTime}] must be greater than WaiterConfiguration.minDelay [${options.minDelay}] for this waiter`; } else if (options.maxDelay < options.minDelay) { - throw `WaiterOptions.maxDelay [${options.maxWaitTime}] must be greater than WaiterOptions.minDelay [${options.minDelay}] for this waiter`; + throw `WaiterConfiguration.maxDelay [${options.maxDelay}] must be greater than WaiterConfiguration.minDelay [${options.minDelay}] for this waiter`; } }; From 0e282da4e53f2f3cb1825703fd93ab36a973a502 Mon Sep 17 00:00:00 2001 From: Alex Forsyth Date: Wed, 23 Dec 2020 15:12:14 -0500 Subject: [PATCH 3/6] fix: minor update to min/max delay description wording --- packages/util-waiter/src/waiter.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/util-waiter/src/waiter.ts b/packages/util-waiter/src/waiter.ts index c226965d91dc..beb348dd274c 100644 --- a/packages/util-waiter/src/waiter.ts +++ b/packages/util-waiter/src/waiter.ts @@ -18,15 +18,15 @@ export interface 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. + * to service default if not specified. If specified, this value MUST be greater than + * or equal to 1 and less than or equal to maxDelay. */ 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. + * of time in seconds to delay between each retry. This value defaults to service default + * if not specified. If specified, this value MUST be greater than or equal to 1. */ maxDelay?: number; } From 8fa6e08a303a5864413b1ffb3ffda3d3351d9ff8 Mon Sep 17 00:00:00 2001 From: Alex Forsyth Date: Wed, 23 Dec 2020 15:14:18 -0500 Subject: [PATCH 4/6] fix: added additional clarifying language --- packages/util-waiter/src/waiter.ts | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/packages/util-waiter/src/waiter.ts b/packages/util-waiter/src/waiter.ts index beb348dd274c..017c63ce1fff 100644 --- a/packages/util-waiter/src/waiter.ts +++ b/packages/util-waiter/src/waiter.ts @@ -17,15 +17,15 @@ export interface WaiterConfiguration { abortController?: AbortController; /** - * The minimum amount of time to delay between retries in seconds. This value defaults - * to service default if not specified. If specified, this value MUST be greater than + * 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. If specified, this value MUST be greater than * or equal to 1 and less than or equal to maxDelay. */ 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 service default + * The maximum amount of time to delay between retries in seconds. This is the ceiling of the exponential backoff. + * The maximum amount of time in seconds to delay between each retry. This value defaults to service default * if not specified. If specified, this value MUST be greater than or equal to 1. */ maxDelay?: number; From efb0f0ee9b74d34b221cc4634f3a6cb625246416 Mon Sep 17 00:00:00 2001 From: Alex Forsyth Date: Wed, 23 Dec 2020 15:50:29 -0500 Subject: [PATCH 5/6] fix: clarify comments around minDelay and maxDelay --- packages/util-waiter/src/utils/validate.ts | 4 ++++ packages/util-waiter/src/waiter.ts | 10 +++++----- 2 files changed, 9 insertions(+), 5 deletions(-) diff --git a/packages/util-waiter/src/utils/validate.ts b/packages/util-waiter/src/utils/validate.ts index cd9ad1d787f1..49644886e3dd 100644 --- a/packages/util-waiter/src/utils/validate.ts +++ b/packages/util-waiter/src/utils/validate.ts @@ -7,6 +7,10 @@ import { WaiterOptions } from "../waiter"; export const validateWaiterOptions = (options: WaiterOptions): void => { if (options.maxWaitTime < 1) { throw `WaiterConfiguration.maxWaitTime must be greater than 0`; + } else if (options.minDelay < 1) { + throw `WaiterConfiguration.minDelay must be greater than 0`; + } else if (options.maxDelay < 1) { + throw `WaiterConfiguration.maxDelay must be greater than 0`; } else if (options.maxWaitTime <= options.minDelay) { throw `WaiterConfiguration.maxWaitTime [${options.maxWaitTime}] must be greater than WaiterConfiguration.minDelay [${options.minDelay}] for this waiter`; } else if (options.maxDelay < options.minDelay) { diff --git a/packages/util-waiter/src/waiter.ts b/packages/util-waiter/src/waiter.ts index 017c63ce1fff..7383ba1e12fb 100644 --- a/packages/util-waiter/src/waiter.ts +++ b/packages/util-waiter/src/waiter.ts @@ -17,15 +17,15 @@ export interface WaiterConfiguration { abortController?: AbortController; /** - * 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. 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. This is the ceiling of the exponential backoff. - * The maximum amount of time in seconds to delay between each retry. This value defaults to service default + * 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; From ca7b5dba1ae91d0b2f3fa4103d0bc73b268b5fcd Mon Sep 17 00:00:00 2001 From: Alex Forsyth Date: Wed, 23 Dec 2020 17:56:58 -0500 Subject: [PATCH 6/6] fix: waiter validation throws a proper error object --- packages/util-waiter/src/utils/validate.spec.ts | 14 +++++++------- packages/util-waiter/src/utils/validate.ts | 14 +++++++++----- 2 files changed, 16 insertions(+), 12 deletions(-) diff --git a/packages/util-waiter/src/utils/validate.spec.ts b/packages/util-waiter/src/utils/validate.spec.ts index e8bd7e335447..bff31c85656b 100644 --- a/packages/util-waiter/src/utils/validate.spec.ts +++ b/packages/util-waiter/src/utils/validate.spec.ts @@ -34,8 +34,8 @@ describe(validateWaiterOptions.name, () => { validateWaiterOptions(waiterOptions); expect(1).toBe("SHOULD NOT GET HERE"); } catch (e) { - expect(e).toBe( - "WaiterConfiguration.maxDelay [120] must be greater than WaiterConfiguration.minDelay [200] for this waiter" + expect(e.toString()).toBe( + "Error: WaiterConfiguration.maxDelay [120] must be greater than WaiterConfiguration.minDelay [200] for this waiter" ); done(); } @@ -59,7 +59,7 @@ describe(validateWaiterOptions.name, () => { try { validateWaiterOptions(waiterOptions); } catch (e) { - expect(e).toBe("WaiterConfiguration.maxWaitTime must be greater than 0"); + expect(e.toString()).toBe("Error: WaiterConfiguration.maxWaitTime must be greater than 0"); done(); } }); @@ -70,8 +70,8 @@ describe(validateWaiterOptions.name, () => { try { validateWaiterOptions(waiterOptions); } catch (e) { - expect(e).toBe( - "WaiterConfiguration.maxWaitTime [150] must be greater than WaiterConfiguration.minDelay [200] for this waiter" + expect(e.toString()).toBe( + "Error: WaiterConfiguration.maxWaitTime [150] must be greater than WaiterConfiguration.minDelay [200] for this waiter" ); done(); } @@ -83,8 +83,8 @@ describe(validateWaiterOptions.name, () => { try { validateWaiterOptions(waiterOptions); } catch (e) { - expect(e).toBe( - "WaiterConfiguration.maxWaitTime [200] must be greater than WaiterConfiguration.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 49644886e3dd..c186d67fe38c 100644 --- a/packages/util-waiter/src/utils/validate.ts +++ b/packages/util-waiter/src/utils/validate.ts @@ -6,14 +6,18 @@ import { WaiterOptions } from "../waiter"; */ export const validateWaiterOptions = (options: WaiterOptions): void => { if (options.maxWaitTime < 1) { - throw `WaiterConfiguration.maxWaitTime must be greater than 0`; + throw new Error(`WaiterConfiguration.maxWaitTime must be greater than 0`); } else if (options.minDelay < 1) { - throw `WaiterConfiguration.minDelay must be greater than 0`; + throw new Error(`WaiterConfiguration.minDelay must be greater than 0`); } else if (options.maxDelay < 1) { - throw `WaiterConfiguration.maxDelay must be greater than 0`; + throw new Error(`WaiterConfiguration.maxDelay must be greater than 0`); } else if (options.maxWaitTime <= options.minDelay) { - throw `WaiterConfiguration.maxWaitTime [${options.maxWaitTime}] must be greater than WaiterConfiguration.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 `WaiterConfiguration.maxDelay [${options.maxDelay}] must be greater than WaiterConfiguration.minDelay [${options.minDelay}] for this waiter`; + throw new Error( + `WaiterConfiguration.maxDelay [${options.maxDelay}] must be greater than WaiterConfiguration.minDelay [${options.minDelay}] for this waiter` + ); } };