From 58fb27e70f66f3db4553a2a2cfa1d055bd0d7e3a Mon Sep 17 00:00:00 2001
From: James Phillips <james@hashicorp.com>
Date: Wed, 6 Jan 2016 11:35:16 -0800
Subject: [PATCH] Factors server error checking into a new function.

---
 api/api.go       | 15 +++++++++++++++
 api/api_test.go  | 14 ++++++++++++++
 api/lock.go      |  7 +------
 api/semaphore.go |  7 +------
 4 files changed, 31 insertions(+), 12 deletions(-)

diff --git a/api/api.go b/api/api.go
index c5745ab6a4d0..0a2a76e5dc6d 100644
--- a/api/api.go
+++ b/api/api.go
@@ -273,6 +273,21 @@ func durToMsec(dur time.Duration) string {
 	return fmt.Sprintf("%dms", ms)
 }
 
+// serverError is a string we look for to detect 500 errors.
+const serverError = "Unexpected response code: 500"
+
+// IsServerError returns true for 500 errors from the Consul servers, these are
+// usually retryable at a later time.
+func IsServerError(err error) bool {
+	if err == nil {
+		return false
+	}
+
+	// TODO (slackpad) - Make a real error type here instead of using
+	// a string check.
+	return strings.Contains(err.Error(), serverError)
+}
+
 // setWriteOptions is used to annotate the request with
 // additional write options
 func (r *request) setWriteOptions(q *WriteOptions) {
diff --git a/api/api_test.go b/api/api_test.go
index 4d60859f9cfb..c2d178ddee87 100644
--- a/api/api_test.go
+++ b/api/api_test.go
@@ -272,3 +272,17 @@ func TestAPI_durToMsec(t *testing.T) {
 		t.Fatalf("bad: %s", ms)
 	}
 }
+
+func TestAPI_IsServerError(t *testing.T) {
+	if IsServerError(nil) {
+		t.Fatalf("should not be a server error")
+	}
+
+	if IsServerError(fmt.Errorf("not the error you are looking for")) {
+		t.Fatalf("should not be a server error")
+	}
+
+	if !IsServerError(fmt.Errorf(serverError)) {
+		t.Fatalf("should be a server error")
+	}
+}
diff --git a/api/lock.go b/api/lock.go
index fafe8b525906..11cfdedbc8cb 100644
--- a/api/lock.go
+++ b/api/lock.go
@@ -2,7 +2,6 @@ package api
 
 import (
 	"fmt"
-	"strings"
 	"sync"
 	"time"
 )
@@ -362,15 +361,11 @@ WAIT:
 RETRY:
 	pair, meta, err := kv.Get(l.opts.Key, opts)
 	if err != nil {
-		// TODO (slackpad) - Make a real error type here instead of using
-		// a string check.
-		const serverError = "Unexpected response code: 500"
-
 		// If configured we can try to ride out a brief Consul unavailability
 		// by doing retries. Note that we have to attempt the retry in a non-
 		// blocking fashion so that we have a clean place to reset the retry
 		// counter if service is restored.
-		if retries > 0 && strings.Contains(err.Error(), serverError) {
+		if retries > 0 && IsServerError(err) {
 			time.Sleep(l.opts.MonitorRetryTime)
 			retries--
 			opts.WaitIndex = 0
diff --git a/api/semaphore.go b/api/semaphore.go
index 2152ead203b8..e6645ac1d37b 100644
--- a/api/semaphore.go
+++ b/api/semaphore.go
@@ -4,7 +4,6 @@ import (
 	"encoding/json"
 	"fmt"
 	"path"
-	"strings"
 	"sync"
 	"time"
 )
@@ -488,15 +487,11 @@ WAIT:
 RETRY:
 	pairs, meta, err := kv.List(s.opts.Prefix, opts)
 	if err != nil {
-		// TODO (slackpad) - Make a real error type here instead of using
-		// a string check.
-		const serverError = "Unexpected response code: 500"
-
 		// If configured we can try to ride out a brief Consul unavailability
 		// by doing retries. Note that we have to attempt the retry in a non-
 		// blocking fashion so that we have a clean place to reset the retry
 		// counter if service is restored.
-		if retries > 0 && strings.Contains(err.Error(), serverError) {
+		if retries > 0 && IsServerError(err) {
 			time.Sleep(s.opts.MonitorRetryTime)
 			retries--
 			opts.WaitIndex = 0