[core-lro] No polling allowed when the operation is in a terminal sta…

…te (#24159)

### Packages impacted by this PR
@azure/core-lro

### Issues associated with this PR
[Failures](https://dev.azure.com/azure-sdk/public/_build/results?buildId=2034536&view=logs&j=71cc2e7d-6454-5bf0-71b5-f1eef07a6c23&t=f2857310-5d68-5640-1409-0ba88dcfe076&l=1299) in Azure/autorest.typescript#1586

### Describe the problem that is addressed by this PR
`poll()` always sends the polling request as long as the operation has a location to poll from. However, once the operation is in a terminal state, there is no need to send the polling request.

### 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?
Sending the useless polling request is not the end of the world and won't affect the state of the poller. I don't feel strongly about the change but it is nice to not do unjustified extra work.

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

### Provide a list of related PRs _(if any)_
N/A

### 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)_
- [x] Added a changelog (if necessary)

sdk/core/core-lro/CHANGELOG.md

@@ -1,6 +1,6 @@
 # Release History
 
-## 2.4.1 (Unreleased)
+## 2.5.0 (Unreleased)
 
 ### Features Added
 
@@ -10,6 +10,8 @@
 
 ### Other Changes
 
+- poll() is optimized to no longer send a polling request if the operation is already in a terminal state.
+
 ## 2.4.0 (2022-09-29)
 
 ### Features Added

sdk/core/core-lro/package.json

@@ -2,7 +2,7 @@
   "name": "@azure/core-lro",
   "author": "Microsoft Corporation",
   "sdk-type": "client",
-  "version": "2.4.1",
+  "version": "2.5.0",
   "description": "Isomorphic client library for supporting long-running operations in node.js and browser.",
   "tags": [
     "isomorphic",

sdk/core/core-lro/src/poller/poller.ts

@@ -96,7 +96,7 @@ export function buildCreatePoller<TResponse, TResult, TState extends OperationSt
     type Handler = (state: TState) => void;
     const handlers = new Map<symbol, Handler>();
     const handleProgressEvents = async (): Promise<void> => handlers.forEach((h) => h(state));
-
+    const cancelErrMsg = "Operation was canceled";
     let currentPollIntervalInMs = intervalInMs;
 
     const poller: SimplePollerLike<TState, TResult> = {
@@ -132,28 +132,37 @@ export function buildCreatePoller<TResponse, TResult, TState extends OperationSt
               await poller.poll({ abortSignal });
             }
           }
-          switch (state.status) {
-            case "succeeded": {
-              return poller.getResult() as TResult;
-            }
-            case "canceled": {
-              if (!resolveOnUnsuccessful) throw new Error("Operation was canceled");
-              return poller.getResult() as TResult;
-            }
-            case "failed": {
-              if (!resolveOnUnsuccessful) throw state.error;
-              return poller.getResult() as TResult;
-            }
-            case "notStarted":
-            case "running": {
-              // Unreachable
-              throw new Error(`polling completed without succeeding or failing`);
+          if (resolveOnUnsuccessful) {
+            return poller.getResult() as TResult;
+          } else {
+            switch (state.status) {
+              case "succeeded":
+                return poller.getResult() as TResult;
+              case "canceled":
+                throw new Error(cancelErrMsg);
+              case "failed":
+                throw state.error;
+              case "notStarted":
+              case "running":
+                throw new Error(`Polling completed without succeeding or failing`);
             }
           }
         })().finally(() => {
           resultPromise = undefined;
         })),
       async poll(pollOptions?: { abortSignal?: AbortSignalLike }): Promise<void> {
+        if (resolveOnUnsuccessful) {
+          if (poller.isDone()) return;
+        } else {
+          switch (state.status) {
+            case "succeeded":
+              return;
+            case "canceled":
+              throw new Error(cancelErrMsg);
+            case "failed":
+              throw state.error;
+          }
+        }
         await pollOperation({
           poll,
           state,
@@ -172,11 +181,13 @@ export function buildCreatePoller<TResponse, TResult, TState extends OperationSt
           setErrorAsResult: !resolveOnUnsuccessful,
         });
         await handleProgressEvents();
-        if (state.status === "canceled" && !resolveOnUnsuccessful) {
-          throw new Error("Operation was canceled");
-        }
-        if (state.status === "failed" && !resolveOnUnsuccessful) {
-          throw state.error;
+        if (!resolveOnUnsuccessful) {
+          switch (state.status) {
+            case "canceled":
+              throw new Error(cancelErrMsg);
+            case "failed":
+              throw state.error;
+          }
         }
       },
     };

sdk/core/core-lro/test/lro.spec.ts

@@ -2498,6 +2498,90 @@ matrix(
           assert.equal(pollCount, implName === "createPoller" ? 2 : 1);
         });
       });
+      describe("general behavior", function () {
+        it("poll() doesn't poll after the poller is in a succeed status", async function () {
+          const poller = await createTestPoller({
+            routes: [
+              {
+                method: "PUT",
+                status: 200,
+                body: `{ "properties": { "provisioningState": "Succeeded"}, "id": "100", "name": "foo" }`,
+              },
+            ],
+            throwOnNon2xxResponse,
+          });
+          await poller.poll(); // This will fail if a polling request is sent
+          const result = await poller.pollUntilDone();
+          assert.equal(result.properties?.provisioningState, "Succeeded");
+        });
+        it("poll() doesn't poll after the poller is in a failed status", async function () {
+          const bodyObj = { properties: { provisioningState: "Failed" }, id: "100", name: "foo" };
+          const poller = await createTestPoller({
+            routes: [
+              {
+                method: "PUT",
+                status: 200,
+                body: JSON.stringify(bodyObj),
+              },
+            ],
+            throwOnNon2xxResponse,
+          });
+          await assertDivergentBehavior({
+            op: poller.poll() as any,
+            notThrowing: {
+              result: undefined,
+            },
+            throwing: {
+              messagePattern: /failed/,
+            },
+            throwOnNon2xxResponse,
+          });
+          await assertDivergentBehavior({
+            op: poller.pollUntilDone(),
+            notThrowing: {
+              result: { ...bodyObj, statusCode: 200 },
+            },
+            throwing: {
+              messagePattern: /failed/,
+            },
+            throwOnNon2xxResponse,
+          });
+        });
+        it("poll() doesn't poll after the poller is in a canceled status", async function () {
+          const bodyObj = { properties: { provisioningState: "Canceled" }, id: "100", name: "foo" };
+          const poller = await createTestPoller({
+            routes: [
+              {
+                method: "PUT",
+                status: 200,
+                body: JSON.stringify(bodyObj),
+              },
+            ],
+            throwOnNon2xxResponse,
+          });
+          await assertDivergentBehavior({
+            op: poller.poll() as any,
+            notThrowing: {
+              result: undefined,
+            },
+            throwing: {
+              messagePattern: /canceled/,
+            },
+            throwOnNon2xxResponse,
+          });
+          await assertDivergentBehavior({
+            op: poller.pollUntilDone(),
+            notThrowing: {
+              result: { ...bodyObj, statusCode: 200 },
+            },
+            throwing: {
+              messagePattern: /canceled/,
+            },
+            throwOnNon2xxResponse,
+          });
+          assert.equal(poller.getResult()?.properties?.provisioningState, "Canceled");
+        });
+      });
     });
   }
 );