sdk - rework LRO and ProvisioningState pollers to match ARM spec for terminal statuses

sdk/client/client.go

@@ -720,7 +720,7 @@ func (c *Client) retryableClient(ctx context.Context, checkRetry retryablehttp.C
 	// In case the context has deadline defined, adjust the retry count to a value
 	// that the total time spent for retrying is right before the deadline exceeded.
 	if deadline, ok := ctx.Deadline(); ok {
-		r.RetryMax = safeRetryNumber(deadline.Sub(time.Now()))
+		r.RetryMax = safeRetryNumber(time.Until(deadline))
 	}
 
 	tlsConfig := tls.Config{

sdk/client/msgraph/client.go

@@ -32,7 +32,7 @@ type Client struct {
 	apiVersion ApiVersion
 
 	// tenantId is the tenant ID to use in requests
-	tenantId string
+	tenantId string // nolint: unused
 }
 
 func NewClient(api environments.Api, serviceName string, apiVersion ApiVersion) (*Client, error) {
@@ -96,7 +96,7 @@ func (c *Client) NewRequest(ctx context.Context, input client.RequestOptions) (*
 	}
 
 	req.URL.RawQuery = query.Encode()
-	//req.RetryFunc = client.RequestRetryAny(defaultRetryFunctions...)
+	// req.RetryFunc = client.RequestRetryAny(defaultRetryFunctions...)
 	req.ValidStatusCodes = input.ExpectedStatusCodes
 
 	return req, nil

sdk/client/pollers/interface.go

@@ -73,7 +73,7 @@ func (e PollingCancelledError) Error() string {
 		return fmt.Sprintf("polling was cancelled: %+v", e.Message)
 	}
 
-	return fmt.Sprintf("polling was cancelled")
+	return "polling was cancelled"
 }
 
 var _ error = PollingDroppedConnectionError{}
@@ -89,7 +89,7 @@ func (e PollingDroppedConnectionError) Error() string {
 		return fmt.Sprintf("experienced a dropped connection when polling: %+v", e.Message)
 	}
 
-	return fmt.Sprintf("experienced a dropped connection when polling")
+	return "experienced a dropped connection when polling"
 }
 
 var _ error = PollingFailedError{}
@@ -108,5 +108,5 @@ func (e PollingFailedError) Error() string {
 		return fmt.Sprintf("polling failed: %+v", e.Message)
 	}
 
-	return fmt.Sprintf("polling failed")
+	return "polling failed"
 }

sdk/client/pollers/poller.go

@@ -114,7 +114,8 @@ func (p *Poller) PollUntilDone(ctx context.Context) error {
 				retryDuration = p.latestResponse.PollInterval
 			}
 			endTime := time.Now().Add(retryDuration)
-			select {
+
+			select { // nolint: gosimple
 			case <-time.After(time.Until(endTime)):
 				{
 					break
@@ -160,24 +161,20 @@ func (p *Poller) PollUntilDone(ctx context.Context) error {
 				case PollingStatusCancelled:
 					p.latestError = fmt.Errorf("internal-error: a polling status of `Cancelled` should be surfaced as a PollingCancelledError")
 					done = true
-					break
 
 				case PollingStatusFailed:
 					p.latestError = fmt.Errorf("internal-error: a polling status of `Failed` should be surfaced as a PollingFailedError")
 					done = true
-					break
 
 				case PollingStatusInProgress:
 					continue
 
 				case PollingStatusSucceeded:
 					done = true
-					break
 
 				default:
 					p.latestError = fmt.Errorf("internal-error: unimplemented polling status %q", string(response.Status))
 					done = true
-					break
 				}
 
 				if done {

sdk/client/resourcemanager/errors.go

@@ -43,7 +43,7 @@ API Response:
 
 // parseErrorFromApiResponse parses the error from the API Response
 // into an Error type, which allows for better surfacing of errors
-func parseErrorFromApiResponse(response http.Response) (*Error, error) {
+func parseErrorFromApiResponse(response *http.Response) (*Error, error) {
 	respBody, err := io.ReadAll(response.Body)
 	if err != nil {
 		return nil, fmt.Errorf("parsing response body: %+v", err)
@@ -125,7 +125,6 @@ func parseErrorFromApiResponse(response http.Response) (*Error, error) {
 			if v.ActivityId != "" {
 				activityId = v.ActivityId
 			}
-			break
 		}
 		return &Error{
 			ActivityId:   activityId,

sdk/client/resourcemanager/errors_test.go

@@ -24,7 +24,7 @@ func TestParseErrorFromApiResponse_LongRunningOperation(t *testing.T) {
 		Message:      "The specified name is already in use.",
 		Status:       "Failed",
 	}
-	actual, err := parseErrorFromApiResponse(input)
+	actual, err := parseErrorFromApiResponse(&input)
 	if err != nil {
 		t.Fatalf("parsing error from api response: %+v", err)
 	}
@@ -49,7 +49,7 @@ func TestParseErrorFromApiResponse_ResourceManagerType1(t *testing.T) {
 		Message:      "Cannot update the site 'func-tfbug-b78d100425' because it uses AlwaysOn feature which is not allowed in the target compute mode.",
 		Status:       "Unknown",
 	}
-	actual, err := parseErrorFromApiResponse(input)
+	actual, err := parseErrorFromApiResponse(&input)
 	if err != nil {
 		t.Fatalf("parsing error from api response: %+v", err)
 	}
@@ -74,7 +74,7 @@ func TestParseErrorFromApiResponse_ResourceManagerType2(t *testing.T) {
 		Message:      "Failed to start operation. Verify input and try operation again.\nFailed to start operation. Verify input and try operation again.\nPossible Causes: \"Invalid parameters were specified.\"\nRecommended Action: \"Verify the input and try again.\"",
 		Status:       "BadRequest",
 	}
-	actual, err := parseErrorFromApiResponse(input)
+	actual, err := parseErrorFromApiResponse(&input)
 	if err != nil {
 		t.Fatalf("parsing error from api response: %+v", err)
 	}
@@ -98,7 +98,7 @@ func TestParseErrorFromApiResponse_ResourceManagerType2Alt(t *testing.T) {
 		Message:      "The request is not valid.\nResource name is not valid. It must be between 3 and 26 characters and can only include alphanumeric characters and hyphens.",
 		Status:       "ValidationError",
 	}
-	actual, err := parseErrorFromApiResponse(input)
+	actual, err := parseErrorFromApiResponse(&input)
 	if err != nil {
 		t.Fatalf("parsing error from api response: %+v", err)
 	}
@@ -123,7 +123,7 @@ func TestParseErrorFromApiResponse_ResourceManagerType3(t *testing.T) {
 		Message:      "Outbound Trust/Untrust certificate can not be deleted from rule stack SUBSCRIPTION~XXXXXXXX-XXXX-XXXX-XXXXXXXXXXXX~RG~acctestRG-PAN-230725055238618023~STACK~testAcc-palrs-230725055238618023 as associated rule list(s) already has SSL outbound inspection set with status code BadRequest",
 		Status:       "Failed",
 	}
-	actual, err := parseErrorFromApiResponse(input)
+	actual, err := parseErrorFromApiResponse(&input)
 	if err != nil {
 		t.Fatalf("parsing error from api response: %+v", err)
 	}
@@ -147,7 +147,7 @@ func TestParseErrorFromApiResponse_EmptyResponseShouldOutputHttpResponseAnyway(t
 		Message:      "Couldn't parse Azure API Response into a friendly error - please see the original HTTP Response for more details (and file a bug so we can fix this!).",
 		Status:       "Unknown",
 	}
-	actual, err := parseErrorFromApiResponse(input)
+	actual, err := parseErrorFromApiResponse(&input)
 	if err != nil {
 		t.Fatalf("parsing error from api response: %+v", err)
 	}
@@ -172,7 +172,7 @@ func TestParseErrorFromApiResponse_UnexpectedHtmlShouldOutputHttpResponseAnyway(
 		Message:      "Couldn't parse Azure API Response into a friendly error - please see the original HTTP Response for more details (and file a bug so we can fix this!).",
 		Status:       "Unknown",
 	}
-	actual, err := parseErrorFromApiResponse(input)
+	actual, err := parseErrorFromApiResponse(&input)
 	if err != nil {
 		t.Fatalf("parsing error from api response: %+v", err)
 	}
@@ -197,7 +197,7 @@ func TestParseErrorFromApiResponse_UnexpectedLiteralStringShouldOutputHttpRespon
 		Message:      "Couldn't parse Azure API Response into a friendly error - please see the original HTTP Response for more details (and file a bug so we can fix this!).",
 		Status:       "Unknown",
 	}
-	actual, err := parseErrorFromApiResponse(input)
+	actual, err := parseErrorFromApiResponse(&input)
 	if err != nil {
 		t.Fatalf("parsing error from api response: %+v", err)
 	}
@@ -222,7 +222,7 @@ func TestParseErrorFromApiResponse_UnexpectedLiteralQuotedStringShouldOutputHttp
 		Message:      "Couldn't parse Azure API Response into a friendly error - please see the original HTTP Response for more details (and file a bug so we can fix this!).",
 		Status:       "Unknown",
 	}
-	actual, err := parseErrorFromApiResponse(input)
+	actual, err := parseErrorFromApiResponse(&input)
 	if err != nil {
 		t.Fatalf("parsing error from api response: %+v", err)
 	}
@@ -246,7 +246,7 @@ func TestParseErrorFromApiResponse_UnexpectedShouldOutputHttpResponseAnyway(t *t
 		Message:      "Couldn't parse Azure API Response into a friendly error - please see the original HTTP Response for more details (and file a bug so we can fix this!).",
 		Status:       "Unknown",
 	}
-	actual, err := parseErrorFromApiResponse(input)
+	actual, err := parseErrorFromApiResponse(&input)
 	if err != nil {
 		t.Fatalf("parsing error from api response: %+v", err)
 	}

sdk/client/resourcemanager/poller_helpers_test.go

@@ -44,6 +44,12 @@ func responseWithHttpStatusCode(input int) expectedResponse {
 	}
 }
 
+func responseWithNoTerminalState() expectedResponse {
+	return expectedResponse{
+		status: pointer.ToEnum[status](""),
+	}
+}
+
 func dropConnection(t *testing.T, w http.ResponseWriter) {
 	httpHijacker, ok := w.(http.Hijacker)
 	if !ok {

sdk/client/resourcemanager/poller_lro.go

@@ -34,7 +34,7 @@ type longRunningOperationPoller struct {
 }
 
 func pollingUriForLongRunningOperation(resp *client.Response) string {
-	pollingUrl := resp.Header.Get(http.CanonicalHeaderKey("Azure-AsyncOperation"))
+	pollingUrl := resp.Header.Get("Azure-AsyncOperation")
 	if pollingUrl == "" {
 		pollingUrl = resp.Header.Get("Location")
 	}
@@ -174,23 +174,8 @@ func (p *longRunningOperationPoller) Poll(ctx context.Context) (result *pollers.
 			return nil, fmt.Errorf("internal-error: polling support for the Content-Type %q was not implemented: %+v", contentType, err)
 		}
 
-		if op.Properties.ProvisioningState == "" && op.Status == "" {
-			return nil, fmt.Errorf("expected either `provisioningState` or `status` to be returned from the LRO API but both were empty")
-		}
-
-		for k, v := range longRunningOperationCustomStatuses {
-			if strings.EqualFold(string(op.Properties.ProvisioningState), string(k)) {
-				result.Status = v
-				break
-			}
-			if strings.EqualFold(string(op.Status), string(k)) {
-				result.Status = v
-				break
-			}
-		}
-
-		if result.Status == pollers.PollingStatusFailed {
-			lroError, parseError := parseErrorFromApiResponse(*result.HttpResponse.Response)
+		if result.Status == pollers.PollingStatusFailed || op.Status == statusFailed || op.Properties.ProvisioningState == statusFailed {
+			lroError, parseError := parseErrorFromApiResponse(result.HttpResponse.Response)
 			if parseError != nil {
 				return nil, parseError
 			}
@@ -201,8 +186,8 @@ func (p *longRunningOperationPoller) Poll(ctx context.Context) (result *pollers.
 			}
 		}
 
-		if result.Status == pollers.PollingStatusCancelled {
-			lroError, parseError := parseErrorFromApiResponse(*result.HttpResponse.Response)
+		if result.Status == pollers.PollingStatusCancelled || op.Status == statusCanceled || op.Properties.ProvisioningState == statusCanceled {
+			lroError, parseError := parseErrorFromApiResponse(result.HttpResponse.Response)
 			if parseError != nil {
 				return nil, parseError
 			}
@@ -213,12 +198,16 @@ func (p *longRunningOperationPoller) Poll(ctx context.Context) (result *pollers.
 			}
 		}
 
-		if result.Status == "" {
-			err = fmt.Errorf("`result.Status` was nil/empty - `op.Status` was %q / `op.Properties.ProvisioningState` was %q", string(op.Status), string(op.Properties.ProvisioningState))
+		if result.Status == pollers.PollingStatusSucceeded || op.Status == statusSucceeded || op.Properties.ProvisioningState == statusSucceeded {
+			result.Status = pollers.PollingStatusSucceeded
+			return
 		}
+
+		// If we don't have a terminal status anywhere, we must assume we're still in progress
+		result.Status = pollers.PollingStatusInProgress
 	}
 
-	return
+	return result, nil
 }
 
 type operationResult struct {
@@ -249,3 +238,11 @@ const (
 	statusInProgress status = "InProgress"
 	statusSucceeded  status = "Succeeded"
 )
+
+func statusIsTerminal(s status) bool {
+	switch s {
+	case statusCanceled, statusCancelled, statusFailed, statusSucceeded, statusInProgress:
+		return true
+	}
+	return false
+}