From 905b3a5a29682ed74e9dc5e4c5e2404778ff9f08 Mon Sep 17 00:00:00 2001 From: ShriprasadM Date: Thu, 19 Sep 2024 18:30:20 +0530 Subject: [PATCH] Log non bid reasons in bidder framework (#2891) Co-authored-by: Shriprasad Marathe Co-authored-by: ashish.shinde Co-authored-by: dhruv.sonone --- analytics/pubstack/pubstack_module_test.go | 2 +- exchange/bidder.go | 7 + exchange/bidder_test.go | 146 +++++ exchange/exchange.go | 51 +- exchange/exchange_test.go | 101 ++-- exchange/non_bid_reason.go | 50 +- exchange/non_bid_reason_test.go | 65 +++ exchange/seat_non_bids.go | 56 +- exchange/seat_non_bids_test.go | 529 ++++++++++++++++-- openrtb_ext/response.go | 8 +- stored_requests/data/by_id/accounts/test.json | 2 +- 11 files changed, 859 insertions(+), 158 deletions(-) create mode 100644 exchange/non_bid_reason_test.go diff --git a/analytics/pubstack/pubstack_module_test.go b/analytics/pubstack/pubstack_module_test.go index 911de4c6959..6b16d0b7e92 100644 --- a/analytics/pubstack/pubstack_module_test.go +++ b/analytics/pubstack/pubstack_module_test.go @@ -99,7 +99,7 @@ func TestNewModuleSuccess(t *testing.T) { { ImpId: "123", StatusCode: 34, - Ext: openrtb_ext.NonBidExt{Prebid: openrtb_ext.ExtResponseNonBidPrebid{Bid: openrtb_ext.NonBidObject{}}}, + Ext: &openrtb_ext.NonBidExt{Prebid: openrtb_ext.ExtResponseNonBidPrebid{Bid: openrtb_ext.NonBidObject{}}}, }, }, }, diff --git a/exchange/bidder.go b/exchange/bidder.go index fb2c73be5c2..cfaea36c0bb 100644 --- a/exchange/bidder.go +++ b/exchange/bidder.go @@ -76,12 +76,14 @@ type bidRequestOptions struct { type extraBidderRespInfo struct { respProcessingStartTime time.Time + seatNonBidBuilder SeatNonBidBuilder } type extraAuctionResponseInfo struct { fledge *openrtb_ext.Fledge bidsFound bool bidderResponseStartTime time.Time + seatNonBidBuilder SeatNonBidBuilder } const ImpIdReqBody = "Stored bid response for impression id: " @@ -135,6 +137,7 @@ type bidderAdapterConfig struct { func (bidder *bidderAdapter) requestBid(ctx context.Context, bidderRequest BidderRequest, conversions currency.Conversions, reqInfo *adapters.ExtraRequestInfo, adsCertSigner adscert.Signer, bidRequestOptions bidRequestOptions, alternateBidderCodes openrtb_ext.ExtAlternateBidderCodes, hookExecutor hookexecution.StageExecutor, ruleToAdjustments openrtb_ext.AdjustmentsByDealID) ([]*entities.PbsOrtbSeatBid, extraBidderRespInfo, []error) { request := openrtb_ext.RequestWrapper{BidRequest: bidderRequest.BidRequest} reject := hookExecutor.ExecuteBidderRequestStage(&request, string(bidderRequest.BidderName)) + seatNonBidBuilder := SeatNonBidBuilder{} if reject != nil { return nil, extraBidderRespInfo{}, []error{reject} } @@ -398,13 +401,17 @@ func (bidder *bidderAdapter) requestBid(ctx context.Context, bidderRequest Bidde } } else { errs = append(errs, httpInfo.err) + nonBidReason := httpInfoToNonBidReason(httpInfo) + seatNonBidBuilder.rejectImps(httpInfo.request.ImpIDs, nonBidReason, string(bidderRequest.BidderName)) } } + seatBids := make([]*entities.PbsOrtbSeatBid, 0, len(seatBidMap)) for _, seatBid := range seatBidMap { seatBids = append(seatBids, seatBid) } + extraRespInfo.seatNonBidBuilder = seatNonBidBuilder return seatBids, extraRespInfo, errs } diff --git a/exchange/bidder_test.go b/exchange/bidder_test.go index 31c497d6f1c..9d5faf47549 100644 --- a/exchange/bidder_test.go +++ b/exchange/bidder_test.go @@ -9,11 +9,15 @@ import ( "errors" "fmt" "io" + "net" "net/http" "net/http/httptest" "net/http/httptrace" + "net/url" + "os" "sort" "strings" + "syscall" "testing" "time" @@ -3096,6 +3100,148 @@ func TestGetBidType(t *testing.T) { } } +func TestSeatNonBid(t *testing.T) { + type args struct { + BidRequest *openrtb2.BidRequest + Seat string + SeatRequests []*adapters.RequestData + BidderResponse func() (*http.Response, error) + client *http.Client + } + type expect struct { + seatBids []*entities.PbsOrtbSeatBid + seatNonBids SeatNonBidBuilder + errors []error + } + testCases := []struct { + name string + args args + expect expect + }{ + { + name: "NBR_101_timeout_for_context_deadline_exceeded", + args: args{ + Seat: "pubmatic", + BidRequest: &openrtb2.BidRequest{ + Imp: []openrtb2.Imp{{ID: "1234"}}, + }, + SeatRequests: []*adapters.RequestData{{ImpIDs: []string{"1234"}}}, + BidderResponse: func() (*http.Response, error) { return nil, context.DeadlineExceeded }, + client: &http.Client{Timeout: time.Nanosecond}, // for timeout + }, + expect: expect{ + seatNonBids: SeatNonBidBuilder{ + "pubmatic": {{ + ImpId: "1234", + StatusCode: int(ErrorTimeout), + }}, + }, + errors: []error{&errortypes.Timeout{Message: context.DeadlineExceeded.Error()}}, + seatBids: []*entities.PbsOrtbSeatBid{{Bids: []*entities.PbsOrtbBid{}, Currency: "USD", Seat: "pubmatic", HttpCalls: []*openrtb_ext.ExtHttpCall{}}}, + }, + }, { + name: "NBR_103_Bidder_Unreachable_Connection_Refused", + args: args{ + Seat: "appnexus", + SeatRequests: []*adapters.RequestData{{ImpIDs: []string{"1234", "4567"}}}, + BidRequest: &openrtb2.BidRequest{Imp: []openrtb2.Imp{{ID: "1234"}, {ID: "4567"}}}, + BidderResponse: func() (*http.Response, error) { + return nil, &net.OpError{Err: os.NewSyscallError(syscall.ECONNREFUSED.Error(), syscall.ECONNREFUSED)} + }, + }, + expect: expect{ + seatNonBids: SeatNonBidBuilder{ + "appnexus": { + {ImpId: "1234", StatusCode: int(ErrorBidderUnreachable)}, + {ImpId: "4567", StatusCode: int(ErrorBidderUnreachable)}, + }, + }, + seatBids: []*entities.PbsOrtbSeatBid{{Bids: []*entities.PbsOrtbBid{}, Currency: "USD", Seat: "appnexus", HttpCalls: []*openrtb_ext.ExtHttpCall{}}}, + errors: []error{&url.Error{Op: "Get", URL: "", Err: &net.OpError{Err: os.NewSyscallError(syscall.ECONNREFUSED.Error(), syscall.ECONNREFUSED)}}}, + }, + }, { + name: "no_impids_populated_in_request_data", + args: args{ + SeatRequests: []*adapters.RequestData{{ + ImpIDs: nil, // no imp ids + }}, + BidRequest: &openrtb2.BidRequest{Imp: []openrtb2.Imp{{ID: "1234"}}}, + BidderResponse: func() (*http.Response, error) { + return nil, errors.New("some_error") + }, + }, + expect: expect{ + seatNonBids: SeatNonBidBuilder{}, + seatBids: []*entities.PbsOrtbSeatBid{{Bids: []*entities.PbsOrtbBid{}, Currency: "USD", HttpCalls: []*openrtb_ext.ExtHttpCall{}}}, + errors: []error{&url.Error{Op: "Get", URL: "", Err: errors.New("some_error")}}, + }, + }, + } + for _, test := range testCases { + t.Run(test.name, func(t *testing.T) { + mockBidder := &mockBidder{} + mockBidder.On("MakeRequests", mock.Anything, mock.Anything).Return(test.args.SeatRequests, []error(nil)) + mockMetricsEngine := &metrics.MetricsEngineMock{} + mockMetricsEngine.On("RecordOverheadTime", mock.Anything, mock.Anything).Return(nil) + mockMetricsEngine.On("RecordBidderServerResponseTime", mock.Anything).Return(nil) + roundTrip := &mockRoundTripper{} + roundTrip.On("RoundTrip", mock.Anything).Return(test.args.BidderResponse()) + client := &http.Client{ + Transport: roundTrip, + Timeout: 0, + } + if test.args.client != nil { + client.Timeout = test.args.client.Timeout + } + bidder := AdaptBidder(mockBidder, client, &config.Configuration{}, mockMetricsEngine, openrtb_ext.BidderAppnexus, &config.DebugInfo{}, test.args.Seat) + + ctx := context.Background() + if client.Timeout > 0 { + ctxTimeout, cancel := context.WithTimeout(ctx, client.Timeout) + ctx = ctxTimeout + defer cancel() + } + seatBids, responseExtra, errors := bidder.requestBid(ctx, BidderRequest{ + BidRequest: test.args.BidRequest, + BidderName: openrtb_ext.BidderName(test.args.Seat), + }, nil, &adapters.ExtraRequestInfo{}, &MockSigner{}, bidRequestOptions{}, openrtb_ext.ExtAlternateBidderCodes{}, hookexecution.EmptyHookExecutor{}, nil) + assert.Equal(t, test.expect.seatBids, seatBids) + assert.Equal(t, test.expect.seatNonBids, responseExtra.seatNonBidBuilder) + assert.Equal(t, test.expect.errors, errors) + for _, nonBids := range responseExtra.seatNonBidBuilder { + for _, nonBid := range nonBids { + for _, seatBid := range seatBids { + for _, bid := range seatBid.Bids { + // ensure non bids are not present in seat bids + if nonBid.ImpId == bid.Bid.ImpID { + assert.Fail(t, "imp id [%s] present in both seat bid and non seat bid", nonBid.ImpId) + } + } + } + } + } + }) + } +} + +type mockRoundTripper struct { + mock.Mock +} + +func (rt *mockRoundTripper) RoundTrip(request *http.Request) (*http.Response, error) { + args := rt.Called(request) + var response *http.Response + if args.Get(0) != nil { + response = args.Get(0).(*http.Response) + } + var err error + if args.Get(1) != nil { + err = args.Get(1).(error) + } + + return response, err +} + type mockBidderTmaxCtx struct { startTime, deadline, now time.Time ok bool diff --git a/exchange/exchange.go b/exchange/exchange.go index 5c27b0d3c5a..a8787ac3db2 100644 --- a/exchange/exchange.go +++ b/exchange/exchange.go @@ -95,6 +95,8 @@ type seatResponseExtra struct { // httpCalls is the list of debugging info. It should only be populated if the request.test == 1. // This will become response.ext.debug.httpcalls.{bidder} on the final Response. HttpCalls []*openrtb_ext.ExtHttpCall + // NonBid contains non bid reason information + NonBid *openrtb_ext.NonBid } type bidResponseWrapper struct { @@ -103,6 +105,7 @@ type bidResponseWrapper struct { bidder openrtb_ext.BidderName adapter openrtb_ext.BidderName bidderResponseStartTime time.Time + seatNonBidBuilder SeatNonBidBuilder } type BidIDGenerator interface { @@ -373,7 +376,8 @@ func (e *exchange) HoldAuction(ctx context.Context, r *AuctionRequest, debugLog fledge *openrtb_ext.Fledge anyBidsReturned bool // List of bidders we have requests for. - liveAdapters []openrtb_ext.BidderName + liveAdapters []openrtb_ext.BidderName + seatNonBidBuilder SeatNonBidBuilder = SeatNonBidBuilder{} ) if len(r.StoredAuctionResponses) > 0 { @@ -399,13 +403,15 @@ func (e *exchange) HoldAuction(ctx context.Context, r *AuctionRequest, debugLog fledge = extraRespInfo.fledge anyBidsReturned = extraRespInfo.bidsFound r.BidderResponseStartTime = extraRespInfo.bidderResponseStartTime + if extraRespInfo.seatNonBidBuilder != nil { + seatNonBidBuilder = extraRespInfo.seatNonBidBuilder + } } var ( auc *auction cacheErrs []error bidResponseExt *openrtb_ext.ExtBidResponse - seatNonBids = nonBids{} ) if anyBidsReturned { @@ -423,7 +429,7 @@ func (e *exchange) HoldAuction(ctx context.Context, r *AuctionRequest, debugLog if rejectedBid.Bids[0].Bid.DealID != "" { rejectionReason = ResponseRejectedBelowDealFloor } - seatNonBids.addBid(rejectedBid.Bids[0], int(rejectionReason), rejectedBid.Seat) + seatNonBidBuilder.rejectBid(rejectedBid.Bids[0], int(rejectionReason), rejectedBid.Seat) } } @@ -431,7 +437,7 @@ func (e *exchange) HoldAuction(ctx context.Context, r *AuctionRequest, debugLog //If includebrandcategory is present in ext then CE feature is on. if requestExtPrebid.Targeting != nil && requestExtPrebid.Targeting.IncludeBrandCategory != nil { var rejections []string - bidCategory, adapterBids, rejections, err = applyCategoryMapping(ctx, *requestExtPrebid.Targeting, adapterBids, e.categoriesFetcher, targData, &randomDeduplicateBidBooleanGenerator{}, &seatNonBids) + bidCategory, adapterBids, rejections, err = applyCategoryMapping(ctx, *requestExtPrebid.Targeting, adapterBids, e.categoriesFetcher, targData, &randomDeduplicateBidBooleanGenerator{}, &seatNonBidBuilder) if err != nil { return nil, fmt.Errorf("Error in category mapping : %s", err.Error()) } @@ -526,14 +532,14 @@ func (e *exchange) HoldAuction(ctx context.Context, r *AuctionRequest, debugLog e.bidValidationEnforcement.SetBannerCreativeMaxSize(r.Account.Validations) // Build the response - bidResponse := e.buildBidResponse(ctx, liveAdapters, adapterBids, r.BidRequestWrapper, adapterExtra, auc, bidResponseExt, cacheInstructions.returnCreative, r.ImpExtInfoMap, r.PubID, errs, &seatNonBids) + bidResponse := e.buildBidResponse(ctx, liveAdapters, adapterBids, r.BidRequestWrapper, adapterExtra, auc, bidResponseExt, cacheInstructions.returnCreative, r.ImpExtInfoMap, r.PubID, errs, &seatNonBidBuilder) bidResponse = adservertargeting.Apply(r.BidRequestWrapper, r.ResolvedBidRequest, bidResponse, r.QueryParams, bidResponseExt, r.Account.TruncateTargetAttribute) bidResponse.Ext, err = encodeBidResponseExt(bidResponseExt) if err != nil { return nil, err } - bidResponseExt = setSeatNonBid(bidResponseExt, seatNonBids) + bidResponseExt = setSeatNonBid(bidResponseExt, seatNonBidBuilder) return &AuctionResponse{ BidResponse: bidResponse, @@ -713,7 +719,7 @@ func (e *exchange) getAllBids( adapterBids := make(map[openrtb_ext.BidderName]*entities.PbsOrtbSeatBid, len(bidderRequests)) adapterExtra := make(map[openrtb_ext.BidderName]*seatResponseExtra, len(bidderRequests)) chBids := make(chan *bidResponseWrapper, len(bidderRequests)) - extraRespInfo := extraAuctionResponseInfo{} + extraRespInfo := extraAuctionResponseInfo{seatNonBidBuilder: SeatNonBidBuilder{}} e.me.RecordOverheadTime(metrics.MakeBidderRequests, time.Since(pbsRequestStartTime)) @@ -753,6 +759,7 @@ func (e *exchange) getAllBids( // Add in time reporting elapsed := time.Since(start) brw.adapterSeatBids = seatBids + brw.seatNonBidBuilder = extraBidderRespInfo.seatNonBidBuilder // Structure to record extra tracking data generated during bidding ae := new(seatResponseExtra) ae.ResponseTimeMillis = int(elapsed / time.Millisecond) @@ -805,6 +812,10 @@ func (e *exchange) getAllBids( } //but we need to add all bidders data to adapterExtra to have metrics and other metadata adapterExtra[brw.bidder] = brw.adapterExtra + + // collect adapter non bids + extraRespInfo.seatNonBidBuilder.append(brw.seatNonBidBuilder) + } return adapterBids, adapterExtra, extraRespInfo @@ -922,7 +933,7 @@ func errsToBidderWarnings(errs []error) []openrtb_ext.ExtBidderMessage { } // This piece takes all the bids supplied by the adapters and crafts an openRTB response to send back to the requester -func (e *exchange) buildBidResponse(ctx context.Context, liveAdapters []openrtb_ext.BidderName, adapterSeatBids map[openrtb_ext.BidderName]*entities.PbsOrtbSeatBid, bidRequest *openrtb_ext.RequestWrapper, adapterExtra map[openrtb_ext.BidderName]*seatResponseExtra, auc *auction, bidResponseExt *openrtb_ext.ExtBidResponse, returnCreative bool, impExtInfoMap map[string]ImpExtInfo, pubID string, errList []error, seatNonBids *nonBids) *openrtb2.BidResponse { +func (e *exchange) buildBidResponse(ctx context.Context, liveAdapters []openrtb_ext.BidderName, adapterSeatBids map[openrtb_ext.BidderName]*entities.PbsOrtbSeatBid, bidRequest *openrtb_ext.RequestWrapper, adapterExtra map[openrtb_ext.BidderName]*seatResponseExtra, auc *auction, bidResponseExt *openrtb_ext.ExtBidResponse, returnCreative bool, impExtInfoMap map[string]ImpExtInfo, pubID string, errList []error, seatNonBidBuilder *SeatNonBidBuilder) *openrtb2.BidResponse { bidResponse := new(openrtb2.BidResponse) bidResponse.ID = bidRequest.ID @@ -937,7 +948,7 @@ func (e *exchange) buildBidResponse(ctx context.Context, liveAdapters []openrtb_ for a, adapterSeatBids := range adapterSeatBids { //while processing every single bib, do we need to handle categories here? if adapterSeatBids != nil && len(adapterSeatBids.Bids) > 0 { - sb := e.makeSeatBid(adapterSeatBids, a, adapterExtra, auc, returnCreative, impExtInfoMap, bidRequest, bidResponseExt, pubID, seatNonBids) + sb := e.makeSeatBid(adapterSeatBids, a, adapterExtra, auc, returnCreative, impExtInfoMap, bidRequest, bidResponseExt, pubID, seatNonBidBuilder) seatBids = append(seatBids, *sb) bidResponse.Cur = adapterSeatBids.Currency } @@ -957,7 +968,7 @@ func encodeBidResponseExt(bidResponseExt *openrtb_ext.ExtBidResponse) ([]byte, e return buffer.Bytes(), err } -func applyCategoryMapping(ctx context.Context, targeting openrtb_ext.ExtRequestTargeting, seatBids map[openrtb_ext.BidderName]*entities.PbsOrtbSeatBid, categoriesFetcher stored_requests.CategoryFetcher, targData *targetData, booleanGenerator deduplicateChanceGenerator, seatNonBids *nonBids) (map[string]string, map[openrtb_ext.BidderName]*entities.PbsOrtbSeatBid, []string, error) { +func applyCategoryMapping(ctx context.Context, targeting openrtb_ext.ExtRequestTargeting, seatBids map[openrtb_ext.BidderName]*entities.PbsOrtbSeatBid, categoriesFetcher stored_requests.CategoryFetcher, targData *targetData, booleanGenerator deduplicateChanceGenerator, seatNonBidBuilder *SeatNonBidBuilder) (map[string]string, map[openrtb_ext.BidderName]*entities.PbsOrtbSeatBid, []string, error) { res := make(map[string]string) type bidDedupe struct { @@ -1019,7 +1030,7 @@ func applyCategoryMapping(ctx context.Context, targeting openrtb_ext.ExtRequestT //on receiving bids from adapters if no unique IAB category is returned or if no ad server category is returned discard the bid bidsToRemove = append(bidsToRemove, bidInd) rejections = updateRejections(rejections, bidID, "Bid did not contain a category") - seatNonBids.addBid(bid, int(ResponseRejectedCategoryMappingInvalid), string(bidderName)) + seatNonBidBuilder.rejectBid(bid, int(ResponseRejectedCategoryMappingInvalid), string(bidderName)) continue } if translateCategories { @@ -1256,14 +1267,14 @@ func (e *exchange) makeExtBidResponse(adapterBids map[openrtb_ext.BidderName]*en // Return an openrtb seatBid for a bidder // buildBidResponse is responsible for ensuring nil bid seatbids are not included -func (e *exchange) makeSeatBid(adapterBid *entities.PbsOrtbSeatBid, adapter openrtb_ext.BidderName, adapterExtra map[openrtb_ext.BidderName]*seatResponseExtra, auc *auction, returnCreative bool, impExtInfoMap map[string]ImpExtInfo, bidRequest *openrtb_ext.RequestWrapper, bidResponseExt *openrtb_ext.ExtBidResponse, pubID string, seatNonBids *nonBids) *openrtb2.SeatBid { +func (e *exchange) makeSeatBid(adapterBid *entities.PbsOrtbSeatBid, adapter openrtb_ext.BidderName, adapterExtra map[openrtb_ext.BidderName]*seatResponseExtra, auc *auction, returnCreative bool, impExtInfoMap map[string]ImpExtInfo, bidRequest *openrtb_ext.RequestWrapper, bidResponseExt *openrtb_ext.ExtBidResponse, pubID string, seatNonBidBuilder *SeatNonBidBuilder) *openrtb2.SeatBid { seatBid := &openrtb2.SeatBid{ Seat: adapter.String(), Group: 0, // Prebid cannot support roadblocking } var errList []error - seatBid.Bid, errList = e.makeBid(adapterBid.Bids, auc, returnCreative, impExtInfoMap, bidRequest, bidResponseExt, adapter, pubID, seatNonBids) + seatBid.Bid, errList = e.makeBid(adapterBid.Bids, auc, returnCreative, impExtInfoMap, bidRequest, bidResponseExt, adapter, pubID, seatNonBidBuilder) if len(errList) > 0 { adapterExtra[adapter].Errors = append(adapterExtra[adapter].Errors, errsToBidderErrors(errList)...) } @@ -1271,7 +1282,7 @@ func (e *exchange) makeSeatBid(adapterBid *entities.PbsOrtbSeatBid, adapter open return seatBid } -func (e *exchange) makeBid(bids []*entities.PbsOrtbBid, auc *auction, returnCreative bool, impExtInfoMap map[string]ImpExtInfo, bidRequest *openrtb_ext.RequestWrapper, bidResponseExt *openrtb_ext.ExtBidResponse, adapter openrtb_ext.BidderName, pubID string, seatNonBids *nonBids) ([]openrtb2.Bid, []error) { +func (e *exchange) makeBid(bids []*entities.PbsOrtbBid, auc *auction, returnCreative bool, impExtInfoMap map[string]ImpExtInfo, bidRequest *openrtb_ext.RequestWrapper, bidResponseExt *openrtb_ext.ExtBidResponse, adapter openrtb_ext.BidderName, pubID string, seatNonBidBuilder *SeatNonBidBuilder) ([]openrtb2.Bid, []error) { result := make([]openrtb2.Bid, 0, len(bids)) errs := make([]error, 0, 1) @@ -1283,12 +1294,12 @@ func (e *exchange) makeBid(bids []*entities.PbsOrtbBid, auc *auction, returnCrea } bidResponseExt.Warnings[adapter] = append(bidResponseExt.Warnings[adapter], dsaMessage) - seatNonBids.addBid(bid, int(ResponseRejectedGeneral), adapter.String()) + seatNonBidBuilder.rejectBid(bid, int(ResponseRejectedGeneral), adapter.String()) continue // Don't add bid to result } if e.bidValidationEnforcement.BannerCreativeMaxSize == config.ValidationEnforce && bid.BidType == openrtb_ext.BidTypeBanner { if !e.validateBannerCreativeSize(bid, bidResponseExt, adapter, pubID, e.bidValidationEnforcement.BannerCreativeMaxSize) { - seatNonBids.addBid(bid, int(ResponseRejectedCreativeSizeNotAllowed), adapter.String()) + seatNonBidBuilder.rejectBid(bid, int(ResponseRejectedCreativeSizeNotAllowed), adapter.String()) continue // Don't add bid to result } } else if e.bidValidationEnforcement.BannerCreativeMaxSize == config.ValidationWarn && bid.BidType == openrtb_ext.BidTypeBanner { @@ -1297,7 +1308,7 @@ func (e *exchange) makeBid(bids []*entities.PbsOrtbBid, auc *auction, returnCrea if _, ok := impExtInfoMap[bid.Bid.ImpID]; ok { if e.bidValidationEnforcement.SecureMarkup == config.ValidationEnforce && (bid.BidType == openrtb_ext.BidTypeBanner || bid.BidType == openrtb_ext.BidTypeVideo) { if !e.validateBidAdM(bid, bidResponseExt, adapter, pubID, e.bidValidationEnforcement.SecureMarkup) { - seatNonBids.addBid(bid, int(ResponseRejectedCreativeNotSecure), adapter.String()) + seatNonBidBuilder.rejectBid(bid, int(ResponseRejectedCreativeNotSecure), adapter.String()) continue // Don't add bid to result } } else if e.bidValidationEnforcement.SecureMarkup == config.ValidationWarn && (bid.BidType == openrtb_ext.BidTypeBanner || bid.BidType == openrtb_ext.BidTypeVideo) { @@ -1597,8 +1608,8 @@ func setErrorMessageSecureMarkup(validationType string) string { } // setSeatNonBid adds SeatNonBids within bidResponse.Ext.Prebid.SeatNonBid -func setSeatNonBid(bidResponseExt *openrtb_ext.ExtBidResponse, seatNonBids nonBids) *openrtb_ext.ExtBidResponse { - if len(seatNonBids.seatNonBidsMap) == 0 { +func setSeatNonBid(bidResponseExt *openrtb_ext.ExtBidResponse, seatNonBidBuilder SeatNonBidBuilder) *openrtb_ext.ExtBidResponse { + if len(seatNonBidBuilder) == 0 { return bidResponseExt } if bidResponseExt == nil { @@ -1608,6 +1619,6 @@ func setSeatNonBid(bidResponseExt *openrtb_ext.ExtBidResponse, seatNonBids nonBi bidResponseExt.Prebid = &openrtb_ext.ExtResponsePrebid{} } - bidResponseExt.Prebid.SeatNonBid = seatNonBids.get() + bidResponseExt.Prebid.SeatNonBid = seatNonBidBuilder.Slice() return bidResponseExt } diff --git a/exchange/exchange_test.go b/exchange/exchange_test.go index aec062b70c6..df272a1f5bf 100644 --- a/exchange/exchange_test.go +++ b/exchange/exchange_test.go @@ -172,7 +172,7 @@ func TestCharacterEscape(t *testing.T) { var errList []error // 4) Build bid response - bidResp := e.buildBidResponse(context.Background(), liveAdapters, adapterBids, bidRequest, adapterExtra, nil, nil, true, nil, "", errList, &nonBids{}) + bidResp := e.buildBidResponse(context.Background(), liveAdapters, adapterBids, bidRequest, adapterExtra, nil, nil, true, nil, "", errList, &SeatNonBidBuilder{}) // 5) Assert we have no errors and one '&' character as we are supposed to if len(errList) > 0 { @@ -1343,7 +1343,7 @@ func TestGetBidCacheInfoEndToEnd(t *testing.T) { var errList []error // 4) Build bid response - bid_resp := e.buildBidResponse(context.Background(), liveAdapters, adapterBids, bidRequest, adapterExtra, auc, nil, true, nil, "", errList, &nonBids{}) + bid_resp := e.buildBidResponse(context.Background(), liveAdapters, adapterBids, bidRequest, adapterExtra, auc, nil, true, nil, "", errList, &SeatNonBidBuilder{}) expectedBidResponse := &openrtb2.BidResponse{ SeatBid: []openrtb2.SeatBid{ @@ -1433,7 +1433,7 @@ func TestBidReturnsCreative(t *testing.T) { //Run tests for _, test := range testCases { - resultingBids, resultingErrs := e.makeBid(sampleBids, sampleAuction, test.inReturnCreative, nil, &openrtb_ext.RequestWrapper{}, nil, "", "", &nonBids{}) + resultingBids, resultingErrs := e.makeBid(sampleBids, sampleAuction, test.inReturnCreative, nil, &openrtb_ext.RequestWrapper{}, nil, "", "", &SeatNonBidBuilder{}) assert.Equal(t, 0, len(resultingErrs), "%s. Test should not return errors \n", test.description) assert.Equal(t, test.expectedCreativeMarkup, resultingBids[0].AdM, "%s. Ad markup string doesn't match expected \n", test.description) @@ -1718,7 +1718,7 @@ func TestBidResponseCurrency(t *testing.T) { } // Run tests for i := range testCases { - actualBidResp := e.buildBidResponse(context.Background(), liveAdapters, testCases[i].adapterBids, bidRequest, adapterExtra, nil, bidResponseExt, true, nil, "", errList, &nonBids{}) + actualBidResp := e.buildBidResponse(context.Background(), liveAdapters, testCases[i].adapterBids, bidRequest, adapterExtra, nil, bidResponseExt, true, nil, "", errList, &SeatNonBidBuilder{}) assert.Equalf(t, testCases[i].expectedBidResponse, actualBidResp, fmt.Sprintf("[TEST_FAILED] Objects must be equal for test: %s \n Expected: >>%s<< \n Actual: >>%s<< ", testCases[i].description, testCases[i].expectedBidResponse.Ext, actualBidResp.Ext)) } } @@ -1786,7 +1786,7 @@ func TestBidResponseImpExtInfo(t *testing.T) { expectedBidResponseExt := `{"origbidcpm":0,"prebid":{"meta":{"adaptercode":"appnexus"},"type":"video","passthrough":{"imp_passthrough_val":1}},"storedrequestattributes":{"h":480,"mimes":["video/mp4"]}}` - actualBidResp := e.buildBidResponse(context.Background(), liveAdapters, adapterBids, bidRequest, nil, nil, nil, true, impExtInfo, "", errList, &nonBids{}) + actualBidResp := e.buildBidResponse(context.Background(), liveAdapters, adapterBids, bidRequest, nil, nil, nil, true, impExtInfo, "", errList, &SeatNonBidBuilder{}) resBidExt := string(actualBidResp.SeatBid[0].Bid[0].Ext) assert.Equalf(t, expectedBidResponseExt, resBidExt, "Expected bid response extension is incorrect") @@ -2607,7 +2607,7 @@ func TestCategoryMapping(t *testing.T) { adapterBids[bidderName1] = &seatBid - bidCategory, adapterBids, rejections, err := applyCategoryMapping(context.TODO(), *requestExt.Prebid.Targeting, adapterBids, categoriesFetcher, targData, &randomDeduplicateBidBooleanGenerator{}, &nonBids{}) + bidCategory, adapterBids, rejections, err := applyCategoryMapping(context.TODO(), *requestExt.Prebid.Targeting, adapterBids, categoriesFetcher, targData, &randomDeduplicateBidBooleanGenerator{}, &SeatNonBidBuilder{}) assert.Equal(t, nil, err, "Category mapping error should be empty") assert.Equal(t, 1, len(rejections), "There should be 1 bid rejection message") @@ -2662,7 +2662,7 @@ func TestCategoryMappingNoIncludeBrandCategory(t *testing.T) { adapterBids[bidderName1] = &seatBid - bidCategory, adapterBids, rejections, err := applyCategoryMapping(context.TODO(), *requestExt.Prebid.Targeting, adapterBids, categoriesFetcher, targData, &randomDeduplicateBidBooleanGenerator{}, &nonBids{}) + bidCategory, adapterBids, rejections, err := applyCategoryMapping(context.TODO(), *requestExt.Prebid.Targeting, adapterBids, categoriesFetcher, targData, &randomDeduplicateBidBooleanGenerator{}, &SeatNonBidBuilder{}) assert.Equal(t, nil, err, "Category mapping error should be empty") assert.Empty(t, rejections, "There should be no bid rejection messages") @@ -2714,7 +2714,7 @@ func TestCategoryMappingTranslateCategoriesNil(t *testing.T) { adapterBids[bidderName1] = &seatBid - bidCategory, adapterBids, rejections, err := applyCategoryMapping(context.TODO(), *requestExt.Prebid.Targeting, adapterBids, categoriesFetcher, targData, &randomDeduplicateBidBooleanGenerator{}, &nonBids{}) + bidCategory, adapterBids, rejections, err := applyCategoryMapping(context.TODO(), *requestExt.Prebid.Targeting, adapterBids, categoriesFetcher, targData, &randomDeduplicateBidBooleanGenerator{}, &SeatNonBidBuilder{}) assert.Equal(t, nil, err, "Category mapping error should be empty") assert.Equal(t, 1, len(rejections), "There should be 1 bid rejection message") @@ -2796,7 +2796,7 @@ func TestCategoryMappingTranslateCategoriesFalse(t *testing.T) { adapterBids[bidderName1] = &seatBid - bidCategory, adapterBids, rejections, err := applyCategoryMapping(context.TODO(), *requestExt.Prebid.Targeting, adapterBids, categoriesFetcher, targData, &randomDeduplicateBidBooleanGenerator{}, &nonBids{}) + bidCategory, adapterBids, rejections, err := applyCategoryMapping(context.TODO(), *requestExt.Prebid.Targeting, adapterBids, categoriesFetcher, targData, &randomDeduplicateBidBooleanGenerator{}, &SeatNonBidBuilder{}) assert.Equal(t, nil, err, "Category mapping error should be empty") assert.Empty(t, rejections, "There should be no bid rejection messages") @@ -2874,7 +2874,7 @@ func TestCategoryDedupe(t *testing.T) { }, } deduplicateGenerator := fakeBooleanGenerator{value: tt.dedupeGeneratorValue} - bidCategory, adapterBids, rejections, err := applyCategoryMapping(context.TODO(), *requestExt.Prebid.Targeting, adapterBids, categoriesFetcher, targData, &deduplicateGenerator, &nonBids{}) + bidCategory, adapterBids, rejections, err := applyCategoryMapping(context.TODO(), *requestExt.Prebid.Targeting, adapterBids, categoriesFetcher, targData, &deduplicateGenerator, &SeatNonBidBuilder{}) assert.Nil(t, err) assert.Equal(t, 3, len(rejections)) @@ -2943,7 +2943,7 @@ func TestNoCategoryDedupe(t *testing.T) { adapterBids[bidderName1] = &seatBid - bidCategory, adapterBids, rejections, err := applyCategoryMapping(context.TODO(), *requestExt.Prebid.Targeting, adapterBids, categoriesFetcher, targData, &randomDeduplicateBidBooleanGenerator{}, &nonBids{}) + bidCategory, adapterBids, rejections, err := applyCategoryMapping(context.TODO(), *requestExt.Prebid.Targeting, adapterBids, categoriesFetcher, targData, &randomDeduplicateBidBooleanGenerator{}, &SeatNonBidBuilder{}) assert.Equal(t, nil, err, "Category mapping error should be empty") assert.Equal(t, 2, len(rejections), "There should be 2 bid rejection messages") @@ -3008,7 +3008,7 @@ func TestCategoryMappingBidderName(t *testing.T) { adapterBids[bidderName1] = &seatBid1 adapterBids[bidderName2] = &seatBid2 - bidCategory, adapterBids, rejections, err := applyCategoryMapping(context.TODO(), *requestExt.Prebid.Targeting, adapterBids, categoriesFetcher, targData, &randomDeduplicateBidBooleanGenerator{}, &nonBids{}) + bidCategory, adapterBids, rejections, err := applyCategoryMapping(context.TODO(), *requestExt.Prebid.Targeting, adapterBids, categoriesFetcher, targData, &randomDeduplicateBidBooleanGenerator{}, &SeatNonBidBuilder{}) assert.NoError(t, err, "Category mapping error should be empty") assert.Empty(t, rejections, "There should be 0 bid rejection messages") @@ -3062,7 +3062,7 @@ func TestCategoryMappingBidderNameNoCategories(t *testing.T) { adapterBids[bidderName1] = &seatBid1 adapterBids[bidderName2] = &seatBid2 - bidCategory, adapterBids, rejections, err := applyCategoryMapping(context.TODO(), *requestExt.Prebid.Targeting, adapterBids, categoriesFetcher, targData, &randomDeduplicateBidBooleanGenerator{}, &nonBids{}) + bidCategory, adapterBids, rejections, err := applyCategoryMapping(context.TODO(), *requestExt.Prebid.Targeting, adapterBids, categoriesFetcher, targData, &randomDeduplicateBidBooleanGenerator{}, &SeatNonBidBuilder{}) assert.NoError(t, err, "Category mapping error should be empty") assert.Empty(t, rejections, "There should be 0 bid rejection messages") @@ -3163,7 +3163,7 @@ func TestBidRejectionErrors(t *testing.T) { adapterBids[bidderName] = &seatBid - bidCategory, adapterBids, rejections, err := applyCategoryMapping(context.TODO(), *test.reqExt.Prebid.Targeting, adapterBids, categoriesFetcher, targData, &randomDeduplicateBidBooleanGenerator{}, &nonBids{}) + bidCategory, adapterBids, rejections, err := applyCategoryMapping(context.TODO(), *test.reqExt.Prebid.Targeting, adapterBids, categoriesFetcher, targData, &randomDeduplicateBidBooleanGenerator{}, &SeatNonBidBuilder{}) if len(test.expectedCatDur) > 0 { // Bid deduplication case @@ -3226,7 +3226,7 @@ func TestCategoryMappingTwoBiddersOneBidEachNoCategorySamePrice(t *testing.T) { adapterBids[bidderNameApn1] = &seatBidApn1 adapterBids[bidderNameApn2] = &seatBidApn2 - bidCategory, _, rejections, err := applyCategoryMapping(context.TODO(), *requestExt.Prebid.Targeting, adapterBids, categoriesFetcher, targData, &randomDeduplicateBidBooleanGenerator{}, &nonBids{}) + bidCategory, _, rejections, err := applyCategoryMapping(context.TODO(), *requestExt.Prebid.Targeting, adapterBids, categoriesFetcher, targData, &randomDeduplicateBidBooleanGenerator{}, &SeatNonBidBuilder{}) assert.NoError(t, err, "Category mapping error should be empty") assert.Len(t, rejections, 1, "There should be 1 bid rejection message") @@ -3310,7 +3310,7 @@ func TestCategoryMappingTwoBiddersManyBidsEachNoCategorySamePrice(t *testing.T) adapterBids[bidderNameApn1] = &seatBidApn1 adapterBids[bidderNameApn2] = &seatBidApn2 - _, adapterBids, rejections, err := applyCategoryMapping(context.TODO(), *requestExt.Prebid.Targeting, adapterBids, categoriesFetcher, targData, &fakeBooleanGenerator{value: true}, &nonBids{}) + _, adapterBids, rejections, err := applyCategoryMapping(context.TODO(), *requestExt.Prebid.Targeting, adapterBids, categoriesFetcher, targData, &fakeBooleanGenerator{value: true}, &SeatNonBidBuilder{}) assert.NoError(t, err, "Category mapping error should be empty") @@ -4774,7 +4774,7 @@ func TestMakeBidWithValidation(t *testing.T) { givenBids []*entities.PbsOrtbBid givenSeat openrtb_ext.BidderName expectedNumOfBids int - expectedNonBids *nonBids + expectedNonBids *SeatNonBidBuilder expectedNumDebugErrors int expectedNumDebugWarnings int }{ @@ -4785,15 +4785,13 @@ func TestMakeBidWithValidation(t *testing.T) { givenBids: []*entities.PbsOrtbBid{{Bid: &openrtb2.Bid{Ext: json.RawMessage(`{"dsa": {"adrender":1}}`)}}, {Bid: &openrtb2.Bid{}}}, givenSeat: "pubmatic", expectedNumOfBids: 1, - expectedNonBids: &nonBids{ - seatNonBidsMap: map[string][]openrtb_ext.NonBid{ - "pubmatic": { - { - StatusCode: 300, - Ext: openrtb_ext.NonBidExt{ - Prebid: openrtb_ext.ExtResponseNonBidPrebid{ - Bid: openrtb_ext.NonBidObject{}, - }, + expectedNonBids: &SeatNonBidBuilder{ + "pubmatic": { + { + StatusCode: 300, + Ext: &openrtb_ext.NonBidExt{ + Prebid: openrtb_ext.ExtResponseNonBidPrebid{ + Bid: openrtb_ext.NonBidObject{}, }, }, }, @@ -4807,17 +4805,15 @@ func TestMakeBidWithValidation(t *testing.T) { givenBids: []*entities.PbsOrtbBid{{Bid: &openrtb2.Bid{W: 200, H: 200}, BidType: openrtb_ext.BidTypeBanner}, {Bid: &openrtb2.Bid{W: 50, H: 50}, BidType: openrtb_ext.BidTypeBanner}}, givenSeat: "pubmatic", expectedNumOfBids: 1, - expectedNonBids: &nonBids{ - seatNonBidsMap: map[string][]openrtb_ext.NonBid{ - "pubmatic": { - { - StatusCode: 351, - Ext: openrtb_ext.NonBidExt{ - Prebid: openrtb_ext.ExtResponseNonBidPrebid{ - Bid: openrtb_ext.NonBidObject{ - W: 200, - H: 200, - }, + expectedNonBids: &SeatNonBidBuilder{ + "pubmatic": { + { + StatusCode: 351, + Ext: &openrtb_ext.NonBidExt{ + Prebid: openrtb_ext.ExtResponseNonBidPrebid{ + Bid: openrtb_ext.NonBidObject{ + W: 200, + H: 200, }, }, }, @@ -4832,7 +4828,7 @@ func TestMakeBidWithValidation(t *testing.T) { givenBids: []*entities.PbsOrtbBid{{Bid: &openrtb2.Bid{W: 200, H: 200}, BidType: openrtb_ext.BidTypeBanner}, {Bid: &openrtb2.Bid{W: 50, H: 50}, BidType: openrtb_ext.BidTypeBanner}}, givenSeat: "pubmatic", expectedNumOfBids: 2, - expectedNonBids: &nonBids{}, + expectedNonBids: &SeatNonBidBuilder{}, expectedNumDebugErrors: 1, }, { @@ -4841,12 +4837,15 @@ func TestMakeBidWithValidation(t *testing.T) { givenBids: []*entities.PbsOrtbBid{{Bid: &openrtb2.Bid{AdM: "http://domain.com/invalid", ImpID: "1"}, BidType: openrtb_ext.BidTypeBanner}, {Bid: &openrtb2.Bid{AdM: "https://domain.com/valid", ImpID: "2"}, BidType: openrtb_ext.BidTypeBanner}}, givenSeat: "pubmatic", expectedNumOfBids: 1, - expectedNonBids: &nonBids{ - seatNonBidsMap: map[string][]openrtb_ext.NonBid{ - "pubmatic": { - { - ImpId: "1", - StatusCode: 352, + expectedNonBids: &SeatNonBidBuilder{ + "pubmatic": { + { + ImpId: "1", + StatusCode: 352, + Ext: &openrtb_ext.NonBidExt{ + Prebid: openrtb_ext.ExtResponseNonBidPrebid{ + Bid: openrtb_ext.NonBidObject{}, + }, }, }, }, @@ -4859,7 +4858,7 @@ func TestMakeBidWithValidation(t *testing.T) { givenBids: []*entities.PbsOrtbBid{{Bid: &openrtb2.Bid{AdM: "http://domain.com/invalid", ImpID: "1"}, BidType: openrtb_ext.BidTypeBanner}, {Bid: &openrtb2.Bid{AdM: "https://domain.com/valid", ImpID: "2"}, BidType: openrtb_ext.BidTypeBanner}}, givenSeat: "pubmatic", expectedNumOfBids: 2, - expectedNonBids: &nonBids{}, + expectedNonBids: &SeatNonBidBuilder{}, expectedNumDebugErrors: 1, }, { @@ -4868,7 +4867,7 @@ func TestMakeBidWithValidation(t *testing.T) { givenBids: []*entities.PbsOrtbBid{{Bid: &openrtb2.Bid{AdM: "http://domain.com/invalid"}, BidType: openrtb_ext.BidTypeBanner}, {Bid: &openrtb2.Bid{AdM: "https://domain.com/valid"}, BidType: openrtb_ext.BidTypeBanner}}, givenSeat: "pubmatic", expectedNumOfBids: 2, - expectedNonBids: &nonBids{}, + expectedNonBids: &SeatNonBidBuilder{}, }, { name: "Creative_size_validation_skipped,_Adm_Validation_enforced,_one_of_two_bids_has_invalid_dimensions", @@ -4876,7 +4875,7 @@ func TestMakeBidWithValidation(t *testing.T) { givenBids: []*entities.PbsOrtbBid{{Bid: &openrtb2.Bid{W: 200, H: 200}, BidType: openrtb_ext.BidTypeBanner}, {Bid: &openrtb2.Bid{W: 50, H: 50}, BidType: openrtb_ext.BidTypeBanner}}, givenSeat: "pubmatic", expectedNumOfBids: 2, - expectedNonBids: &nonBids{}, + expectedNonBids: &SeatNonBidBuilder{}, }, } @@ -4925,7 +4924,7 @@ func TestMakeBidWithValidation(t *testing.T) { } e.bidValidationEnforcement = test.givenValidations sampleBids := test.givenBids - nonBids := &nonBids{} + nonBids := &SeatNonBidBuilder{} resultingBids, resultingErrs := e.makeBid(sampleBids, sampleAuction, true, ImpExtInfoMap, bidRequest, bidExtResponse, test.givenSeat, "", nonBids) assert.Equal(t, 0, len(resultingErrs)) @@ -6060,7 +6059,7 @@ func TestSelectNewDuration(t *testing.T) { func TestSetSeatNonBid(t *testing.T) { type args struct { bidResponseExt *openrtb_ext.ExtBidResponse - seatNonBids nonBids + seatNonBids SeatNonBidBuilder } tests := []struct { name string @@ -6069,12 +6068,12 @@ func TestSetSeatNonBid(t *testing.T) { }{ { name: "empty-seatNonBidsMap", - args: args{seatNonBids: nonBids{}, bidResponseExt: nil}, + args: args{seatNonBids: SeatNonBidBuilder{}, bidResponseExt: nil}, want: nil, }, { name: "nil-bidResponseExt", - args: args{seatNonBids: nonBids{seatNonBidsMap: map[string][]openrtb_ext.NonBid{"key": nil}}, bidResponseExt: nil}, + args: args{seatNonBids: SeatNonBidBuilder{"key": nil}, bidResponseExt: nil}, want: &openrtb_ext.ExtBidResponse{ Prebid: &openrtb_ext.ExtResponsePrebid{ SeatNonBid: []openrtb_ext.SeatNonBid{{ diff --git a/exchange/non_bid_reason.go b/exchange/non_bid_reason.go index fe9a8e26c48..edfd3bc1e3d 100644 --- a/exchange/non_bid_reason.go +++ b/exchange/non_bid_reason.go @@ -1,12 +1,22 @@ package exchange +import ( + "errors" + "net" + "syscall" + + "github.com/prebid/prebid-server/v2/errortypes" +) + // SeatNonBid list the reasons why bid was not resulted in positive bid // reason could be either No bid, Error, Request rejection or Response rejection -// Reference: https://github.com/InteractiveAdvertisingBureau/openrtb/blob/master/extensions/community_extensions/seat-non-bid.md -type NonBidReason int +// Reference: https://github.com/InteractiveAdvertisingBureau/openrtb/blob/master/extensions/community_extensions/seat-non-bid.md#list-non-bid-status-codes +type NonBidReason int64 const ( - NoBidUnknownError NonBidReason = 0 // No Bid - General + ErrorGeneral NonBidReason = 100 // Error - General + ErrorTimeout NonBidReason = 101 // Error - Timeout + ErrorBidderUnreachable NonBidReason = 103 // Error - Bidder Unreachable ResponseRejectedGeneral NonBidReason = 300 ResponseRejectedBelowFloor NonBidReason = 301 // Response Rejected - Below Floor ResponseRejectedCategoryMappingInvalid NonBidReason = 303 // Response Rejected - Category Mapping Invalid @@ -15,15 +25,33 @@ const ( ResponseRejectedCreativeNotSecure NonBidReason = 352 // Response Rejected - Invalid Creative (Not Secure) ) -// Ptr returns pointer to own value. -func (n NonBidReason) Ptr() *NonBidReason { - return &n +func errorToNonBidReason(err error) NonBidReason { + switch errortypes.ReadCode(err) { + case errortypes.TimeoutErrorCode: + return ErrorTimeout + default: + return ErrorGeneral + } } -// Val safely dereferences pointer, returning default value (NoBidUnknownError) for nil. -func (n *NonBidReason) Val() NonBidReason { - if n == nil { - return NoBidUnknownError +// httpInfoToNonBidReason determines NoBidReason code (NBR) +// It will first try to resolve the NBR based on prebid's proprietary error code. +// If proprietary error code not found then it will try to determine NBR using +// system call level error code +func httpInfoToNonBidReason(httpInfo *httpCallInfo) NonBidReason { + nonBidReason := errorToNonBidReason(httpInfo.err) + if nonBidReason != ErrorGeneral { + return nonBidReason } - return *n + if isBidderUnreachableError(httpInfo) { + return ErrorBidderUnreachable + } + return ErrorGeneral +} + +// isBidderUnreachableError checks if the error is due to connection refused or no such host +func isBidderUnreachableError(httpInfo *httpCallInfo) bool { + var dnsErr *net.DNSError + isNoSuchHost := errors.As(httpInfo.err, &dnsErr) && dnsErr.IsNotFound + return errors.Is(httpInfo.err, syscall.ECONNREFUSED) || isNoSuchHost } diff --git a/exchange/non_bid_reason_test.go b/exchange/non_bid_reason_test.go new file mode 100644 index 00000000000..ab5c9b4f957 --- /dev/null +++ b/exchange/non_bid_reason_test.go @@ -0,0 +1,65 @@ +package exchange + +import ( + "errors" + "net" + "syscall" + "testing" + + "github.com/prebid/prebid-server/v2/errortypes" + "github.com/stretchr/testify/assert" +) + +func Test_httpInfoToNonBidReason(t *testing.T) { + type args struct { + httpInfo *httpCallInfo + } + tests := []struct { + name string + args args + want NonBidReason + }{ + { + name: "error-timeout", + args: args{ + httpInfo: &httpCallInfo{ + err: &errortypes.Timeout{}, + }, + }, + want: ErrorTimeout, + }, + { + name: "error-general", + args: args{ + httpInfo: &httpCallInfo{ + err: errors.New("some_error"), + }, + }, + want: ErrorGeneral, + }, + { + name: "error-bidderUnreachable", + args: args{ + httpInfo: &httpCallInfo{ + err: syscall.ECONNREFUSED, + }, + }, + want: ErrorBidderUnreachable, + }, + { + name: "error-biddersUnreachable-no-such-host", + args: args{ + httpInfo: &httpCallInfo{ + err: &net.DNSError{IsNotFound: true}, + }, + }, + want: ErrorBidderUnreachable, + }, + } + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + actual := httpInfoToNonBidReason(tt.args.httpInfo) + assert.Equal(t, tt.want, actual) + }) + } +} diff --git a/exchange/seat_non_bids.go b/exchange/seat_non_bids.go index 78c1b23e3f3..760431ef44f 100644 --- a/exchange/seat_non_bids.go +++ b/exchange/seat_non_bids.go @@ -5,22 +5,18 @@ import ( "github.com/prebid/prebid-server/v2/openrtb_ext" ) -type nonBids struct { - seatNonBidsMap map[string][]openrtb_ext.NonBid -} +type SeatNonBidBuilder map[string][]openrtb_ext.NonBid -// addBid is not thread safe as we are initializing and writing to map -func (snb *nonBids) addBid(bid *entities.PbsOrtbBid, nonBidReason int, seat string) { - if bid == nil || bid.Bid == nil { +// rejectBid appends a non bid object to the builder based on a bid +func (b SeatNonBidBuilder) rejectBid(bid *entities.PbsOrtbBid, nonBidReason int, seat string) { + if b == nil || bid == nil || bid.Bid == nil { return } - if snb.seatNonBidsMap == nil { - snb.seatNonBidsMap = make(map[string][]openrtb_ext.NonBid) - } + nonBid := openrtb_ext.NonBid{ ImpId: bid.Bid.ImpID, StatusCode: nonBidReason, - Ext: openrtb_ext.NonBidExt{ + Ext: &openrtb_ext.NonBidExt{ Prebid: openrtb_ext.ExtResponseNonBidPrebid{Bid: openrtb_ext.NonBidObject{ Price: bid.Bid.Price, ADomain: bid.Bid.ADomain, @@ -36,16 +32,29 @@ func (snb *nonBids) addBid(bid *entities.PbsOrtbBid, nonBidReason int, seat stri }}, }, } - - snb.seatNonBidsMap[seat] = append(snb.seatNonBidsMap[seat], nonBid) + b[seat] = append(b[seat], nonBid) } -func (snb *nonBids) get() []openrtb_ext.SeatNonBid { - if snb == nil { - return nil +// rejectImps appends a non bid object to the builder for every specified imp +func (b SeatNonBidBuilder) rejectImps(impIds []string, nonBidReason NonBidReason, seat string) { + nonBids := []openrtb_ext.NonBid{} + for _, impId := range impIds { + nonBid := openrtb_ext.NonBid{ + ImpId: impId, + StatusCode: int(nonBidReason), + } + nonBids = append(nonBids, nonBid) } - var seatNonBid []openrtb_ext.SeatNonBid - for seat, nonBids := range snb.seatNonBidsMap { + + if len(nonBids) > 0 { + b[seat] = append(b[seat], nonBids...) + } +} + +// slice transforms the seat non bid map into a slice of SeatNonBid objects representing the non-bids for each seat +func (b SeatNonBidBuilder) Slice() []openrtb_ext.SeatNonBid { + seatNonBid := make([]openrtb_ext.SeatNonBid, 0) + for seat, nonBids := range b { seatNonBid = append(seatNonBid, openrtb_ext.SeatNonBid{ Seat: seat, NonBid: nonBids, @@ -53,3 +62,16 @@ func (snb *nonBids) get() []openrtb_ext.SeatNonBid { } return seatNonBid } + +// append adds the nonBids from the input nonBids to the current nonBids. +// This method is not thread safe as we are initializing and writing to map +func (b SeatNonBidBuilder) append(nonBids ...SeatNonBidBuilder) { + if b == nil { + return + } + for _, nonBid := range nonBids { + for seat, nonBids := range nonBid { + b[seat] = append(b[seat], nonBids...) + } + } +} diff --git a/exchange/seat_non_bids_test.go b/exchange/seat_non_bids_test.go index 103c0939496..b754f885965 100644 --- a/exchange/seat_non_bids_test.go +++ b/exchange/seat_non_bids_test.go @@ -9,9 +9,9 @@ import ( "github.com/stretchr/testify/assert" ) -func TestSeatNonBidsAdd(t *testing.T) { +func TestRejectBid(t *testing.T) { type fields struct { - seatNonBidsMap map[string][]openrtb_ext.NonBid + builder SeatNonBidBuilder } type args struct { bid *entities.PbsOrtbBid @@ -22,89 +22,512 @@ func TestSeatNonBidsAdd(t *testing.T) { name string fields fields args args - want map[string][]openrtb_ext.NonBid + want SeatNonBidBuilder }{ { - name: "nil-seatNonBidsMap", - fields: fields{seatNonBidsMap: nil}, - args: args{}, - want: nil, + name: "nil_builder", + fields: fields{ + builder: nil, + }, + args: args{}, + want: nil, }, { - name: "nil-seatNonBidsMap-with-bid-object", - fields: fields{seatNonBidsMap: nil}, - args: args{bid: &entities.PbsOrtbBid{Bid: &openrtb2.Bid{}}, seat: "bidder1"}, - want: sampleSeatNonBidMap("bidder1", 1), + name: "nil_pbsortbid", + fields: fields{ + builder: SeatNonBidBuilder{}, + }, + args: args{ + bid: nil, + }, + want: SeatNonBidBuilder{}, }, { - name: "multiple-nonbids-for-same-seat", - fields: fields{seatNonBidsMap: sampleSeatNonBidMap("bidder2", 1)}, - args: args{bid: &entities.PbsOrtbBid{Bid: &openrtb2.Bid{}}, seat: "bidder2"}, - want: sampleSeatNonBidMap("bidder2", 2), + name: "nil_bid", + fields: fields{ + builder: SeatNonBidBuilder{}, + }, + args: args{ + bid: &entities.PbsOrtbBid{ + Bid: nil, + }, + }, + want: SeatNonBidBuilder{}, + }, + { + name: "append_nonbids_new_seat", + fields: fields{ + builder: SeatNonBidBuilder{}, + }, + args: args{ + bid: &entities.PbsOrtbBid{ + Bid: &openrtb2.Bid{ + ImpID: "Imp1", + Price: 10, + }, + }, + nonBidReason: int(ErrorGeneral), + seat: "seat1", + }, + want: SeatNonBidBuilder{ + "seat1": []openrtb_ext.NonBid{ + { + ImpId: "Imp1", + StatusCode: int(ErrorGeneral), + Ext: &openrtb_ext.NonBidExt{ + Prebid: openrtb_ext.ExtResponseNonBidPrebid{ + Bid: openrtb_ext.NonBidObject{ + Price: 10, + }, + }, + }, + }, + }, + }, + }, + { + name: "append_nonbids_for_different_seat", + fields: fields{ + builder: SeatNonBidBuilder{ + "seat1": []openrtb_ext.NonBid{ + { + ImpId: "Imp1", + StatusCode: int(ErrorGeneral), + }, + }, + }, + }, + args: args{ + bid: &entities.PbsOrtbBid{ + Bid: &openrtb2.Bid{ + ImpID: "Imp2", + Price: 10, + }, + }, + nonBidReason: int(ErrorGeneral), + seat: "seat2", + }, + want: SeatNonBidBuilder{ + "seat1": []openrtb_ext.NonBid{ + { + ImpId: "Imp1", + StatusCode: int(ErrorGeneral), + }, + }, + "seat2": []openrtb_ext.NonBid{ + { + ImpId: "Imp2", + StatusCode: int(ErrorGeneral), + Ext: &openrtb_ext.NonBidExt{ + Prebid: openrtb_ext.ExtResponseNonBidPrebid{ + Bid: openrtb_ext.NonBidObject{ + Price: 10, + }, + }, + }, + }, + }, + }, + }, + { + name: "append_nonbids_for_existing_seat", + fields: fields{ + builder: SeatNonBidBuilder{ + "seat1": []openrtb_ext.NonBid{ + { + ImpId: "Imp1", + StatusCode: int(ErrorGeneral), + }, + }, + }, + }, + args: args{ + bid: &entities.PbsOrtbBid{ + Bid: &openrtb2.Bid{ + ImpID: "Imp2", + Price: 10, + }, + }, + nonBidReason: int(ErrorGeneral), + seat: "seat1", + }, + want: SeatNonBidBuilder{ + "seat1": []openrtb_ext.NonBid{ + { + ImpId: "Imp1", + StatusCode: int(ErrorGeneral), + }, + { + ImpId: "Imp2", + StatusCode: int(ErrorGeneral), + Ext: &openrtb_ext.NonBidExt{ + Prebid: openrtb_ext.ExtResponseNonBidPrebid{ + Bid: openrtb_ext.NonBidObject{ + Price: 10, + }, + }, + }, + }, + }, + }, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - snb := &nonBids{ - seatNonBidsMap: tt.fields.seatNonBidsMap, - } - snb.addBid(tt.args.bid, tt.args.nonBidReason, tt.args.seat) - assert.Equalf(t, tt.want, snb.seatNonBidsMap, "expected seatNonBidsMap not nil") + snb := tt.fields.builder + snb.rejectBid(tt.args.bid, tt.args.nonBidReason, tt.args.seat) + assert.Equal(t, tt.want, snb) }) } } -func TestSeatNonBidsGet(t *testing.T) { - type fields struct { - snb *nonBids - } +func TestAppend(t *testing.T) { tests := []struct { - name string - fields fields - want []openrtb_ext.SeatNonBid + name string + builder SeatNonBidBuilder + toAppend []SeatNonBidBuilder + expected SeatNonBidBuilder }{ { - name: "get-seat-nonbids", - fields: fields{&nonBids{sampleSeatNonBidMap("bidder1", 2)}}, - want: sampleSeatBids("bidder1", 2), + name: "nil_buider", + builder: nil, + toAppend: []SeatNonBidBuilder{{"seat1": []openrtb_ext.NonBid{{ImpId: "imp1"}}}}, + expected: nil, + }, + { + name: "empty_builder", + builder: SeatNonBidBuilder{}, + toAppend: []SeatNonBidBuilder{{"seat1": []openrtb_ext.NonBid{{ImpId: "imp1"}}}}, + expected: SeatNonBidBuilder{"seat1": []openrtb_ext.NonBid{{ImpId: "imp1"}}}, + }, + { + name: "append_one_different_seat", + builder: SeatNonBidBuilder{"seat1": []openrtb_ext.NonBid{{ImpId: "imp1"}}}, + toAppend: []SeatNonBidBuilder{{"seat2": []openrtb_ext.NonBid{{ImpId: "imp2"}}}}, + expected: SeatNonBidBuilder{"seat1": []openrtb_ext.NonBid{{ImpId: "imp1"}}, "seat2": []openrtb_ext.NonBid{{ImpId: "imp2"}}}, + }, + { + name: "append_multiple_different_seats", + builder: SeatNonBidBuilder{"seat1": []openrtb_ext.NonBid{{ImpId: "imp1"}}}, + toAppend: []SeatNonBidBuilder{{"seat2": []openrtb_ext.NonBid{{ImpId: "imp2"}}}, {"seat3": []openrtb_ext.NonBid{{ImpId: "imp3"}}}}, + expected: SeatNonBidBuilder{"seat1": []openrtb_ext.NonBid{{ImpId: "imp1"}}, "seat2": []openrtb_ext.NonBid{{ImpId: "imp2"}}, "seat3": []openrtb_ext.NonBid{{ImpId: "imp3"}}}, + }, + { + name: "nil_append", + builder: SeatNonBidBuilder{"seat1": []openrtb_ext.NonBid{{ImpId: "imp1"}}}, + toAppend: nil, + expected: SeatNonBidBuilder{"seat1": []openrtb_ext.NonBid{{ImpId: "imp1"}}}, + }, + { + name: "empty_append", + builder: SeatNonBidBuilder{"seat1": []openrtb_ext.NonBid{{ImpId: "imp1"}}}, + toAppend: []SeatNonBidBuilder{}, + expected: SeatNonBidBuilder{"seat1": []openrtb_ext.NonBid{{ImpId: "imp1"}}}, }, { - name: "nil-seat-nonbids", - fields: fields{nil}, + name: "append_multiple_same_seat", + builder: SeatNonBidBuilder{ + "seat1": []openrtb_ext.NonBid{ + {ImpId: "imp1"}, + }, + }, + toAppend: []SeatNonBidBuilder{ + { + "seat1": []openrtb_ext.NonBid{ + {ImpId: "imp2"}, + }, + }, + }, + expected: SeatNonBidBuilder{ + "seat1": []openrtb_ext.NonBid{ + {ImpId: "imp1"}, + {ImpId: "imp2"}, + }, + }, }, } for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - if got := tt.fields.snb.get(); !assert.Equal(t, tt.want, got) { - t.Errorf("seatNonBids.get() = %v, want %v", got, tt.want) - } + tt.builder.append(tt.toAppend...) + assert.Equal(t, tt.expected, tt.builder) }) } } -var sampleSeatNonBidMap = func(seat string, nonBidCount int) map[string][]openrtb_ext.NonBid { - nonBids := make([]openrtb_ext.NonBid, 0) - for i := 0; i < nonBidCount; i++ { - nonBids = append(nonBids, openrtb_ext.NonBid{ - Ext: openrtb_ext.NonBidExt{Prebid: openrtb_ext.ExtResponseNonBidPrebid{Bid: openrtb_ext.NonBidObject{}}}, - }) +func TestRejectImps(t *testing.T) { + tests := []struct { + name string + impIDs []string + builder SeatNonBidBuilder + want SeatNonBidBuilder + }{ + { + name: "nil_imps", + impIDs: nil, + builder: SeatNonBidBuilder{}, + want: SeatNonBidBuilder{}, + }, + { + name: "empty_imps", + impIDs: []string{}, + builder: SeatNonBidBuilder{}, + want: SeatNonBidBuilder{}, + }, + { + name: "one_imp", + impIDs: []string{"imp1"}, + builder: SeatNonBidBuilder{}, + want: SeatNonBidBuilder{ + "seat1": []openrtb_ext.NonBid{ + { + ImpId: "imp1", + StatusCode: 300, + }, + }, + }, + }, + { + name: "many_imps", + impIDs: []string{"imp1", "imp2"}, + builder: SeatNonBidBuilder{}, + want: SeatNonBidBuilder{ + "seat1": []openrtb_ext.NonBid{ + { + ImpId: "imp1", + StatusCode: 300, + }, + { + ImpId: "imp2", + StatusCode: 300, + }, + }, + }, + }, + { + name: "many_imps_appended_to_prepopulated_list", + impIDs: []string{"imp1", "imp2"}, + builder: SeatNonBidBuilder{ + "seat0": []openrtb_ext.NonBid{ + { + ImpId: "imp0", + StatusCode: 0, + }, + }, + }, + want: SeatNonBidBuilder{ + "seat0": []openrtb_ext.NonBid{ + { + ImpId: "imp0", + StatusCode: 0, + }, + }, + "seat1": []openrtb_ext.NonBid{ + { + ImpId: "imp1", + StatusCode: 300, + }, + { + ImpId: "imp2", + StatusCode: 300, + }, + }, + }, + }, + { + name: "many_imps_appended_to_prepopulated_list_same_seat", + impIDs: []string{"imp1", "imp2"}, + builder: SeatNonBidBuilder{ + "seat1": []openrtb_ext.NonBid{ + { + ImpId: "imp0", + StatusCode: 300, + }, + }, + }, + want: SeatNonBidBuilder{ + "seat1": []openrtb_ext.NonBid{ + { + ImpId: "imp0", + StatusCode: 300, + }, + { + ImpId: "imp1", + StatusCode: 300, + }, + { + ImpId: "imp2", + StatusCode: 300, + }, + }, + }, + }, } - return map[string][]openrtb_ext.NonBid{ - seat: nonBids, + + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + test.builder.rejectImps(test.impIDs, 300, "seat1") + + assert.Equal(t, len(test.builder), len(test.want)) + for seat := range test.want { + assert.ElementsMatch(t, test.want[seat], test.builder[seat]) + } + }) } } -var sampleSeatBids = func(seat string, nonBidCount int) []openrtb_ext.SeatNonBid { - seatNonBids := make([]openrtb_ext.SeatNonBid, 0) - seatNonBid := openrtb_ext.SeatNonBid{ - Seat: seat, - NonBid: make([]openrtb_ext.NonBid, 0), +func TestSlice(t *testing.T) { + tests := []struct { + name string + builder SeatNonBidBuilder + want []openrtb_ext.SeatNonBid + }{ + { + name: "nil", + builder: nil, + want: []openrtb_ext.SeatNonBid{}, + }, + { + name: "empty", + builder: SeatNonBidBuilder{}, + want: []openrtb_ext.SeatNonBid{}, + }, + { + name: "one_no_nonbids", + builder: SeatNonBidBuilder{ + "a": []openrtb_ext.NonBid{}, + }, + want: []openrtb_ext.SeatNonBid{ + { + NonBid: []openrtb_ext.NonBid{}, + Seat: "a", + }, + }, + }, + { + name: "one_with_nonbids", + builder: SeatNonBidBuilder{ + "a": []openrtb_ext.NonBid{ + { + ImpId: "imp1", + StatusCode: 100, + }, + { + ImpId: "imp2", + StatusCode: 200, + }, + }, + }, + want: []openrtb_ext.SeatNonBid{ + { + NonBid: []openrtb_ext.NonBid{ + { + ImpId: "imp1", + StatusCode: 100, + }, + { + ImpId: "imp2", + StatusCode: 200, + }, + }, + Seat: "a", + }, + }, + }, + { + name: "many_no_nonbids", + builder: SeatNonBidBuilder{ + "a": []openrtb_ext.NonBid{}, + "b": []openrtb_ext.NonBid{}, + "c": []openrtb_ext.NonBid{}, + }, + want: []openrtb_ext.SeatNonBid{ + { + NonBid: []openrtb_ext.NonBid{}, + Seat: "a", + }, + { + NonBid: []openrtb_ext.NonBid{}, + Seat: "b", + }, + { + NonBid: []openrtb_ext.NonBid{}, + Seat: "c", + }, + }, + }, + { + name: "many_with_nonbids", + builder: SeatNonBidBuilder{ + "a": []openrtb_ext.NonBid{ + { + ImpId: "imp1", + StatusCode: 100, + }, + { + ImpId: "imp2", + StatusCode: 200, + }, + }, + "b": []openrtb_ext.NonBid{ + { + ImpId: "imp3", + StatusCode: 300, + }, + }, + "c": []openrtb_ext.NonBid{ + { + ImpId: "imp4", + StatusCode: 400, + }, + { + ImpId: "imp5", + StatusCode: 500, + }, + }, + }, + want: []openrtb_ext.SeatNonBid{ + { + NonBid: []openrtb_ext.NonBid{ + { + ImpId: "imp1", + StatusCode: 100, + }, + { + ImpId: "imp2", + StatusCode: 200, + }, + }, + Seat: "a", + }, + { + NonBid: []openrtb_ext.NonBid{ + { + ImpId: "imp3", + StatusCode: 300, + }, + }, + Seat: "b", + }, + { + NonBid: []openrtb_ext.NonBid{ + { + ImpId: "imp4", + StatusCode: 400, + }, + { + ImpId: "imp5", + StatusCode: 500, + }, + }, + Seat: "c", + }, + }, + }, } - for i := 0; i < nonBidCount; i++ { - seatNonBid.NonBid = append(seatNonBid.NonBid, openrtb_ext.NonBid{ - Ext: openrtb_ext.NonBidExt{Prebid: openrtb_ext.ExtResponseNonBidPrebid{Bid: openrtb_ext.NonBidObject{}}}, + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + result := test.builder.Slice() + assert.ElementsMatch(t, test.want, result) }) } - seatNonBids = append(seatNonBids, seatNonBid) - return seatNonBids } diff --git a/openrtb_ext/response.go b/openrtb_ext/response.go index d9baea3f4da..449ff939bf5 100644 --- a/openrtb_ext/response.go +++ b/openrtb_ext/response.go @@ -132,14 +132,14 @@ type NonBidExt struct { // NonBid represnts the Non Bid Reason (statusCode) for given impression ID type NonBid struct { - ImpId string `json:"impid"` - StatusCode int `json:"statuscode"` - Ext NonBidExt `json:"ext"` + ImpId string `json:"impid"` + StatusCode int `json:"statuscode"` + Ext *NonBidExt `json:"ext,omitempty"` } // SeatNonBid is collection of NonBid objects with seat information type SeatNonBid struct { NonBid []NonBid `json:"nonbid"` Seat string `json:"seat"` - Ext json.RawMessage `json:"ext"` + Ext json.RawMessage `json:"ext,omitempty"` } diff --git a/stored_requests/data/by_id/accounts/test.json b/stored_requests/data/by_id/accounts/test.json index 699f6bd1e57..a53f8997f37 100644 --- a/stored_requests/data/by_id/accounts/test.json +++ b/stored_requests/data/by_id/accounts/test.json @@ -49,4 +49,4 @@ } } } -} +} \ No newline at end of file