Merge pull request hudl#52 from seh/expose-http-status-code-in-errors

Expose response HTTP status code in errors

errors.go

@@ -2,6 +2,32 @@ package fargo
 
 // MIT Licensed (see README.md) - Copyright (c) 2013 Hudl <@Hudl>
 
+import (
+	"fmt"
+)
+
+type unsuccessfulHTTPResponse struct {
+	statusCode    int
+	messagePrefix string
+}
+
+func (u *unsuccessfulHTTPResponse) Error() string {
+	if len(u.messagePrefix) > 0 {
+		return fmt.Sprint(u.messagePrefix, ", rcode = ", u.statusCode)
+	}
+	return fmt.Sprint("rcode = ", u.statusCode)
+}
+
+// HTTPResponseStatusCode extracts the HTTP status code for the response from Eureka that motivated
+// the supplied error, if any. If the returned present value is true, the returned code is an HTTP
+// status code.
+func HTTPResponseStatusCode(err error) (code int, present bool) {
+	if u, ok := err.(*unsuccessfulHTTPResponse); ok {
+		return u.statusCode, true
+	}
+	return 0, false
+}
+
 type AppNotFoundError struct {
 	specific string
 }

errors_test.go

@@ -0,0 +1,52 @@
+package fargo
+
+// MIT Licensed (see README.md) - Copyright (c) 2013 Hudl <@Hudl>
+
+import (
+	"errors"
+	"strconv"
+	"testing"
+
+	. "github.com/smartystreets/goconvey/convey"
+)
+
+func TestHTTPResponseStatusCode(t *testing.T) {
+	Convey("An nil error should have no HTTP status code", t, func() {
+		_, present := HTTPResponseStatusCode(nil)
+		So(present, ShouldBeFalse)
+	})
+	Convey("A foreign error should have no detectable HTTP status code", t, func() {
+		_, present := HTTPResponseStatusCode(errors.New("other"))
+		So(present, ShouldBeFalse)
+	})
+	Convey("A fargo error generated from a response from Eureka", t, func() {
+		verify := func(err *unsuccessfulHTTPResponse) {
+			Convey("should have the given HTTP status code", func() {
+				code, present := HTTPResponseStatusCode(err)
+				So(present, ShouldBeTrue)
+				So(code, ShouldEqual, err.statusCode)
+				Convey("should produce a message", func() {
+					msg := err.Error()
+					if len(err.messagePrefix) == 0 {
+						Convey("that lacks a prefx", func() {
+							So(msg, ShouldNotStartWith, ",")
+						})
+					} else {
+						Convey("that starts with the given prefix", func() {
+							So(msg, ShouldStartWith, err.messagePrefix)
+						})
+					}
+					Convey("that contains the status code in decimal notation", func() {
+						So(msg, ShouldContainSubstring, strconv.Itoa(err.statusCode))
+					})
+				})
+			})
+		}
+		Convey("with a message prefix", func() {
+			verify(&unsuccessfulHTTPResponse{500, "operation failed"})
+		})
+		Convey("without a message prefix", func() {
+			verify(&unsuccessfulHTTPResponse{statusCode: 500})
+		})
+	})
+}

net.go

@@ -131,7 +131,7 @@ func (e *EurekaConnection) RegisterInstance(ins *Instance) error {
 			ins.Id(), ins.App, err.Error())
 		return err
 	}
-	if rcode == 200 {
+	if rcode == http.StatusOK {
 		log.Noticef("Instance=%s already exists in App=%s, aborting registration", ins.Id(), ins.App)
 		return nil
 	}
@@ -164,7 +164,7 @@ func (e *EurekaConnection) ReregisterInstance(ins *Instance) error {
 	if rcode != 204 {
 		log.Warningf("HTTP returned %d registering Instance=%s App=%s Body=\"%s\"", rcode,
 			ins.Id(), ins.App, string(body))
-		return fmt.Errorf("http returned %d possible failure registering instance\n", rcode)
+		return &unsuccessfulHTTPResponse{rcode, "possible failure registering instance"}
 	}
 
 	// read back our registration to pick up eureka-supplied values
@@ -182,8 +182,8 @@ func (e *EurekaConnection) GetInstance(app, insId string) (*Instance, error) {
 	if err != nil {
 		return nil, err
 	}
-	if rcode != 200 {
-		return nil, fmt.Errorf("Error getting instance, rcode = %d", rcode)
+	if rcode != http.StatusOK {
+		return nil, &unsuccessfulHTTPResponse{rcode, "unable to retrieve instance"}
 	}
 	var ins *Instance
 	if e.UseJson {
@@ -219,9 +219,9 @@ func (e *EurekaConnection) DeregisterInstance(ins *Instance) error {
 	}
 	// Eureka promises to return HTTP status code upon deregistration success, but fargo used to accept status code 204
 	// here instead. Accommodate both for backward compatibility with any fake or proxy Eureka stand-ins.
-	if rcode != 200 && rcode != 204 {
+	if rcode != http.StatusOK && rcode != http.StatusNoContent {
 		log.Warningf("HTTP returned %d deregistering Instance=%s App=%s", rcode, ins.Id(), ins.App)
-		return fmt.Errorf("http returned %d possible failure deregistering instance\n", rcode)
+		return &unsuccessfulHTTPResponse{rcode, "possible failure deregistering instance"}
 	}
 
 	return nil
@@ -243,7 +243,7 @@ func (e EurekaConnection) AddMetadataString(ins *Instance, key, value string) er
 	if rcode < 200 || rcode >= 300 {
 		log.Warningf("HTTP returned %d updating metadata Instance=%s App=%s Body=\"%s\"", rcode,
 			ins.Id(), ins.App, string(body))
-		return fmt.Errorf("http returned %d possible failure updating instance metadata ", rcode)
+		return &unsuccessfulHTTPResponse{rcode, "possible failure updating instance metadata"}
 	}
 	ins.SetMetadataString(key, value)
 	return nil
@@ -265,7 +265,7 @@ func (e EurekaConnection) UpdateInstanceStatus(ins *Instance, status StatusType)
 	if rcode < 200 || rcode >= 300 {
 		log.Warningf("HTTP returned %d updating status Instance=%s App=%s Body=\"%s\"", rcode,
 			ins.Id(), ins.App, string(body))
-		return fmt.Errorf("http returned %d possible failure updating instance status ", rcode)
+		return &unsuccessfulHTTPResponse{rcode, "possible failure updating instance status"}
 	}
 	return nil
 }
@@ -286,9 +286,9 @@ func (e *EurekaConnection) HeartBeatInstance(ins *Instance) error {
 		log.Errorf("Error sending heartbeat for Instance=%s App=%s, error: %s", ins.Id(), ins.App, err.Error())
 		return err
 	}
-	if rcode != 200 {
+	if rcode != http.StatusOK {
 		log.Errorf("Sending heartbeat for Instance=%s App=%s returned code %d", ins.Id(), ins.App, rcode)
-		return fmt.Errorf("heartbeat returned code %d\n", rcode)
+		return &unsuccessfulHTTPResponse{rcode, "heartbeat failed"}
 	}
 	return nil
 }

tests/net_test.go

@@ -3,11 +3,33 @@ package fargo_test
 // MIT Licensed (see README.md) - Copyright (c) 2013 Hudl <@Hudl>
 
 import (
+	"fmt"
+	"net/http"
+	"testing"
+
 	"github.com/hudl/fargo"
 	. "github.com/smartystreets/goconvey/convey"
-	"testing"
 )
 
+func shouldNotBearAnHTTPStatusCode(actual interface{}, expected ...interface{}) string {
+	if code, present := fargo.HTTPResponseStatusCode(actual.(error)); present {
+		return fmt.Sprintf("Expected: no HTTP status code\nActual:   %d", code)
+	}
+	return ""
+}
+
+func shouldBearHTTPStatusCode(actual interface{}, expected ...interface{}) string {
+	expectedCode := expected[0]
+	code, present := fargo.HTTPResponseStatusCode(actual.(error))
+	if !present {
+		return fmt.Sprintf("Expected: %d\nActual:   no HTTP status code", expectedCode)
+	}
+	if code != expectedCode {
+		return fmt.Sprintf("Expected: %d\nActual:   %d", expectedCode, code)
+	}
+	return ""
+}
+
 func TestConnectionCreation(t *testing.T) {
 	Convey("Pull applications", t, func() {
 		cfg, err := fargo.ReadConfig("./config_sample/local.gcfg")
@@ -64,6 +86,7 @@ func TestRegistration(t *testing.T) {
 			}
 			err := e.HeartBeatInstance(&j)
 			So(err, ShouldNotBeNil)
+			So(err, shouldBearHTTPStatusCode, http.StatusNotFound)
 		})
 		Convey("Register an instance to TESTAPP", t, func() {
 			Convey("Instance registers correctly", func() {
@@ -149,6 +172,7 @@ func DontTestDeregistration(t *testing.T) {
 		Convey("Instance cannot check in", func() {
 			err := e.HeartBeatInstance(&i)
 			So(err, ShouldNotBeNil)
+			So(err, shouldBearHTTPStatusCode, http.StatusNotFound)
 		})
 	})
 }