Merge pull request #1108 from hashicorp/b/broader-tcp-dial-failure-handling

Error Handling - update connection timeout regex to catch broader range of error conditions

Author: jackofallops

sdk/client/client.go

@@ -777,10 +777,10 @@ func extendedRetryPolicy(resp *http.Response, err error) (bool, error) {
 
 	// A regular expression to catch dial timeouts in the underlying TCP session
 	// connection
-	tcpDialTimeoutRe := regexp.MustCompile(`dial tcp .*: i/o timeout`)
+	tcpDialTCPRe := regexp.MustCompile(`dial tcp`)
 
-	// A regular expression to match complete packet loss - see comment below on packet-loss scenarios
-	// completePacketLossRe := regexp.MustCompile(`EOF$`)
+	// A regular expression to match complete packet loss
+	completePacketLossRe := regexp.MustCompile(`EOF`)
 
 	if err != nil {
 		var v *url.Error
@@ -805,15 +805,13 @@ func extendedRetryPolicy(resp *http.Response, err error) (bool, error) {
 				return false, v
 			}
 
-			if tcpDialTimeoutRe.MatchString(v.Error()) {
+			if tcpDialTCPRe.MatchString(v.Error()) {
 				return false, v
 			}
 
-			// TODO - Need to investigate how to deal with total packet-loss situations that doesn't break LRO retries.
-			// Such as Temporary Proxy outage, or recoverable disruption to network traffic (e.g. bgp events etc)
-			// if completePacketLossRe.MatchString(v.Error()) {
-			//	return false, v
-			// }
+			if completePacketLossRe.MatchString(v.Error()) {
+				return false, v
+			}
 
 			var certificateVerificationError *tls.CertificateVerificationError
 			if ok := errors.As(v.Err, &certificateVerificationError); ok {

sdk/client/resourcemanager/poller_lro.go

@@ -7,6 +7,7 @@ import (
 	"bytes"
 	"context"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"io"
 	"net/http"
@@ -27,6 +28,9 @@ type longRunningOperationPoller struct {
 	initialRetryDuration time.Duration
 	originalUrl          *url.URL
 	pollingUrl           *url.URL
+
+	droppedConnectionCount int
+	maxDroppedConnections  int
 }
 
 func pollingUriForLongRunningOperation(resp *client.Response) string {
@@ -39,8 +43,9 @@ func pollingUriForLongRunningOperation(resp *client.Response) string {
 
 func longRunningOperationPollerFromResponse(resp *client.Response, client *client.Client) (*longRunningOperationPoller, error) {
 	poller := longRunningOperationPoller{
-		client:               client,
-		initialRetryDuration: 10 * time.Second,
+		client:                client,
+		initialRetryDuration:  10 * time.Second,
+		maxDroppedConnections: 3,
 	}
 
 	pollingUrl := pollingUriForLongRunningOperation(resp)
@@ -107,9 +112,20 @@ func (p *longRunningOperationPoller) Poll(ctx context.Context) (result *pollers.
 	}
 	result.HttpResponse, err = req.Execute(ctx)
 	if err != nil {
+		var e *url.Error
+		if errors.As(err, &e) {
+			p.droppedConnectionCount++
+			if p.droppedConnectionCount < p.maxDroppedConnections {
+				result.Status = pollers.PollingStatusUnknown
+				return result, nil
+			}
+		}
+
 		return nil, err
 	}
 
+	p.droppedConnectionCount = 0
+
 	if result.HttpResponse != nil {
 		var respBody []byte
 		respBody, err = io.ReadAll(result.HttpResponse.Body)

sdk/client/resourcemanager/poller_lro_test.go

@@ -326,6 +326,7 @@ func TestPollerLRO_InProvisioningState_AcceptedThenDroppedThenInProgressThenSucc
 
 	expectedStatuses := []pollers.PollingStatus{
 		pollers.PollingStatusInProgress, // the 202 Accepted
+		pollers.PollingStatusUnknown,
 		// NOTE: the Dropped Connection will be ignored/silently retried
 		pollers.PollingStatusInProgress, // working on it
 		pollers.PollingStatusSucceeded,  // good
@@ -366,7 +367,7 @@ func TestPollerLRO_InStatus_AcceptedThenDroppedThenInProgressThenSuccess(t *test
 
 	expectedStatuses := []pollers.PollingStatus{
 		pollers.PollingStatusInProgress, // the 202 Accepted
-		// NOTE: the Dropped Connection will be ignored/silently retried
+		pollers.PollingStatusUnknown,
 		pollers.PollingStatusInProgress, // working on it
 		pollers.PollingStatusSucceeded,  // good
 	}

sdk/client/resourcemanager/poller_provisioning_state.go

@@ -5,6 +5,7 @@ package resourcemanager
 
 import (
 	"context"
+	"errors"
 	"fmt"
 	"net/http"
 	"net/url"
@@ -27,6 +28,9 @@ type provisioningStatePoller struct {
 	initialRetryDuration time.Duration
 	originalUri          string
 	resourcePath         string
+
+	droppedConnectionCount int
+	maxDroppedConnections  int
 }
 
 func provisioningStatePollerFromResponse(response *client.Response, lroIsSelfReference bool, client *Client, pollingInterval time.Duration) (*provisioningStatePoller, error) {
@@ -58,11 +62,12 @@ func provisioningStatePollerFromResponse(response *client.Response, lroIsSelfRef
 	}
 
 	return &provisioningStatePoller{
-		apiVersion:           apiVersion,
-		client:               client,
-		initialRetryDuration: pollingInterval,
-		originalUri:          originalUri,
-		resourcePath:         resourcePath,
+		apiVersion:            apiVersion,
+		client:                client,
+		initialRetryDuration:  pollingInterval,
+		originalUri:           originalUri,
+		resourcePath:          resourcePath,
+		maxDroppedConnections: 3,
 	}, nil
 }
 
@@ -84,8 +89,22 @@ func (p *provisioningStatePoller) Poll(ctx context.Context) (*pollers.PollResult
 	}
 	resp, err := p.client.Execute(ctx, req)
 	if err != nil {
+		var e *url.Error
+		if errors.As(err, &e) {
+			p.droppedConnectionCount++
+			if p.droppedConnectionCount < p.maxDroppedConnections {
+				return &pollers.PollResult{
+					PollInterval: p.initialRetryDuration,
+					Status:       pollers.PollingStatusUnknown,
+				}, nil
+			}
+		}
+
 		return nil, fmt.Errorf("executing request: %+v", err)
 	}
+
+	p.droppedConnectionCount = 0
+
 	if resp == nil {
 		return nil, pollers.PollingDroppedConnectionError{}
 	}

sdk/client/resourcemanager/poller_provisioning_state_test.go

@@ -157,7 +157,7 @@ func TestPollerProvisioningState_OkWithNoBody_AfterPolling(t *testing.T) {
 }
 
 func TestPollerProvisioningState_InProvisioningState_DroppedThenInProgressThenSuccess(t *testing.T) {
-	ctx, cancel := context.WithTimeout(context.TODO(), 30*time.Second)
+	ctx, cancel := context.WithTimeout(context.TODO(), 90*time.Second)
 	defer cancel()
 
 	// changing where we're setting this for the heck of it
@@ -177,15 +177,17 @@ func TestPollerProvisioningState_InProvisioningState_DroppedThenInProgressThenSu
 		apiVersion: "2020-01-01",
 	}
 	poller := provisioningStatePoller{
-		apiVersion:           helper.expectedApiVersion,
-		client:               resourceManagerClient,
-		initialRetryDuration: 10,
-		originalUri:          "/provisioning-state/poll",
-		resourcePath:         "/provisioning-state/poll",
+		apiVersion:            helper.expectedApiVersion,
+		client:                resourceManagerClient,
+		initialRetryDuration:  10,
+		originalUri:           "/provisioning-state/poll",
+		resourcePath:          "/provisioning-state/poll",
+		maxDroppedConnections: 3,
 	}
 
 	expectedStatuses := []pollers.PollingStatus{
 		pollers.PollingStatusInProgress, // working on it
+		pollers.PollingStatusUnknown,
 		// NOTE: the Dropped Connection will be ignored/silently retried
 		pollers.PollingStatusInProgress, // working on it
 		pollers.PollingStatusSucceeded,  // good
@@ -200,7 +202,6 @@ func TestPollerProvisioningState_InProvisioningState_DroppedThenInProgressThenSu
 			t.Fatalf("expected status to be %q but got %q", expected, result.Status)
 		}
 	}
-	// sanity-checking - expect 4 calls but 3 statuses (since the dropped connection is silently retried)
 	helper.assertCalled(t, 4)
 }
 
@@ -225,16 +226,17 @@ func TestPollerProvisioningState_InStatus_DroppedThenInProgressThenSuccess(t *te
 		apiVersion: "2020-01-01",
 	}
 	poller := provisioningStatePoller{
-		apiVersion:           helper.expectedApiVersion,
-		client:               resourceManagerClient,
-		initialRetryDuration: 10,
-		originalUri:          "/provisioning-state/poll",
-		resourcePath:         "/provisioning-state/poll",
+		apiVersion:            helper.expectedApiVersion,
+		client:                resourceManagerClient,
+		initialRetryDuration:  10,
+		originalUri:           "/provisioning-state/poll",
+		resourcePath:          "/provisioning-state/poll",
+		maxDroppedConnections: 3,
 	}
 
 	expectedStatuses := []pollers.PollingStatus{
 		pollers.PollingStatusInProgress, // working on it
-		// NOTE: the Dropped Connection will be ignored/silently retried
+		pollers.PollingStatusUnknown,
 		pollers.PollingStatusInProgress, // working on it
 		pollers.PollingStatusSucceeded,  // good
 	}