Improved error handling on Beachfront adapter (prebid#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
flux-dev-team · May 20, 2019 · f5837cb · f5837cb
1 parent 058c7e3
commit f5837cb
Show file tree

Hide file tree

Showing 10 changed files with 126 additions and 15 deletions.
diff --git 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,15 +142,14 @@ 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
 
 	uri = getEndpoint(request)
 
 	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
@@ -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
@@ -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
@@ -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": []
+}
diff --git 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
@@ -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
@@ -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
@@ -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
@@ -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
@@ -106,6 +106,7 @@
         }
         },
       "mockResponse": {
+        "status": 200,
         "body": {
           "id":"61b87329-8790-47b7-90dd-c53ae7ce1723",
           "seatBid":[