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 all 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
202 changes: 116 additions & 86 deletions endpoints/openrtb2/amp_auction_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -395,103 +395,119 @@ 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 {
regs *openrtb_ext.ExtRegs
invalidConsentURL bool
expectedWarnings map[openrtb_ext.BidderName][]openrtb_ext.ExtBidderMessage
}
invalidConsent := "invalid"

// Simulated Stored Request Backend
stored := map[string]json.RawMessage{"1": json.RawMessage(bid)}
bidderWarning := openrtb_ext.ExtBidderMessage{
Code: 10003,
Message: "debug turned off for bidder",
}
invalidCCPAWarning := openrtb_ext.ExtBidderMessage{
Code: 10001,
Message: "Consent '" + invalidConsent + "' is not recognized as either CCPA or GDPR TCF.",
}
invalidConsentWarning := openrtb_ext.ExtBidderMessage{
Code: 10001,
Message: "CCPA consent is invalid and will be ignored. (request.regs.ext.us_privacy must contain 4 characters)",
}

// 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(),
)
testData := []inputTest{
{
regs: nil,
invalidConsentURL: false,
expectedWarnings: nil,
},
{
regs: nil,
invalidConsentURL: true,
expectedWarnings: map[openrtb_ext.BidderName][]openrtb_ext.ExtBidderMessage{openrtb_ext.BidderReservedGeneral: {invalidCCPAWarning}},
},
{
regs: &openrtb_ext.ExtRegs{USPrivacy: "invalid"},
invalidConsentURL: true,
expectedWarnings: map[openrtb_ext.BidderName][]openrtb_ext.ExtBidderMessage{
openrtb_ext.BidderReservedGeneral: {invalidCCPAWarning, invalidConsentWarning},
openrtb_ext.BidderName("appnexus"): {bidderWarning},
},
},
{
regs: &openrtb_ext.ExtRegs{USPrivacy: "1NYN"},
invalidConsentURL: false,
expectedWarnings: map[openrtb_ext.BidderName][]openrtb_ext.ExtBidderMessage{openrtb_ext.BidderName("appnexus"): {bidderWarning}},
},
}

// Invoke Endpoint
request := httptest.NewRequest("GET", "/openrtb2/auction/amp?tag_id=1", nil)
responseRecorder := httptest.NewRecorder()
endpoint(responseRecorder, request, nil)
for _, testCase := range testData {

// Parse Response
var response AmpResponse
if err := json.Unmarshal(responseRecorder.Body.Bytes(), &response); err != nil {
t.Fatalf("Error unmarshalling response: %s", err.Error())
}
bid, err := getTestBidRequest(true, nil, testCase.regs == nil, testCase.regs)
if err != nil {
t.Fatalf("Failed to marshal the complete openrtb.BidRequest object %v", err)
}

// 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)
}
// Simulated Stored Request Backend
stored := map[string]json.RawMessage{"1": json.RawMessage(bid)}

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)
}
// Build Exchange Endpoint
var mockExchange exchange.Exchange
if testCase.regs != nil {
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(),
)

// Simulated Stored Request Backend
stored := map[string]json.RawMessage{"1": json.RawMessage(bid)}
// Invoke Endpoint
var request *http.Request

// 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(),
)
if testCase.invalidConsentURL {
request = httptest.NewRequest("GET", "/openrtb2/auction/amp?tag_id=1&consent_string="+invalidConsent, 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)
} else {
request = httptest.NewRequest("GET", "/openrtb2/auction/amp?tag_id=1", nil)
}

// Parse Response
var response AmpResponse
if err := json.Unmarshal(responseRecorder.Body.Bytes(), &response); err != nil {
t.Fatalf("Error unmarshalling response: %s", err.Error())
}
responseRecorder := httptest.NewRecorder()
endpoint(responseRecorder, request, nil)

// 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.",
},
},
// 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.regs == nil {
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 {
assert.Equal(t, testCase.expectedWarnings, response.Warnings)
} else {
assert.Empty(t, response.Warnings)
}

} else {
assert.Equal(t, testCase.expectedWarnings, response.Warnings)
}
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 +945,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 +978,21 @@ func (m *mockAmpExchange) HoldAuction(ctx context.Context, r exchange.AuctionReq
return response, nil
}

type mockAmpExchangeWarnings struct{}

func (m *mockAmpExchangeWarnings) HoldAuction(ctx context.Context, r exchange.AuctionRequest, debugLog *exchange.DebugLog) (*openrtb.BidResponse, error) {
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 +1056,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
53 changes: 52 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,55 @@ 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
if !assert.Len(t, warnings, 1, "One warning should be returned from exchange") {
t.FailNow()
}
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