Skip to content

Commit

Permalink
Expose response HTTP status code in errors
Browse files Browse the repository at this point in the history
For various operations involving HTTP requests against the Eureka
server, allow callers to interpret surfaced failures by way of
inspecting the HTTP response's status code.
  • Loading branch information
seh committed Sep 27, 2016
1 parent 9a362c8 commit c913d16
Show file tree
Hide file tree
Showing 4 changed files with 92 additions and 11 deletions.
23 changes: 23 additions & 0 deletions errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,29 @@ 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 {
return fmt.Sprint(u.messagePrefix, ", 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
}
Expand Down
34 changes: 34 additions & 0 deletions errors_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
package fargo

// MIT Licensed (see README.md) - Copyright (c) 2013 Hudl <@Hudl>

import (
"errors"
"testing"
)

func TestHTTPResponseStatusCode(t *testing.T) {
tests := []struct {
input error
present bool
code int
}{
{nil, false, 0},
{errors.New("other"), false, 0},
{&unsuccessfulHTTPResponse{404, "missing"}, true, 404},
}
for _, test := range tests {
code, present := HTTPResponseStatusCode(test.input)
if present {
if !test.present {
t.Errorf("input %v: want absent, got code %d", test.input, code)
continue
}
if code != test.code {
t.Errorf("input %v: want %d, got %d", test.input, test.code, code)
}
} else if test.present {
t.Errorf("input %v: want code %d, got absent", test.input, test.code)
}
}
}
20 changes: 10 additions & 10 deletions net.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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
Expand All @@ -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 {
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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
}
Expand All @@ -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
}
Expand Down
26 changes: 25 additions & 1 deletion tests/net_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
Expand Down Expand Up @@ -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() {
Expand Down Expand Up @@ -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)
})
})
}
Expand Down

0 comments on commit c913d16

Please sign in to comment.