From 5f5e285e87b7359a34efe75d45efe63bf0b881fa Mon Sep 17 00:00:00 2001 From: jackofallops Date: Tue, 8 Jul 2025 11:55:27 +0200 Subject: [PATCH 1/3] remove brittle custom statuses and rely on API defined terminal statuses --- sdk/client/resourcemanager/poller_lro.go | 35 ++-- .../resourcemanager/poller_lro_statuses.go | 115 ------------- sdk/client/resourcemanager/poller_lro_test.go | 157 +++++++++--------- 3 files changed, 92 insertions(+), 215 deletions(-) delete mode 100644 sdk/client/resourcemanager/poller_lro_statuses.go diff --git a/sdk/client/resourcemanager/poller_lro.go b/sdk/client/resourcemanager/poller_lro.go index 527257c3514..7d1e5d814f0 100644 --- a/sdk/client/resourcemanager/poller_lro.go +++ b/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 { diff --git a/sdk/client/resourcemanager/poller_lro_statuses.go b/sdk/client/resourcemanager/poller_lro_statuses.go deleted file mode 100644 index a74a8e16a79..00000000000 --- a/sdk/client/resourcemanager/poller_lro_statuses.go +++ /dev/null @@ -1,115 +0,0 @@ -// Copyright (c) HashiCorp, Inc. -// SPDX-License-Identifier: MPL-2.0 - -package resourcemanager - -import "github.com/hashicorp/go-azure-sdk/sdk/client/pollers" - -var longRunningOperationCustomStatuses = map[status]pollers.PollingStatus{ - // Expected/Documented Terminal Statuses - statusCanceled: pollers.PollingStatusCancelled, - statusCancelled: pollers.PollingStatusCancelled, - statusFailed: pollers.PollingStatusFailed, - statusInProgress: pollers.PollingStatusInProgress, - statusSucceeded: pollers.PollingStatusSucceeded, - - // Unexpected Statuses below this point: - - // whilst the standard set above should be sufficient, some APIs differ from the spec and should be documented below: - // Dashboard@2022-08-01 returns `Accepted` rather than `InProgress` during creation - "Accepted": pollers.PollingStatusInProgress, - - // EventGrid@2022-06-15 returns `Active` rather than `InProgress` during creation - "Active": pollers.PollingStatusInProgress, - - // NetAppVolumeReplication @ 2023-05-01 returns `AuthorizeReplication` during authorizing replication - "AuthorizeReplication": pollers.PollingStatusInProgress, - - // VMWare @ 2022-05-01 returns `Building` rather than `InProgress` during creation - "Building": pollers.PollingStatusInProgress, - - // NetAppVolumeReplication @ 2023-05-01 returns `BreakReplication` during breaking replication - "BreakReplication": pollers.PollingStatusInProgress, - - // Mysql @ 2022-01-01 returns `CancelInProgress` during Update - "CancelInProgress": pollers.PollingStatusInProgress, - - // CostManagement@2021-10-01 returns `Completed` rather than `Succeeded`: https://github.com/Azure/azure-sdk-for-go/issues/20342 - "Completed": pollers.PollingStatusSucceeded, - - // StreamAnalytics@2020-03-01 introduced `ConfiguringNetworking` as undocumented granular statuses on 2024-04-09 - "ConfiguringNetworking": pollers.PollingStatusInProgress, - - // ServiceFabricManaged @ 2021-05-01 (NodeTypes CreateOrUpdate) returns `Created` rather than `InProgress` during Creation - "Created": pollers.PollingStatusInProgress, - - // ContainerRegistry@2019-06-01-preview returns `Creating` rather than `InProgress` during creation - "Creating": pollers.PollingStatusInProgress, - - // StreamAnalytics@2020-03-01 introduced `CreatingVirtualMachines` as undocumented granular statuses on 2024-04-09 - "CreatingVirtualMachines": pollers.PollingStatusInProgress, - - // CosmosDB @ 2023-04-15 returns `Dequeued` rather than `InProgress` during creation/update - "Dequeued": pollers.PollingStatusInProgress, - - // StorageSync@2020-03-01 returns `finishNewStorageSyncService` rather than `InProgress` during creation/update (https://github.com/hashicorp/go-azure-sdk/issues/565) - "finishNewStorageSyncService": pollers.PollingStatusInProgress, - - // StorageSync@2020-03-01 returns `newManagedIdentityCredentialStep` rather than `InProgress` during creation/update (https://github.com/hashicorp/go-azure-sdk/issues/565) - "newManagedIdentityCredentialStep": pollers.PollingStatusInProgress, - - // StorageSync@2020-03-01 returns `newPrivateDnsEntries` rather than `InProgress` during creation/update (https://github.com/hashicorp/go-azure-sdk/issues/565) - "newPrivateDnsEntries": pollers.PollingStatusInProgress, - - // StorageSync@2020-03-01 (CloudEndpoints) returns `newReplicaGroup` rather than `InProgress` during creation/update (https://github.com/hashicorp/go-azure-sdk/issues/565) - "newReplicaGroup": pollers.PollingStatusInProgress, - - // StorageSync@2020-03-01 returns `notifySyncServicePartition` rather than `InProgress` during creation - // polling after StorageSyncServicesCreate: `result.Status` was nil/empty - `op.Status` was "notifySyncServicePartition" / `op.Properties.ProvisioningState` was "" - "notifySyncServicePartition": pollers.PollingStatusInProgress, - - // NetApp @ 2023-05-01 (Volume Update) returns `Patching` during Update - "Patching": pollers.PollingStatusInProgress, - - // AnalysisServices @ 2017-08-01 (Servers Suspend) returns `Pausing` during update - "Pausing": pollers.PollingStatusInProgress, - - // ContainerInstance @ 2023-05-01 returns `Pending` during creation/update - "Pending": pollers.PollingStatusInProgress, - - // SAPVirtualInstance @ 2023-04-01 returns `Preparing System Configuration` during Creation - "Preparing System Configuration": pollers.PollingStatusInProgress, - - // AnalysisServices @ 2017-08-01 (Servers) returns `Provisioning` during Creation - "Provisioning": pollers.PollingStatusInProgress, - - // Resources @ 2020-10-01 (DeploymentScripts) returns `ProvisioningResources` during Creation - "ProvisioningResources": pollers.PollingStatusInProgress, - - // AnalysisServices @ 2017-08-01 (Servers Resume) returns `Resuming` during Update - "Resuming": pollers.PollingStatusInProgress, - - // HealthcareApis @ 2022-12-01 returns `Requested` during Creation - "Requested": pollers.PollingStatusInProgress, - - // SignalR@2022-02-01 returns `Running` rather than `InProgress` during creation - "Running": pollers.PollingStatusInProgress, - - // AnalysisServices @ 2017-08-01 (Servers Suspend) returns `Scaling` during Update - "Scaling": pollers.PollingStatusInProgress, - - // StreamAnalytics@2020-03-01 introduced `SettingUpStreamingRuntime` as undocumented granular statuses on 2024-04-09 - "SettingUpStreamingRuntime": pollers.PollingStatusInProgress, - - // KubernetesConfiguration@2022-11-01 returns `Updating` rather than `InProgress` during update - "Updating": pollers.PollingStatusInProgress, - - // HealthBot @ 2022-08-08 (HealthBots CreateOrUpdate) returns `Working` during Creation - "Working": pollers.PollingStatusInProgress, - - // StorageSync@2020-03-01 returns `validateInput` rather than `InProgress` during creation/update (https://github.com/hashicorp/go-azure-sdk/issues/565) - "validateInput": pollers.PollingStatusInProgress, - - // EventGrid @ 2022-06-15 returns `AwaitingManualAction` while waiting for manual validation of a webhook (https://github.com/hashicorp/terraform-provider-azurerm/issues/25689) - "AwaitingManualAction": pollers.PollingStatusInProgress, -} diff --git a/sdk/client/resourcemanager/poller_lro_test.go b/sdk/client/resourcemanager/poller_lro_test.go index cb93a85ff4d..aaec8d6162b 100644 --- a/sdk/client/resourcemanager/poller_lro_test.go +++ b/sdk/client/resourcemanager/poller_lro_test.go @@ -5,7 +5,6 @@ package resourcemanager import ( "context" - "fmt" "net/http" "net/http/httptest" "testing" @@ -14,82 +13,6 @@ import ( "github.com/hashicorp/go-azure-sdk/sdk/client/pollers" ) -func TestPollerLRO_InProvisioningState_TerminalStates(t *testing.T) { - // This test ensures that each of the Custom Statuses works in the provisioningState field - ctx := context.TODO() - for state, expected := range longRunningOperationCustomStatuses { - t.Run(fmt.Sprintf("%s/%s", string(state), string(expected)), func(t *testing.T) { - helpers := newLongRunningOperationsEndpoint([]expectedResponse{ - responseWithStatusInProvisioningState(state), - }) - server := httptest.NewServer(http.HandlerFunc(helpers.endpoint(t))) - defer server.Close() - - response := &client.Response{ - Response: helpers.response(), - } - client := client.NewClient(server.URL, "MyService", "2020-02-01") - poller, err := longRunningOperationPollerFromResponse(response, client) - if err != nil { - t.Fatal(err.Error()) - } - - t.Logf("Polling..") - result, err := poller.Poll(ctx) - if err != nil { - if expected == pollers.PollingStatusCancelled || expected == pollers.PollingStatusFailed { - return - } - - t.Fatal(err.Error()) - } - if result.Status != expected { - t.Fatalf("expected status to be %q but got %q", expected, result.Status) - } - // sanity-checking - helpers.assertCalled(t, 1) - }) - } -} - -func TestPollerLRO_InStatus_TerminalStates(t *testing.T) { - // This test ensures that each of the Custom Statuses works in the status field - ctx := context.TODO() - for state, expected := range longRunningOperationCustomStatuses { - t.Run(fmt.Sprintf("%s/%s", string(state), string(expected)), func(t *testing.T) { - helpers := newLongRunningOperationsEndpoint([]expectedResponse{ - responseWithStatusInStatusField(state), - }) - server := httptest.NewServer(http.HandlerFunc(helpers.endpoint(t))) - defer server.Close() - - response := &client.Response{ - Response: helpers.response(), - } - client := client.NewClient(server.URL, "MyService", "2020-02-01") - poller, err := longRunningOperationPollerFromResponse(response, client) - if err != nil { - t.Fatal(err.Error()) - } - - t.Logf("Polling..") - result, err := poller.Poll(ctx) - if err != nil { - if expected == pollers.PollingStatusCancelled || expected == pollers.PollingStatusFailed { - return - } - - t.Fatal(err.Error()) - } - if result.Status != expected { - t.Fatalf("expected status to be %q but got %q", expected, result.Status) - } - // sanity-checking - helpers.assertCalled(t, 1) - }) - } -} - func TestPollerLRO_InProvisioningState_ImmediateSuccess(t *testing.T) { ctx := context.TODO() helpers := newLongRunningOperationsEndpoint([]expectedResponse{ @@ -304,6 +227,86 @@ func TestPollerLRO_InStatus_AcceptedThenInProgressThenSuccess(t *testing.T) { helpers.assertCalled(t, 3) } +func TestPollerLRO_NoTerminalStatusThenSuccess(t *testing.T) { + ctx := context.TODO() + helpers := newLongRunningOperationsEndpoint([]expectedResponse{ + responseWithHttpStatusCode(http.StatusAccepted), + responseWithNoTerminalState(), + responseWithNoTerminalState(), + responseWithStatusInStatusField(statusSucceeded), + }) + server := httptest.NewServer(http.HandlerFunc(helpers.endpoint(t))) + defer server.Close() + + response := &client.Response{ + Response: helpers.response(), + } + client := client.NewClient(server.URL, "MyService", "2020-02-01") + poller, err := longRunningOperationPollerFromResponse(response, client) + if err != nil { + t.Fatal(err.Error()) + } + + expectedStatuses := []pollers.PollingStatus{ + pollers.PollingStatusInProgress, + pollers.PollingStatusInProgress, + pollers.PollingStatusInProgress, + pollers.PollingStatusSucceeded, + } + for i, expected := range expectedStatuses { + t.Logf("Poll %d..", i) + result, err := poller.Poll(ctx) + if err != nil { + t.Fatal(err.Error()) + } + if result.Status != expected { + t.Fatalf("expected status to be %q but got %q", expected, result.Status) + } + } + // sanity-checking + helpers.assertCalled(t, 4) +} + +func TestPollerLRO_ArbitraryStatusThenSuccess(t *testing.T) { + ctx := context.TODO() + helpers := newLongRunningOperationsEndpoint([]expectedResponse{ + responseWithHttpStatusCode(http.StatusAccepted), + responseWithStatusInStatusField("Reticulating Splines"), + responseWithStatusInStatusField("Retracting Phong Shader"), + responseWithStatusInStatusField(statusSucceeded), + }) + server := httptest.NewServer(http.HandlerFunc(helpers.endpoint(t))) + defer server.Close() + + response := &client.Response{ + Response: helpers.response(), + } + client := client.NewClient(server.URL, "MyService", "2020-02-01") + poller, err := longRunningOperationPollerFromResponse(response, client) + if err != nil { + t.Fatal(err.Error()) + } + + expectedStatuses := []pollers.PollingStatus{ + pollers.PollingStatusInProgress, + pollers.PollingStatusInProgress, + pollers.PollingStatusInProgress, + pollers.PollingStatusSucceeded, + } + for i, expected := range expectedStatuses { + t.Logf("Poll %d..", i) + result, err := poller.Poll(ctx) + if err != nil { + t.Fatal(err.Error()) + } + if result.Status != expected { + t.Fatalf("expected status to be %q but got %q", expected, result.Status) + } + } + // sanity-checking + helpers.assertCalled(t, 4) +} + func TestPollerLRO_InProvisioningState_AcceptedThenDroppedThenInProgressThenSuccess(t *testing.T) { ctx := context.TODO() helpers := newLongRunningOperationsEndpoint([]expectedResponse{ From ac9b8fda2434ea06ab9f5fe6b23d813e52eb0601 Mon Sep 17 00:00:00 2001 From: jackofallops Date: Tue, 8 Jul 2025 11:55:38 +0200 Subject: [PATCH 2/3] linting fixes --- sdk/client/client.go | 2 +- sdk/client/msgraph/client.go | 4 ++-- sdk/client/pollers/interface.go | 6 +++--- sdk/client/pollers/poller.go | 7 ++----- sdk/client/resourcemanager/errors.go | 3 +-- sdk/client/resourcemanager/errors_test.go | 20 +++++++++---------- .../resourcemanager/poller_helpers_test.go | 6 ++++++ .../poller_provisioning_state_helpers_test.go | 5 +++-- sdk/internal/accept/accept.go | 2 +- sdk/internal/stringfmt/quote_and_split_at.go | 3 +-- 10 files changed, 30 insertions(+), 28 deletions(-) diff --git a/sdk/client/client.go b/sdk/client/client.go index a52e5dad664..4f72f102242 100644 --- a/sdk/client/client.go +++ b/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{ diff --git a/sdk/client/msgraph/client.go b/sdk/client/msgraph/client.go index 0a74b22cad1..e17e33942ac 100644 --- a/sdk/client/msgraph/client.go +++ b/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 diff --git a/sdk/client/pollers/interface.go b/sdk/client/pollers/interface.go index ed355b57650..ce6741bc12c 100644 --- a/sdk/client/pollers/interface.go +++ b/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" } diff --git a/sdk/client/pollers/poller.go b/sdk/client/pollers/poller.go index 1601d28dcc8..673e9093d60 100644 --- a/sdk/client/pollers/poller.go +++ b/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 { diff --git a/sdk/client/resourcemanager/errors.go b/sdk/client/resourcemanager/errors.go index 942ded03662..7bda96b543e 100644 --- a/sdk/client/resourcemanager/errors.go +++ b/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, diff --git a/sdk/client/resourcemanager/errors_test.go b/sdk/client/resourcemanager/errors_test.go index 251e2575374..2d12ef96827 100644 --- a/sdk/client/resourcemanager/errors_test.go +++ b/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) } diff --git a/sdk/client/resourcemanager/poller_helpers_test.go b/sdk/client/resourcemanager/poller_helpers_test.go index ac0c999a99f..e7bdbf1b00e 100644 --- a/sdk/client/resourcemanager/poller_helpers_test.go +++ b/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 { diff --git a/sdk/client/resourcemanager/poller_provisioning_state_helpers_test.go b/sdk/client/resourcemanager/poller_provisioning_state_helpers_test.go index 9bdde59f9e2..a5c654996b8 100644 --- a/sdk/client/resourcemanager/poller_provisioning_state_helpers_test.go +++ b/sdk/client/resourcemanager/poller_provisioning_state_helpers_test.go @@ -85,13 +85,14 @@ func (e *provisioningStateEndpoint) endpoint(t *testing.T) func(w http.ResponseW } } -func (e provisioningStateEndpoint) assertCalled(t *testing.T, expected int) { +func (e *provisioningStateEndpoint) assertCalled(t *testing.T, expected int) { if e.numberOfTimesCalled != expected { t.Fatalf("expected to be called %d times but got %d", expected, e.numberOfTimesCalled) } } -func (e provisioningStateEndpoint) response() *http.Response { +// nolint: unused +func (e *provisioningStateEndpoint) response() *http.Response { return &http.Response{ Header: map[string][]string{ // NOTE: any custom host/port is discarded, since we use the host from the Client when building the diff --git a/sdk/internal/accept/accept.go b/sdk/internal/accept/accept.go index 364f7854b2b..0fe5feb3045 100644 --- a/sdk/internal/accept/accept.go +++ b/sdk/internal/accept/accept.go @@ -26,7 +26,7 @@ func (h Header) FirstChoice() *PreferredType { func (h Header) String() string { out := make([]string, 0) for _, typ := range h.types { - out = append(out, fmt.Sprintf("%s", typ)) + out = append(out, typ.String()) } return strings.Join(out, ", ") } diff --git a/sdk/internal/stringfmt/quote_and_split_at.go b/sdk/internal/stringfmt/quote_and_split_at.go index f499fccadea..1c5e32df9a6 100644 --- a/sdk/internal/stringfmt/quote_and_split_at.go +++ b/sdk/internal/stringfmt/quote_and_split_at.go @@ -5,7 +5,6 @@ package stringfmt import ( "fmt" - "math" ) // QuoteAndSplitString cuts a string to the specified number of characters and then quotes it @@ -13,7 +12,7 @@ import ( func QuoteAndSplitString(message, quoteChar string, characters int) []string { lines := make([]string, 0) - numberOfLines := int(math.Ceil(float64(len(message) / (characters * 1.0)))) + numberOfLines := int(float64(len(message) / (characters * 1.0))) for i := 0; i <= numberOfLines; i++ { start := characters * i remainingString := message[start:] From 9bfe0a18610f4b800db3e49ab136d23e094c5819 Mon Sep 17 00:00:00 2001 From: jackofallops Date: Wed, 9 Jul 2025 16:24:42 +0200 Subject: [PATCH 3/3] update provisioningstatepoller for terminal state processing --- sdk/client/resourcemanager/poller_lro.go | 8 ++++++++ sdk/client/resourcemanager/poller_provisioning_state.go | 5 +++-- 2 files changed, 11 insertions(+), 2 deletions(-) diff --git a/sdk/client/resourcemanager/poller_lro.go b/sdk/client/resourcemanager/poller_lro.go index 7d1e5d814f0..e3b729e9b1f 100644 --- a/sdk/client/resourcemanager/poller_lro.go +++ b/sdk/client/resourcemanager/poller_lro.go @@ -238,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 +} diff --git a/sdk/client/resourcemanager/poller_provisioning_state.go b/sdk/client/resourcemanager/poller_provisioning_state.go index cf80b2c4f0d..f9980830c61 100644 --- a/sdk/client/resourcemanager/poller_provisioning_state.go +++ b/sdk/client/resourcemanager/poller_provisioning_state.go @@ -115,10 +115,11 @@ func (p *provisioningStatePoller) Poll(ctx context.Context) (*pollers.PollResult } status := "" - if string(result.Status) != "" { + if statusIsTerminal(result.Status) { status = string(result.Status) } - if string(result.Properties.ProvisioningState) != "" { + // if the result has a provisioningState field, we should prioritise that + if statusIsTerminal(result.Properties.ProvisioningState) { status = string(result.Properties.ProvisioningState) } if status == "" {