Skip to content

Commit

Permalink
Updated handleError arguments to be pointers for video endpoint (preb…
Browse files Browse the repository at this point in the history 
…id#1128)

* Updated handleError arguments to be pointers for video endpoint

* Removing unneeded pointer to http.ResponseWriter

* Adding units test for update to handleError
  • Loading branch information
@camrice @mansinahar
camrice authored and mansinahar committed Apr 15, 2020
1 parent 31b6ac5 commit da23ce7
Show file tree
Hide file tree
Showing 2 changed files with 100 additions and 13 deletions.
26 changes: 13 additions & 13 deletions endpoints/openrtb2/video_auction.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@ func (deps *endpointDeps) VideoAuctionEndpoint(w http.ResponseWriter, r *http.Re
}
requestJson, err := ioutil.ReadAll(lr)
if err != nil {
handleError(labels, w, []error{err}, ao)
handleError(&labels, w, []error{err}, &ao)
return
}

Expand All @@ -102,35 +102,35 @@ func (deps *endpointDeps) VideoAuctionEndpoint(w http.ResponseWriter, r *http.Re

if err != nil {
if deps.cfg.VideoStoredRequestRequired {
handleError(labels, w, []error{err}, ao)
handleError(&labels, w, []error{err}, &ao)
return
}
} else {
storedRequest, errs := deps.loadStoredVideoRequest(context.Background(), storedRequestId)
if len(errs) > 0 {
handleError(labels, w, errs, ao)
handleError(&labels, w, errs, &ao)
return
}

//merge incoming req with stored video req
resolvedRequest, err = jsonpatch.MergePatch(storedRequest, requestJson)
if err != nil {
handleError(labels, w, []error{err}, ao)
handleError(&labels, w, []error{err}, &ao)
return
}
}
//unmarshal and validate combined result
videoBidReq, errL, podErrors := deps.parseVideoRequest(resolvedRequest)
if len(errL) > 0 {
handleError(labels, w, errL, ao)
handleError(&labels, w, errL, &ao)
return
}

var bidReq = &openrtb.BidRequest{}
if deps.defaultRequest {
if err := json.Unmarshal(deps.defReqJSON, bidReq); err != nil {
err = fmt.Errorf("Invalid JSON in Default Request Settings: %s", err)
handleError(labels, w, []error{err}, ao)
handleError(&labels, w, []error{err}, &ao)
return
}
}
Expand All @@ -154,7 +154,7 @@ func (deps *endpointDeps) VideoAuctionEndpoint(w http.ResponseWriter, r *http.Re
}
err := errors.New(fmt.Sprintf("all pods are incorrect: %s", strings.Join(resPodErr, "; ")))
errL = append(errL, err)
handleError(labels, w, errL, ao)
handleError(&labels, w, errL, &ao)
return
}

Expand All @@ -166,7 +166,7 @@ func (deps *endpointDeps) VideoAuctionEndpoint(w http.ResponseWriter, r *http.Re

errL = deps.validateRequest(bidReq)
if len(errL) > 0 {
handleError(labels, w, errL, ao)
handleError(&labels, w, errL, &ao)
return
}

Expand Down Expand Up @@ -194,7 +194,7 @@ func (deps *endpointDeps) VideoAuctionEndpoint(w http.ResponseWriter, r *http.Re

if acctIdErr := validateAccount(deps.cfg, labels.PubID); acctIdErr != nil {
errL = append(errL, acctIdErr)
handleError(labels, w, errL, ao)
handleError(&labels, w, errL, &ao)
return
}
//execute auction logic
Expand All @@ -203,15 +203,15 @@ func (deps *endpointDeps) VideoAuctionEndpoint(w http.ResponseWriter, r *http.Re
ao.Response = response
if err != nil {
errL := []error{err}
handleError(labels, w, errL, ao)
handleError(&labels, w, errL, &ao)
return
}

//build simplified response
bidResp, err := buildVideoResponse(response, podErrors)
if err != nil {
errL := []error{err}
handleError(labels, w, errL, ao)
handleError(&labels, w, errL, &ao)
return
}
if bidReq.Test == 1 {
Expand All @@ -222,7 +222,7 @@ func (deps *endpointDeps) VideoAuctionEndpoint(w http.ResponseWriter, r *http.Re
//resp, err := json.Marshal(response)
if err != nil {
errL := []error{err}
handleError(labels, w, errL, ao)
handleError(&labels, w, errL, &ao)
return
}

Expand All @@ -238,7 +238,7 @@ func cleanupVideoBidRequest(videoReq *openrtb_ext.BidRequestVideo, podErrors []P
return videoReq
}

func handleError(labels pbsmetrics.Labels, w http.ResponseWriter, errL []error, ao analytics.AuctionObject) {
func handleError(labels *pbsmetrics.Labels, w http.ResponseWriter, errL []error, ao *analytics.AuctionObject) {
labels.RequestStatus = pbsmetrics.RequestStatusErr
var errors string
var status int = http.StatusInternalServerError
Expand Down
87 changes: 87 additions & 0 deletions endpoints/openrtb2/video_auction_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,14 @@ package openrtb2
import (
"context"
"encoding/json"
"errors"
"io/ioutil"
"net/http/httptest"
"strings"
"testing"

"github.com/mxmCherry/openrtb"
"github.com/prebid/prebid-server/analytics"
analyticsConf "github.com/prebid/prebid-server/analytics/config"
"github.com/prebid/prebid-server/config"
"github.com/prebid/prebid-server/exchange"
Expand Down Expand Up @@ -641,6 +643,91 @@ func TestMergeOpenRTBToVideoRequest(t *testing.T) {
assert.Equal(t, videoReq.Site.Page, bidReq.Site.Page, "Device.Site.Page is incorrect")
}

func TestHandleError(t *testing.T) {
ao := analytics.AuctionObject{
Status: 200,
Errors: make([]error, 0),
}

labels := pbsmetrics.Labels{
Source: pbsmetrics.DemandUnknown,
RType: pbsmetrics.ReqTypeVideo,
PubID: pbsmetrics.PublisherUnknown,
Browser: "test browser",
CookieFlag: pbsmetrics.CookieFlagUnknown,
RequestStatus: pbsmetrics.RequestStatusOK,
}

recorder := httptest.NewRecorder()
err1 := errors.New("Error for testing handleError 1")
err2 := errors.New("Error for testing handleError 2")
handleError(&labels, recorder, []error{err1, err2}, &ao)

assert.Equal(t, pbsmetrics.RequestStatusErr, labels.RequestStatus, "labels.RequestStatus should indicate an error")
assert.Equal(t, 500, recorder.Code, "Error status should be written to writer")
assert.Equal(t, 500, ao.Status, "AnalyticsObject should have error status")
assert.Equal(t, 2, len(ao.Errors), "New errors should be appended to AnalyticsObject Errors")
assert.Equal(t, "Error for testing handleError 1", ao.Errors[0].Error(), "Error in AnalyticsObject should have test error message for first error")
assert.Equal(t, "Error for testing handleError 2", ao.Errors[1].Error(), "Error in AnalyticsObject should have test error message for second error")
}

func TestHandleErrorMetrics(t *testing.T) {
ex := &mockExchangeVideo{}
reqData, err := ioutil.ReadFile("sample-requests/video/video_invalid_sample.json")
if err != nil {
t.Fatalf("Failed to fetch a valid request: %v", err)
}
reqBody := string(getRequestPayload(t, reqData))
req := httptest.NewRequest("POST", "/openrtb2/video", strings.NewReader(reqBody))
recorder := httptest.NewRecorder()

deps, met, mod := mockDepsWithMetrics(t, ex)
deps.VideoAuctionEndpoint(recorder, req, nil)

assert.Equal(t, int64(0), met.RequestStatuses[pbsmetrics.ReqTypeVideo][pbsmetrics.RequestStatusOK].Count(), "OK requests count should be 0")
assert.Equal(t, int64(1), met.RequestStatuses[pbsmetrics.ReqTypeVideo][pbsmetrics.RequestStatusErr].Count(), "Error requests count should be 1")
assert.Equal(t, 1, len(mod.auctionObjects), "Mock AnalyticsModule should have 1 AuctionObject")
assert.Equal(t, 500, mod.auctionObjects[0].Status, "AnalyticsObject should have 500 status")
assert.Equal(t, 2, len(mod.auctionObjects[0].Errors), "AnalyticsObject should have Errors length of 2")
assert.Equal(t, "request missing required field: PodConfig.DurationRangeSec", mod.auctionObjects[0].Errors[0].Error(), "First error in AnalyticsObject should have message regarding DurationRangeSec")
assert.Equal(t, "request missing required field: PodConfig.Pods", mod.auctionObjects[0].Errors[1].Error(), "Second error in AnalyticsObject should have message regarding Pods")
}

func mockDepsWithMetrics(t *testing.T, ex *mockExchangeVideo) (*endpointDeps, *pbsmetrics.Metrics, *mockAnalyticsModule) {
theMetrics := pbsmetrics.NewMetrics(metrics.NewRegistry(), openrtb_ext.BidderList(), config.DisabledMetrics{})
mockModule := &mockAnalyticsModule{}
edep := &endpointDeps{
ex,
newParamsValidator(t),
&mockVideoStoredReqFetcher{},
&mockVideoStoredReqFetcher{},
empty_fetcher.EmptyFetcher{},
&config.Configuration{MaxRequestSize: maxSize},
theMetrics,
mockModule,
map[string]string{},
false,
[]byte{},
openrtb_ext.BidderMap,
}

return edep, theMetrics, mockModule
}

type mockAnalyticsModule struct {
auctionObjects []*analytics.AuctionObject
}

func (m *mockAnalyticsModule) LogAuctionObject(ao *analytics.AuctionObject) {
m.auctionObjects = append(m.auctionObjects, ao)
}

func (m *mockAnalyticsModule) LogCookieSyncObject(cso *analytics.CookieSyncObject) { return }

func (m *mockAnalyticsModule) LogSetUIDObject(so *analytics.SetUIDObject) { return }

func (m *mockAnalyticsModule) LogAmpObject(ao *analytics.AmpObject) { return }

func mockDeps(t *testing.T, ex *mockExchangeVideo) *endpointDeps {
theMetrics := pbsmetrics.NewMetrics(metrics.NewRegistry(), openrtb_ext.BidderList(), config.DisabledMetrics{})
edep := &endpointDeps{
Expand Down

0 comments on commit da23ce7

Please sign in to comment.