Skip to content

Commit

Permalink
Enhance Message For Cache Errors (#1175)
Browse files Browse the repository at this point in the history
  • Loading branch information
@SyntaxNode @guscarreon
SyntaxNode authored and guscarreon committed Jan 27, 2020
1 parent 84e2c26 commit 94cc354
Show file tree
Hide file tree
Showing 2 changed files with 74 additions and 40 deletions.
34 changes: 15 additions & 19 deletions prebid_cache_client/client.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"bytes"
"context"
"encoding/json"
"errors"
"fmt"
"io/ioutil"
"net/http"
Expand Down Expand Up @@ -92,15 +93,13 @@ func (c *clientImpl) PutJson(ctx context.Context, values []Cacheable) (uuids []s

postBody, err := encodeValues(values)
if err != nil {
glog.Errorf("Error creating JSON for prebid cache: %v", err)
errs = append(errs, fmt.Errorf("Error creating JSON for prebid cache: %v", err))
logError(&errs, "Error creating JSON for prebid cache: %v", err)
return uuidsToReturn, errs
}

httpReq, err := http.NewRequest("POST", c.putUrl, bytes.NewReader(postBody))
if err != nil {
glog.Errorf("Error creating POST request to prebid cache: %v", err)
errs = append(errs, fmt.Errorf("Error creating POST request to prebid cache: %v", err))
logError(&errs, "Error creating POST request to prebid cache: %v", err)
return uuidsToReturn, errs
}

Expand All @@ -112,51 +111,48 @@ func (c *clientImpl) PutJson(ctx context.Context, values []Cacheable) (uuids []s
elapsedTime := time.Since(startTime)
if err != nil {
c.metrics.RecordPrebidCacheRequestTime(false, elapsedTime)
friendlyErr := fmt.Errorf("Error sending the request to Prebid Cache: %v; Duration=%v", err, elapsedTime)
glog.Error(friendlyErr)
errs = append(errs, friendlyErr)
logError(&errs, "Error sending the request to Prebid Cache: %v; Duration=%v, Items=%v, Payload Size=%v", err, elapsedTime, len(values), len(postBody))
return uuidsToReturn, errs
}
defer anResp.Body.Close()
c.metrics.RecordPrebidCacheRequestTime(true, elapsedTime)

responseBody, err := ioutil.ReadAll(anResp.Body)
if anResp.StatusCode != 200 {
glog.Errorf("Prebid Cache call to %s returned %d: %s", putURL, anResp.StatusCode, responseBody)
errs = append(errs, fmt.Errorf("Prebid Cache call to %s returned %d: %s", putURL, anResp.StatusCode, responseBody))
logError(&errs, "Prebid Cache call to %s returned %d: %s", putURL, anResp.StatusCode, responseBody)
return uuidsToReturn, errs
}

currentIndex := 0
processResponse := func(uuidObj []byte, _ jsonparser.ValueType, _ int, err error) {
if uuid, valueType, _, err := jsonparser.Get(uuidObj, "uuid"); err != nil {
glog.Errorf("Prebid Cache returned a bad value at index %d. Error was: %v. Response body was: %s", currentIndex, err, string(responseBody))
errs = append(errs, fmt.Errorf("Prebid Cache returned a bad value at index %d. Error was: %v. Response body was: %s", currentIndex, err, string(responseBody)))
logError(&errs, "Prebid Cache returned a bad value at index %d. Error was: %v. Response body was: %s", currentIndex, err, string(responseBody))
} else if valueType != jsonparser.String {
glog.Errorf("Prebid Cache returned a %v at index %d in: %v", valueType, currentIndex, string(responseBody))
errs = append(errs, fmt.Errorf("Prebid Cache returned a %v at index %d in: %v", valueType, currentIndex, string(responseBody)))
logError(&errs, "Prebid Cache returned a %v at index %d in: %v", valueType, currentIndex, string(responseBody))
} else {
if uuidsToReturn[currentIndex], err = jsonparser.ParseString(uuid); err != nil {
glog.Errorf("Prebid Cache response index %d could not be parsed as string: %v", currentIndex, err)
errs = append(errs, fmt.Errorf("Prebid Cache response index %d could not be parsed as string: %v", currentIndex, err))
logError(&errs, "Prebid Cache response index %d could not be parsed as string: %v", currentIndex, err)
uuidsToReturn[currentIndex] = ""
}
}
currentIndex++
}

if _, err := jsonparser.ArrayEach(responseBody, processResponse, "responses"); err != nil {
glog.Errorf("Error interpreting Prebid Cache response: %v\nResponse was: %s", err, string(responseBody))
errs = append(errs, fmt.Errorf("Error interpreting Prebid Cache response: %v\nResponse was: %s", err, string(responseBody)))
logError(&errs, "Error interpreting Prebid Cache response: %v\nResponse was: %s", err, string(responseBody))
return uuidsToReturn, errs
}

return uuidsToReturn, errs
}

func logError(errs *[]error, format string, a ...interface{}) {
msg := fmt.Sprintf(format, a...)
glog.Error(msg)
*errs = append(*errs, errors.New(msg))
}

func encodeValues(values []Cacheable) ([]byte, error) {
// This function assumes that m is non-nil and has at least one element.
// clientImp.PutBids should respect this.
var buf bytes.Buffer
buf.WriteString(`{"puts":[`)
for i := 0; i < len(values); i++ {
Expand Down
80 changes: 59 additions & 21 deletions prebid_cache_client/client_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"bytes"
"context"
"encoding/json"
"fmt"
"net/http"
"net/http/httptest"
"strconv"
Expand All @@ -17,7 +18,6 @@ import (
"github.com/stretchr/testify/mock"
)

// Prevents #197
func TestEmptyPut(t *testing.T) {
handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
t.Errorf("The server should not be called.")
Expand Down Expand Up @@ -72,32 +72,70 @@ func TestBadResponse(t *testing.T) {
}

func TestCancelledContext(t *testing.T) {
handler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
testCases := []struct {
description string
cacheable []Cacheable
expectedItems int
expectedPayloadSize int
}{
{
description: "1 Item",
cacheable: []Cacheable{
{
Type: TypeJSON,
Data: json.RawMessage("true"),
},
},
expectedItems: 1,
expectedPayloadSize: 39,
},
{
description: "2 Items",
cacheable: []Cacheable{
{
Type: TypeJSON,
Data: json.RawMessage("true"),
},
{
Type: TypeJSON,
Data: json.RawMessage("false"),
},
},
expectedItems: 2,
expectedPayloadSize: 69,
},
}

// Initialize Stub Server
stubHandler := http.HandlerFunc(func(w http.ResponseWriter, r *http.Request) {
w.WriteHeader(200)
})
server := httptest.NewServer(handler)
defer server.Close()
stubServer := httptest.NewServer(stubHandler)
defer stubServer.Close()

metricsMock := &pbsmetrics.MetricsEngineMock{}
metricsMock.On("RecordPrebidCacheRequestTime", false, mock.Anything).Once()
// Run Tests
for _, testCase := range testCases {
metricsMock := &pbsmetrics.MetricsEngineMock{}
metricsMock.On("RecordPrebidCacheRequestTime", false, mock.Anything).Once()

client := &clientImpl{
httpClient: server.Client(),
putUrl: server.URL,
metrics: metricsMock,
}
client := &clientImpl{
httpClient: stubServer.Client(),
putUrl: stubServer.URL,
metrics: metricsMock,
}

ctx, cancel := context.WithCancel(context.Background())
cancel()
ids, _ := client.PutJson(ctx, []Cacheable{{
Type: TypeJSON,
Data: json.RawMessage("true"),
},
})
assertIntEqual(t, len(ids), 1)
assertStringEqual(t, ids[0], "")
ctx, cancel := context.WithCancel(context.Background())
cancel()
ids, errs := client.PutJson(ctx, testCase.cacheable)

metricsMock.AssertExpectations(t)
expectedErrorMessage := fmt.Sprintf("Items=%v, Payload Size=%v", testCase.expectedItems, testCase.expectedPayloadSize)

assert.Equal(t, testCase.expectedItems, len(ids), testCase.description+":ids")
assert.Len(t, errs, 1)
assert.Contains(t, errs[0].Error(), "Error sending the request to Prebid Cache: context canceled", testCase.description+":error")
assert.Contains(t, errs[0].Error(), expectedErrorMessage, testCase.description+":error_dimensions")
metricsMock.AssertExpectations(t)
}
}

func TestSuccessfulPut(t *testing.T) {
Expand Down

0 comments on commit 94cc354

Please sign in to comment.