Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Debug warnings #1724

Merged
merged 6 commits into from
Mar 18, 2021
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
20 changes: 12 additions & 8 deletions endpoints/openrtb2/amp_auction.go
Original file line number Diff line number Diff line change
Expand Up @@ -35,10 +35,10 @@ import (
const defaultAmpRequestTimeoutMillis = 900

type AmpResponse struct {
Targeting map[string]string `json:"targeting"`
Debug *openrtb_ext.ExtResponseDebug `json:"debug,omitempty"`
Errors map[openrtb_ext.BidderName][]openrtb_ext.ExtBidderError `json:"errors,omitempty"`
Warnings map[openrtb_ext.BidderName][]openrtb_ext.ExtBidderError `json:"warnings,omitempty"`
Targeting map[string]string `json:"targeting"`
Debug *openrtb_ext.ExtResponseDebug `json:"debug,omitempty"`
Errors map[openrtb_ext.BidderName][]openrtb_ext.ExtBidderMessage `json:"errors,omitempty"`
Warnings map[openrtb_ext.BidderName][]openrtb_ext.ExtBidderMessage `json:"warnings,omitempty"`
SyntaxNode marked this conversation as resolved.
Show resolved Hide resolved
}

// NewAmpEndpoint modifies the OpenRTB endpoint to handle AMP requests. This will basically modify the parsing
Expand Down Expand Up @@ -239,9 +239,12 @@ func (deps *endpointDeps) AmpAuction(w http.ResponseWriter, r *http.Request, _ h
ao.Errors = append(ao.Errors, fmt.Errorf("AMP response: failed to unpack OpenRTB response.ext, debug info cannot be forwarded: %v", eRErr))
}

warnings := make(map[openrtb_ext.BidderName][]openrtb_ext.ExtBidderError)
warnings := extResponse.Warnings
if warnings == nil {
warnings = make(map[openrtb_ext.BidderName][]openrtb_ext.ExtBidderMessage)
}
for _, v := range errortypes.WarningOnly(errL) {
bidderErr := openrtb_ext.ExtBidderError{
bidderErr := openrtb_ext.ExtBidderMessage{
Code: errortypes.ReadCode(v),
Message: v.Error(),
}
Expand Down Expand Up @@ -528,8 +531,9 @@ func readPolicy(consent string) (privacy.PolicyWriter, error) {
return ccpa.ConsentWriter{consent}, nil
}

return privacy.NilPolicyWriter{}, &errortypes.InvalidPrivacyConsent{
Message: fmt.Sprintf("Consent '%s' is not recognized as either CCPA or GDPR TCF.", consent),
return privacy.NilPolicyWriter{}, &errortypes.Warning{
Message: fmt.Sprintf("Consent '%s' is not recognized as either CCPA or GDPR TCF.", consent),
WarningCode: errortypes.InvalidPrivacyConsentWarningCode,
}
}

Expand Down
193 changes: 107 additions & 86 deletions endpoints/openrtb2/amp_auction_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -395,103 +395,107 @@ func TestCCPAConsent(t *testing.T) {
}
}

func TestNoConsent(t *testing.T) {
// Build Request
bid, err := getTestBidRequest(true, nil, true, nil)
if err != nil {
t.Fatalf("Failed to marshal the complete openrtb.BidRequest object %v", err)
func TestConsentWarnings(t *testing.T) {
type inputTest struct {
nilRegs bool
regs string
invalidConsentURL bool
bidderWarnings bool
}
testData := []inputTest{
{true, "", false, false},
SyntaxNode marked this conversation as resolved.
Show resolved Hide resolved
{true, "", true, false},
{false, "invalid", true, true},
{false, "1NYN", false, true},
}

// Simulated Stored Request Backend
stored := map[string]json.RawMessage{"1": json.RawMessage(bid)}
for _, testCase := range testData {

// Build Exchange Endpoint
mockExchange := &mockAmpExchange{}
endpoint, _ := NewAmpEndpoint(
mockExchange,
newParamsValidator(t),
&mockAmpStoredReqFetcher{stored},
empty_fetcher.EmptyFetcher{},
&config.Configuration{MaxRequestSize: maxSize},
newTestMetrics(),
analyticsConf.NewPBSAnalytics(&config.Analytics{}),
map[string]string{},
[]byte{},
openrtb_ext.BuildBidderMap(),
)
// Build Request
var regs *openrtb_ext.ExtRegs
if !testCase.nilRegs {
regs = &openrtb_ext.ExtRegs{USPrivacy: testCase.regs}
}
SyntaxNode marked this conversation as resolved.
Show resolved Hide resolved

// Invoke Endpoint
request := httptest.NewRequest("GET", "/openrtb2/auction/amp?tag_id=1", nil)
responseRecorder := httptest.NewRecorder()
endpoint(responseRecorder, request, nil)
bid, err := getTestBidRequest(true, nil, testCase.nilRegs, regs)
if err != nil {
t.Fatalf("Failed to marshal the complete openrtb.BidRequest object %v", err)
}

// Parse Response
var response AmpResponse
if err := json.Unmarshal(responseRecorder.Body.Bytes(), &response); err != nil {
t.Fatalf("Error unmarshalling response: %s", err.Error())
}
// Simulated Stored Request Backend
stored := map[string]json.RawMessage{"1": json.RawMessage(bid)}

// Assert Result
result := mockExchange.lastRequest
assert.NotNil(t, result, "lastRequest")
assert.Nil(t, result.User, "lastRequest.User")
assert.Nil(t, result.Regs, "lastRequest.Regs")
assert.Equal(t, expectedErrorsFromHoldAuction, response.Errors)
assert.Empty(t, response.Warnings)
}
// Build Exchange Endpoint
var mockExchange exchange.Exchange
if testCase.bidderWarnings {
mockExchange = &mockAmpExchangeWarnings{}
} else {
mockExchange = &mockAmpExchange{}
}
endpoint, _ := NewAmpEndpoint(
mockExchange,
newParamsValidator(t),
&mockAmpStoredReqFetcher{stored},
empty_fetcher.EmptyFetcher{},
&config.Configuration{MaxRequestSize: maxSize},
newTestMetrics(),
analyticsConf.NewPBSAnalytics(&config.Analytics{}),
map[string]string{},
[]byte{},
openrtb_ext.BuildBidderMap(),
)

func TestInvalidConsent(t *testing.T) {
// Build Request
bid, err := getTestBidRequest(true, nil, true, nil)
if err != nil {
t.Fatalf("Failed to marshal the complete openrtb.BidRequest object %v", err)
}
// Invoke Endpoint
var request *http.Request
invalidConsent := "invalid"
if testCase.invalidConsentURL {
request = httptest.NewRequest("GET", "/openrtb2/auction/amp?tag_id=1&consent_string="+invalidConsent, nil)

// Simulated Stored Request Backend
stored := map[string]json.RawMessage{"1": json.RawMessage(bid)}
} else {
request = httptest.NewRequest("GET", "/openrtb2/auction/amp?tag_id=1", nil)
}

// Build Exchange Endpoint
mockExchange := &mockAmpExchange{}
endpoint, _ := NewAmpEndpoint(
mockExchange,
newParamsValidator(t),
&mockAmpStoredReqFetcher{stored},
empty_fetcher.EmptyFetcher{},
&config.Configuration{MaxRequestSize: maxSize},
newTestMetrics(),
analyticsConf.NewPBSAnalytics(&config.Analytics{}),
map[string]string{},
[]byte{},
openrtb_ext.BuildBidderMap(),
)
responseRecorder := httptest.NewRecorder()
endpoint(responseRecorder, request, nil)

// Invoke Endpoint
invalidConsent := "invalid"
request := httptest.NewRequest("GET", "/openrtb2/auction/amp?tag_id=1&consent_string="+invalidConsent, nil)
responseRecorder := httptest.NewRecorder()
endpoint(responseRecorder, request, nil)
// Parse Response
var response AmpResponse
if err := json.Unmarshal(responseRecorder.Body.Bytes(), &response); err != nil {
t.Fatalf("Error unmarshalling response: %s", err.Error())
}

// Parse Response
var response AmpResponse
if err := json.Unmarshal(responseRecorder.Body.Bytes(), &response); err != nil {
t.Fatalf("Error unmarshalling response: %s", err.Error())
}
// Assert Result
if !testCase.bidderWarnings {
result := mockExchange.(*mockAmpExchange).lastRequest
assert.NotNil(t, result, "lastRequest")
assert.Nil(t, result.User, "lastRequest.User")
assert.Nil(t, result.Regs, "lastRequest.Regs")
assert.Equal(t, expectedErrorsFromHoldAuction, response.Errors)
if testCase.invalidConsentURL {
expectedWarnings := map[openrtb_ext.BidderName][]openrtb_ext.ExtBidderMessage{
openrtb_ext.BidderReservedGeneral: {
{
Code: 10001,
Message: "Consent '" + invalidConsent + "' is not recognized as either CCPA or GDPR TCF.",
},
},
}
assert.Equal(t, expectedWarnings, response.Warnings)
} else {
assert.Empty(t, response.Warnings)
}

// Assert Result
expectedWarnings := map[openrtb_ext.BidderName][]openrtb_ext.ExtBidderError{
openrtb_ext.BidderReservedGeneral: {
{
Code: 10001,
Message: "Consent '" + invalidConsent + "' is not recognized as either CCPA or GDPR TCF.",
},
},
} else {
if !testCase.invalidConsentURL {
assert.Len(t, response.Warnings["appnexus"], 1, "1 bidder warning should be present")
assert.Len(t, response.Warnings["general"], 0, "General warnings should not be present")

} else {
assert.Len(t, response.Warnings["appnexus"], 1, "1 bidder warning should be present")
assert.Len(t, response.Warnings["general"], 2, "2 General warnings should be present")
}
}
SyntaxNode marked this conversation as resolved.
Show resolved Hide resolved
}
result := mockExchange.lastRequest
assert.NotNil(t, result, "lastRequest")
assert.Nil(t, result.User, "lastRequest.User")
assert.Nil(t, result.Regs, "lastRequest.Regs")
assert.Equal(t, expectedErrorsFromHoldAuction, response.Errors)
assert.Equal(t, expectedWarnings, response.Warnings)
}

func TestNewAndLegacyConsentBothProvided(t *testing.T) {
Expand Down Expand Up @@ -929,7 +933,7 @@ type mockAmpExchange struct {
lastRequest *openrtb.BidRequest
}

var expectedErrorsFromHoldAuction map[openrtb_ext.BidderName][]openrtb_ext.ExtBidderError = map[openrtb_ext.BidderName][]openrtb_ext.ExtBidderError{
var expectedErrorsFromHoldAuction map[openrtb_ext.BidderName][]openrtb_ext.ExtBidderMessage = map[openrtb_ext.BidderName][]openrtb_ext.ExtBidderMessage{
openrtb_ext.BidderName("openx"): {
{
Code: 1,
Expand Down Expand Up @@ -962,6 +966,24 @@ func (m *mockAmpExchange) HoldAuction(ctx context.Context, r exchange.AuctionReq
return response, nil
}

type mockAmpExchangeWarnings struct {
//lastRequest *openrtb.BidRequest
SyntaxNode marked this conversation as resolved.
Show resolved Hide resolved
}

func (m *mockAmpExchangeWarnings) HoldAuction(ctx context.Context, r exchange.AuctionRequest, debugLog *exchange.DebugLog) (*openrtb.BidResponse, error) {
//m.lastRequest = r.BidRequest
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please remove commented out code.

response := &openrtb.BidResponse{
SeatBid: []openrtb.SeatBid{{
Bid: []openrtb.Bid{{
AdM: "<script></script>",
Ext: json.RawMessage(`{ "prebid": {"targeting": { "hb_pb": "1.20", "hb_appnexus_pb": "1.20", "hb_cache_id": "some_id"}}}`),
}},
}},
Ext: json.RawMessage(`{ "warnings": {"appnexus": [{"code": 10003, "message": "debug turned off for bidder"}] }}`),
}
SyntaxNode marked this conversation as resolved.
Show resolved Hide resolved
return response, nil
}

func getTestBidRequest(nilUser bool, userExt *openrtb_ext.ExtUser, nilRegs bool, regsExt *openrtb_ext.ExtRegs) ([]byte, error) {
var width uint64 = 300
var height uint64 = 300
Expand Down Expand Up @@ -1025,7 +1047,6 @@ func getTestBidRequest(nilUser bool, userExt *openrtb_ext.ExtUser, nilRegs bool,
Ext: regsExtData,
}
}

return json.Marshal(bidRequest)
}

Expand Down
8 changes: 6 additions & 2 deletions endpoints/openrtb2/auction.go
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,7 @@ func (deps *endpointDeps) Auction(w http.ResponseWriter, r *http.Request, _ http
if errortypes.ContainsFatalError(errL) && writeError(errL, w, &labels) {
return
}
warnings := errortypes.WarningOnly(errL)

ctx := context.Background()

Expand Down Expand Up @@ -179,6 +180,7 @@ func (deps *endpointDeps) Auction(w http.ResponseWriter, r *http.Request, _ http
RequestType: labels.RType,
StartTime: start,
LegacyLabels: labels,
Warnings: warnings,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To Other Reviewers: This is necessary evil right now since HoldAuction is also composing the endpoint response. We'll need to separate these behaviors in the future, but that's too much work to do here.

}

response, err := deps.ex.HoldAuction(ctx, auctionRequest, nil)
Expand Down Expand Up @@ -363,8 +365,10 @@ func (deps *endpointDeps) validateRequest(req *openrtb.BidRequest) []error {
if ccpaPolicy, err := ccpa.ReadFromRequest(req); err != nil {
return append(errL, err)
} else if _, err := ccpaPolicy.Parse(exchange.GetValidBidders(aliases)); err != nil {
if _, invalidConsent := err.(*errortypes.InvalidPrivacyConsent); invalidConsent {
errL = append(errL, &errortypes.InvalidPrivacyConsent{Message: fmt.Sprintf("CCPA consent is invalid and will be ignored. (%v)", err)})
if _, invalidConsent := err.(*errortypes.Warning); invalidConsent {
errL = append(errL, &errortypes.Warning{
Message: fmt.Sprintf("CCPA consent is invalid and will be ignored. (%v)", err),
WarningCode: errortypes.InvalidPrivacyConsentWarningCode})
consentWriter := ccpa.ConsentWriter{Consent: ""}
if err := consentWriter.Write(req); err != nil {
return append(errL, fmt.Errorf("Unable to remove invalid CCPA consent from the request. (%v)", err))
Expand Down
52 changes: 51 additions & 1 deletion endpoints/openrtb2/auction_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1505,7 +1505,9 @@ func TestCCPAInvalid(t *testing.T) {

errL := deps.validateRequest(&req)

expectedWarning := errortypes.InvalidPrivacyConsent{Message: "CCPA consent is invalid and will be ignored. (request.regs.ext.us_privacy must contain 4 characters)"}
expectedWarning := errortypes.Warning{
Message: "CCPA consent is invalid and will be ignored. (request.regs.ext.us_privacy must contain 4 characters)",
WarningCode: errortypes.InvalidPrivacyConsentWarningCode}
assert.ElementsMatch(t, errL, []error{&expectedWarning})

assert.Empty(t, req.Regs.Ext, "Invalid Consent Removed From Request")
Expand Down Expand Up @@ -2104,6 +2106,54 @@ func TestIOS14EndToEnd(t *testing.T) {
assert.Equal(t, &lmtOne, result.Device.Lmt)
}

func TestAuctionWarnings(t *testing.T) {
reqBody := validRequest(t, "us-privacy-invalid.json")
deps := &endpointDeps{
&warningsCheckExchange{},
newParamsValidator(t),
&mockStoredReqFetcher{},
empty_fetcher.EmptyFetcher{},
empty_fetcher.EmptyFetcher{},
&config.Configuration{MaxRequestSize: int64(len(reqBody))},
newTestMetrics(),
analyticsConf.NewPBSAnalytics(&config.Analytics{}),
map[string]string{},
false,
[]byte{},
openrtb_ext.BuildBidderMap(),
nil,
nil,
hardcodedResponseIPValidator{response: true},
}

req := httptest.NewRequest("POST", "/openrtb2/auction", strings.NewReader(reqBody))
recorder := httptest.NewRecorder()

deps.Auction(recorder, req, nil)

if recorder.Code != http.StatusOK {
t.Errorf("Endpoint should return a 200")
}
warnings := deps.ex.(*warningsCheckExchange).auctionRequest.Warnings
assert.Len(t, warnings, 1, "One warning should be returned from exchange")

actualWarning := warnings[0].(*errortypes.Warning)
SyntaxNode marked this conversation as resolved.
Show resolved Hide resolved
expectedMessage := "CCPA consent is invalid and will be ignored. (request.regs.ext.us_privacy must contain 4 characters)"
assert.Equal(t, expectedMessage, actualWarning.Message, "Warning message is incorrect")

assert.Equal(t, errortypes.InvalidPrivacyConsentWarningCode, actualWarning.WarningCode, "Warning code is incorrect")
}

// warningsCheckExchange is a well-behaved exchange which stores all incoming warnings.
type warningsCheckExchange struct {
auctionRequest exchange.AuctionRequest
}

func (e *warningsCheckExchange) HoldAuction(ctx context.Context, r exchange.AuctionRequest, debugLog *exchange.DebugLog) (*openrtb.BidResponse, error) {
e.auctionRequest = r
return nil, nil
}

// nobidExchange is a well-behaved exchange which always bids "no bid".
type nobidExchange struct {
gotRequest *openrtb.BidRequest
Expand Down
Loading