Waiter doesn't error on Timeout

[trivikr]

Describe the bug
Waiter doesn't error on timeout.
It's tested by waiting for a bucket which doesn't exist.

SDK version number
3.3.0

Is the issue in the browser/Node.js/ReactNative?
All

Details of the browser/Node.js/ReactNative version
N/A

To Reproduce (observed behavior)

v2 Code

const AWS = require("aws-sdk");

(async () => {
  const region = "us-west-2";
  const client = new AWS.S3({ region });

  const Bucket = "test-waiters-bucket-does-not-exist";
  await client
    .waitFor("bucketExists", { Bucket, $waiter: { delay: 2, maxAttempts: 3 } })
    .promise();
})();

v2 Output

(node:14350) UnhandledPromiseRejectionWarning: NotFound: Resource is not in the state bucketExists
...

v3 Code

import { S3, waitForBucketExists } from "@aws-sdk/client-s3";

(async () => {
  const region = "us-west-2";
  const client = new S3({ region });

  const Bucket = "test-waiters-bucket-does-not-exist";
  await waitForBucketExists({ client, maxWaitTime: 30 }, { Bucket });
})();

v3 Output

// No output, it should error on timeout.

Expected behavior
Waiter should error on timeout.

[trivikr]

This is a bug as per Step 2 of Waiter workflow:

While the total execution time of a waiter is less than the allowed time, waiter implementations perform the following steps:

1. Call the operation the :ref:smithy.waiters#waitable-trait is attached to using user-provided input for the operation. Any errors that can be encountered by the operation must be caught so that they can be inspected.
2. If the total time of the waiter exceeds the allowed time, the waiter SHOULD attempt to cancel any in-progress requests and MUST transition to a to a terminal failure state.

[alexforsyth]

I believe this spec says the waiter should transition to a failure state. It does return

{ state: Timeout }

import { S3, waitForBucketExists } from "@aws-sdk/client-s3";

(async () => {
  const region = "us-west-2";
  const client = new S3({ region });

  const Bucket = "test-waiters-bucket-does-not-exist";
  const res = await waitForBucketExists({ client, maxWaitTime: 30 }, { Bucket });
  console.log(res)
})();

```

[alexforsyth]

If we want to change the functionality thats a reasonable discussion but this is intended.
The Waiter function goes something like:

export const waitForBlahBalh = async ( params: configBlah, input: inputBlah): Promise<WaiterResult> ...

Where WaiterResult is:

export declare type WaiterResult = {
    state: WaiterState;
};
export declare enum WaiterState {
    ABORTED = "ABORTED",
    FAILURE = "FAILURE",
    SUCCESS = "SUCCESS",
    RETRY = "RETRY",
    TIMEOUT = "TIMEOUT"
}

[trivikr]

If we want to change the functionality thats a reasonable discussion but this is intended.

I think waiter should throw error if state is anything other than "SUCCESS" or "RETRY".
Note: The RETRY state is internal and not returned to the callee.

This would indicate to the callee that waiting was unsuccessful in the following cases, and enforce user to add catch block :

• ABORTED: Waiting for aborted, the resource may not be in desired state.
• FAILURE: Waiting failed, the resource may not be in desired state.
• TIMEOUT: Waiting timed out, the resource may not be in desired state.

[trivikr]

Decision: We'll create a wrapper named waitUntilOperationName which would throw error on non-SUCCESS cases. The existing API waitForOperationName will be marked as @deprecated.

The naming waitUntil is used, as it's used in Ruby, PHP, Go and Java SDKs.

[trivikr]

Closing, as waitUntilOperationName errors on timeout.

Code

import { S3, waitUntilBucketExists } from "@aws-sdk/client-s3";

const region = "us-west-2";
const client = new S3({ region });

const Bucket = "test-waiters-bucket-does-not-exist";
await waitUntilBucketExists({ client, maxWaitTime: 30 }, { Bucket });

Output

$ time node waitUntilBucketExists.mjs
/Users/trivikr/workspace/test/node_modules/@aws-sdk/util-waiter/dist/cjs/waiter.js:33
        const timeoutError = new Error(`${JSON.stringify({
                             ^

Error [TimeoutError]: {"state":"TIMEOUT","reason":"Waiter has timed out"}
    at Object.checkExceptions (/Users/trivikr/workspace/test/node_modules/@aws-sdk/util-waiter/dist/cjs/waiter.js:33:30)
    at waitUntilBucketExists (/Users/trivikr/workspace/test/node_modules/@aws-sdk/client-s3/dist/cjs/waiters/waitForBucketExists.js:38:26)
    at processTicksAndRejections (internal/process/task_queues.js:95:5)
    at async file:///Users/trivikr/workspace/test/waitUntilBucketExists.mjs:7:1
node waitUntilBucketExists.mjs  0.38s user 0.08s system 1% cpu 29.603 total

[github-actions]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs and link to relevant comments in this thread.