From 1f1517e26dba5c1b1dd0426727043677c4af84e1 Mon Sep 17 00:00:00 2001 From: Jim Naumann Date: Mon, 20 May 2019 12:32:08 -0400 Subject: [PATCH] Improved error handling on Beachfront adapter (#873) * Removed a redundant error message that was causing some confusion. * trying to turn '[]' into null and 200 inti 404 * not a goot path * looking at the status codes * I think I have covers all these bases. * checking for empty array response a failing sanely * removed an attempt to pointlessly reset the status code. * corrected and added test cases --- adapters/beachfront/beachfront.go | 57 ++++++++++++--- .../exemplary/minimal-banner.json | 2 +- .../exemplary/simple-video.json | 1 + .../minimal-banner-empty_array-200.json | 72 +++++++++++++++++++ .../supplemental/minimal-mobile-banner.json | 2 +- .../supplemental/minimal-mobile-video.json | 1 + .../supplemental/minimal-site-banner.json | 2 +- .../supplemental/multi-banner.json | 2 +- .../supplemental/multi-mix.json | 1 + .../supplemental/multi-video.json | 1 + 10 files changed, 126 insertions(+), 15 deletions(-) create mode 100644 adapters/beachfront/beachfronttest/supplemental/minimal-banner-empty_array-200.json diff --git a/adapters/beachfront/beachfront.go b/adapters/beachfront/beachfront.go index 81d7371bf69..bfcc7692d54 100644 --- a/adapters/beachfront/beachfront.go +++ b/adapters/beachfront/beachfront.go @@ -3,11 +3,13 @@ package beachfront import ( "encoding/json" "errors" + "fmt" + "github.com/prebid/prebid-server/errortypes" "net/http" + "strconv" "strings" "github.com/prebid/prebid-server/adapters" - "github.com/prebid/prebid-server/errortypes" "github.com/prebid/prebid-server/openrtb_ext" "github.com/mxmCherry/openrtb" @@ -22,7 +24,7 @@ const VideoEndpoint = "https://reachms.bfmio.com/bid.json?exchange_id=" const VideoEndpointSuffix = "&prebidserver" const beachfrontAdapterName = "BF_PREBID_S2S" -const beachfrontAdapterVersion = "0.2.2" +const beachfrontAdapterVersion = "0.3.0" type BeachfrontAdapter struct { } @@ -140,7 +142,7 @@ func (a *BeachfrontAdapter) MakeRequests(request *openrtb.BidRequest) ([]*adapte var beachfrontRequests BeachfrontRequests var reqJSON []byte var uri string - var errs = make([]error, 0) + var errs = make([]error, 0, len(request.Imp)) var err error var imps int @@ -148,7 +150,6 @@ func (a *BeachfrontAdapter) MakeRequests(request *openrtb.BidRequest) ([]*adapte beachfrontRequests, errs, imps = preprocess(request, uri) - // These are fatal errors ------------- if uri == VideoEndpoint { reqJSON, err = json.Marshal(beachfrontRequests.Video) uri = uri + beachfrontRequests.Video.AppId + VideoEndpointSuffix @@ -175,6 +176,15 @@ func (a *BeachfrontAdapter) MakeRequests(request *openrtb.BidRequest) ([]*adapte headers.Add("Content-Type", "application/json;charset=utf-8") headers.Add("Accept", "application/json") + if request.Device != nil { + addHeaderIfNonEmpty(headers, "User-Agent", request.Device.UA) + addHeaderIfNonEmpty(headers, "X-Forwarded-For", request.Device.IP) + addHeaderIfNonEmpty(headers, "Accept-Language", request.Device.Language) + if request.Device.DNT != nil { + addHeaderIfNonEmpty(headers, "DNT", strconv.Itoa(int(*request.Device.DNT))) + } + } + return []*adapters.RequestData{{ Method: "POST", Uri: uri, @@ -288,9 +298,6 @@ func getBannerRequest(req *openrtb.BidRequest) (BeachfrontBannerRequest, []error return beachfrontReq, errs, imps } -/* -Prepare the request that has been received from Prebid.js, translating it to the beachfront format -*/ func getVideoRequest(req *openrtb.BidRequest) (BeachfrontVideoRequest, []error, int) { var videoImpsIndex = 0 var beachfrontReq = NewBeachfrontVideoRequest() @@ -324,7 +331,7 @@ func getVideoRequest(req *openrtb.BidRequest) (BeachfrontVideoRequest, []error, The req could contain banner,audio,native and video imps when It arrives here. I am only interested in video - The beach front video endpoint is only capable of returning a single nurl and price, wrapped in + The beachfront video endpoint is only capable of returning a single nurl and price, wrapped in an openrtb format, so even though I'm building a request here that will include multiple video impressions, only a single URL will be returned. Hopefully the beachfront endpoint can be modified in the future to return multiple video ads @@ -394,12 +401,33 @@ func getVideoRequest(req *openrtb.BidRequest) (BeachfrontVideoRequest, []error, func (a *BeachfrontAdapter) MakeBids(internalRequest *openrtb.BidRequest, externalRequest *adapters.RequestData, response *adapters.ResponseData) (*adapters.BidderResponse, []error) { var bids []openrtb.Bid var bidtype = getBidType(internalRequest) + + /* + Beachfront is now sending an empty array and 200 as their "no results" response. This should catch that. + */ + + if response.StatusCode == http.StatusOK && len(response.Body) <= 2 { + return nil, nil + } + + if response.StatusCode == http.StatusNoContent { + return nil, nil + } + + if response.StatusCode == http.StatusBadRequest { + return nil, []error{&errortypes.BadInput{ + Message: fmt.Sprintf("Unexpected status code: %d. Run with request.debug = 1 for more info", response.StatusCode), + }} + } + + if response.StatusCode != http.StatusOK { + return nil, []error{fmt.Errorf("Unexpected status code: %d. Run with request.debug = 1 for more info", response.StatusCode)} + } + bids, errs := postprocess(response, externalRequest, internalRequest.ID, bidtype) if len(errs) != 0 { - return nil, append(errs, &errortypes.BadServerResponse{ - Message: "Failed to process the beachfront response", - }) + return nil, errs } bidResponse := adapters.NewBidderResponseWithBidsCapacity(BidCapacity) @@ -499,6 +527,13 @@ func extractVideoCrid(nurl string) string { return strings.TrimSuffix(chunky[2], ":") } +// Thank you, brightroll. +func addHeaderIfNonEmpty(headers http.Header, headerName string, headerValue string) { + if len(headerValue) > 0 { + headers.Add(headerName, headerValue) + } +} + func NewBeachfrontBidder() *BeachfrontAdapter { return &BeachfrontAdapter{} } diff --git a/adapters/beachfront/beachfronttest/exemplary/minimal-banner.json b/adapters/beachfront/beachfronttest/exemplary/minimal-banner.json index d0fae5dc3cb..e234d161ca4 100644 --- a/adapters/beachfront/beachfronttest/exemplary/minimal-banner.json +++ b/adapters/beachfront/beachfronttest/exemplary/minimal-banner.json @@ -56,7 +56,7 @@ "dnt": 0, "ua": "", "adapterName": "BF_PREBID_S2S", - "adapterVersion": "0.2.2", + "adapterVersion": "0.3.0", "user": { } } diff --git a/adapters/beachfront/beachfronttest/exemplary/simple-video.json b/adapters/beachfront/beachfronttest/exemplary/simple-video.json index f62942811bd..2d3ff44f7de 100644 --- a/adapters/beachfront/beachfronttest/exemplary/simple-video.json +++ b/adapters/beachfront/beachfronttest/exemplary/simple-video.json @@ -80,6 +80,7 @@ } }, "mockResponse": { + "status" : 200, "body": { "id":"61b87329-8790-47b7-90dd-c53ae7ce1723", "seatBid":[ diff --git a/adapters/beachfront/beachfronttest/supplemental/minimal-banner-empty_array-200.json b/adapters/beachfront/beachfronttest/supplemental/minimal-banner-empty_array-200.json new file mode 100644 index 00000000000..12baf7235d5 --- /dev/null +++ b/adapters/beachfront/beachfronttest/supplemental/minimal-banner-empty_array-200.json @@ -0,0 +1,72 @@ +{ + "mockBidRequest": { + "id": "some_test_ad", + "site": { + "page": "https://test.opposingviews.com/i/society/republican-sen-collins-may-change-vote-tax-bill?cb=1234534" + }, + "imp": [ + { + "bidfloor": 0.02, + "banner": { + "format": [ + { + "w": 300, + "h": 250 + } + ] + }, + "ext": { + "bidder": { + "bidfloor": 0.02, + "appId": "00000000-1111-2222-3333-11111111111" + } + } + } + ] + }, + + "httpCalls": [ + { + "expectedRequest": { + "uri": "https://display.bfmio.com/prebid_display", + "body": { + "slots": [ + { + "slot": "", + "id": "00000000-1111-2222-3333-11111111111", + "bidfloor": 0.02, + "sizes": [ + { + "w": 300, + "h": 250 + } + ] + } + ], + "domain": "test.opposingviews.com", + "page": "https://test.opposingviews.com/i/society/republican-sen-collins-may-change-vote-tax-bill?cb=1234534", + "referrer": "", + "search": "", + "secure": 0, + "requestId": "some_test_ad", + "isMobile": 0, + "ip": "", + "deviceModel": "", + "deviceOs": "", + "dnt": 0, + "ua": "", + "adapterName": "BF_PREBID_S2S", + "adapterVersion": "0.3.0", + "user": { + } + } + }, + "mockResponse": { + "status": 200, + "body": [] + } + } + ], + + "expectedBids": [] +} \ No newline at end of file diff --git a/adapters/beachfront/beachfronttest/supplemental/minimal-mobile-banner.json b/adapters/beachfront/beachfronttest/supplemental/minimal-mobile-banner.json index 1bc5cf7c37f..b73c025c277 100644 --- a/adapters/beachfront/beachfronttest/supplemental/minimal-mobile-banner.json +++ b/adapters/beachfront/beachfronttest/supplemental/minimal-mobile-banner.json @@ -57,7 +57,7 @@ "dnt": 0, "ua": "", "adapterName": "BF_PREBID_S2S", - "adapterVersion": "0.2.2", + "adapterVersion": "0.3.0", "user": { } } diff --git a/adapters/beachfront/beachfronttest/supplemental/minimal-mobile-video.json b/adapters/beachfront/beachfronttest/supplemental/minimal-mobile-video.json index ffef0972011..e752884425e 100644 --- a/adapters/beachfront/beachfronttest/supplemental/minimal-mobile-video.json +++ b/adapters/beachfront/beachfronttest/supplemental/minimal-mobile-video.json @@ -63,6 +63,7 @@ } }, "mockResponse": { + "status": 200, "body": { "id":"61b87329-8790-47b7-90dd-c53ae7ce1723", "seatBid":[ diff --git a/adapters/beachfront/beachfronttest/supplemental/minimal-site-banner.json b/adapters/beachfront/beachfronttest/supplemental/minimal-site-banner.json index eaf4ae0b6c4..fe3982d7302 100644 --- a/adapters/beachfront/beachfronttest/supplemental/minimal-site-banner.json +++ b/adapters/beachfront/beachfronttest/supplemental/minimal-site-banner.json @@ -56,7 +56,7 @@ "dnt": 0, "ua": "", "adapterName": "BF_PREBID_S2S", - "adapterVersion": "0.2.2", + "adapterVersion": "0.3.0", "user": { } } diff --git a/adapters/beachfront/beachfronttest/supplemental/multi-banner.json b/adapters/beachfront/beachfronttest/supplemental/multi-banner.json index aab07059f7a..f18fa6b9e00 100644 --- a/adapters/beachfront/beachfronttest/supplemental/multi-banner.json +++ b/adapters/beachfront/beachfronttest/supplemental/multi-banner.json @@ -96,7 +96,7 @@ "dnt": 1, "ua": "Opera/9.80 (X11; Linux i686; Ubuntu/14.10) Presto/2.12.388 Version/12.16", "adapterName": "BF_PREBID_S2S", - "adapterVersion": "0.2.2", + "adapterVersion": "0.3.0", "user": { "buyeruid": "some-buyer", "id": "some-user" diff --git a/adapters/beachfront/beachfronttest/supplemental/multi-mix.json b/adapters/beachfront/beachfronttest/supplemental/multi-mix.json index 71f36be3e91..297b757a9a3 100644 --- a/adapters/beachfront/beachfronttest/supplemental/multi-mix.json +++ b/adapters/beachfront/beachfronttest/supplemental/multi-mix.json @@ -154,6 +154,7 @@ } }, "mockResponse": { + "status": 200, "body": { "id":"61b87329-8790-47b7-90dd-c53ae7ce1723", "seatBid":[ diff --git a/adapters/beachfront/beachfronttest/supplemental/multi-video.json b/adapters/beachfront/beachfronttest/supplemental/multi-video.json index f4ad38ad105..22f207fb5a8 100644 --- a/adapters/beachfront/beachfronttest/supplemental/multi-video.json +++ b/adapters/beachfront/beachfronttest/supplemental/multi-video.json @@ -106,6 +106,7 @@ } }, "mockResponse": { + "status": 200, "body": { "id":"61b87329-8790-47b7-90dd-c53ae7ce1723", "seatBid":[