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

Adds timeout notifications for Facebook #1182

Merged
merged 6 commits into from
Feb 10, 2020
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
3 changes: 2 additions & 1 deletion adapters/adpone/adpone.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,10 @@ package adpone
import (
"encoding/json"
"fmt"
"github.com/prebid/prebid-server/openrtb_ext"
"net/http"

"github.com/prebid/prebid-server/openrtb_ext"

"github.com/mxmCherry/openrtb"
"github.com/prebid/prebid-server/adapters"
"github.com/prebid/prebid-server/errortypes"
Expand Down
17 changes: 17 additions & 0 deletions adapters/audienceNetwork/facebook.go
Original file line number Diff line number Diff line change
Expand Up @@ -447,3 +447,20 @@ func NewFacebookBidder(client *http.Client, platformID string, appSecret string)
appSecret: appSecret,
}
}

func (fa *FacebookAdapter) MakeTimeoutNotification(req *adapters.RequestData) (*adapters.RequestData, []error) {
// Note, facebook creates one request per imp, so all these requests will only have one imp in them
auction_id, err := jsonparser.GetString(req.Body, "imp", "[0]", "id")
if err != nil {
return &adapters.RequestData{}, []error{err}
}

uri := fmt.Sprintf("https://www.facebook.com/audiencenetwork/nurl/?partner=%s&app=%s&auction=%s&ortb_loss_code=2", fa.platformID, fa.platformID, auction_id)
timeoutReq := adapters.RequestData{
Method: "GET",
Uri: uri,
Body: nil,
Headers: http.Header{},
}
return &timeoutReq, nil
}
37 changes: 37 additions & 0 deletions adapters/audienceNetwork/facebook_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@ import (
"testing"
"time"

"github.com/prebid/prebid-server/adapters"
"github.com/prebid/prebid-server/adapters/adapterstest"
"github.com/stretchr/testify/assert"
)

type tagInfo struct {
Expand Down Expand Up @@ -40,3 +42,38 @@ type FacebookExt struct {
func TestJsonSamples(t *testing.T) {
adapterstest.RunJSONBidderTest(t, "audienceNetworktest", NewFacebookBidder(nil, "test-platform-id", "test-app-secret"))
}

func TestMakeTimeoutNotice(t *testing.T) {
req := adapters.RequestData{
Body: []byte(`{"imp":[{"id":"1234"}]}}`),
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we also add another test case for an invalid json in this test?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Done

}
fba := NewFacebookBidder(nil, "test-platform-id", "test-app-secret")

tb, ok := fba.(adapters.TimeoutBidder)
if !ok {
t.Error("Facebook adapter is not a TimeoutAdapter")
}

toReq, err := tb.MakeTimeoutNotification(&req)
assert.Nil(t, err, "Facebook MakeTimeoutNotification() return an error %v", err)
expectedUri := "https://www.facebook.com/audiencenetwork/nurl/?partner=test-platform-id&app=test-platform-id&auction=1234&ortb_loss_code=2"
assert.Equal(t, expectedUri, toReq.Uri, "Facebook timeout notification not returning the expected URI.")

}

func TestMakeTimeoutNoticeBadRequest(t *testing.T) {
req := adapters.RequestData{
Body: []byte(`{"imp":[{{"id":"1234"}}`),
}
fba := NewFacebookBidder(nil, "test-platform-id", "test-app-secret")

tb, ok := fba.(adapters.TimeoutBidder)
if !ok {
t.Error("Facebook adapter is not a TimeoutAdapter")
}

toReq, err := tb.MakeTimeoutNotification(&req)
assert.Empty(t, toReq.Uri, "Facebook MakeTimeoutNotification() did not return nil", err)
assert.NotNil(t, err, "Facebook MakeTimeoutNotification() did not return an error")

}
13 changes: 13 additions & 0 deletions adapters/bidder.go
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,19 @@ type Bidder interface {
MakeBids(internalRequest *openrtb.BidRequest, externalRequest *RequestData, response *ResponseData) (*BidderResponse, []error)
}

// TimeoutBidder is used to identify bidders that support timeout notifications.
type TimeoutBidder interface {
Bidder

// MakeTimeoutNotice functions much the same as MakeRequests, except it is fed the bidder request that timed out,
// and expects that only one notification "request" will be generated. A use case for multiple timeout notifications
// has not been anticipated.
//
// Do note that if MakeRequests returns multiple requests, and more than one of these times out, MakeTimeoutNotice will be called
// once for each timed out request.
MakeTimeoutNotification(req *RequestData) (*RequestData, []error)
}

type MisconfiguredBidder struct {
Name string
Error error
Expand Down
24 changes: 24 additions & 0 deletions exchange/bidder.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
"fmt"
"io/ioutil"
"net/http"
"time"

"github.com/mxmCherry/openrtb"
nativeRequests "github.com/mxmCherry/openrtb/native/request"
Expand Down Expand Up @@ -295,6 +296,14 @@ func (bidder *bidderAdapter) doRequest(ctx context.Context, req *adapters.Reques
if err != nil {
if err == context.DeadlineExceeded {
err = &errortypes.Timeout{Message: err.Error()}
if tb, ok := bidder.Bidder.(adapters.TimeoutBidder); ok {
// Toss the timeout notification call into a go routine, as we are out of time'
// and cannot delay processing. We don't do anything result, as there is not much
// we can do about a timeout notification failure. We do not want to get stuck in
// a loop of trying to report timeouts to the timeout notifications.
go bidder.doTimeoutNotification(tb, req)
SyntaxNode marked this conversation as resolved.
Show resolved Hide resolved
}

}
return &httpCallInfo{
request: req,
Expand Down Expand Up @@ -328,6 +337,21 @@ func (bidder *bidderAdapter) doRequest(ctx context.Context, req *adapters.Reques
}
}

func (bidder *bidderAdapter) doTimeoutNotification(timeoutBidder adapters.TimeoutBidder, req *adapters.RequestData) {
ctx, cancel := context.WithTimeout(context.Background(), 200*time.Millisecond)
SyntaxNode marked this conversation as resolved.
Show resolved Hide resolved
defer cancel()
SyntaxNode marked this conversation as resolved.
Show resolved Hide resolved
toReq, errL := timeoutBidder.MakeTimeoutNotification(req)
if toReq != nil && len(errL) == 0 {
httpReq, err := http.NewRequest(toReq.Method, toReq.Uri, bytes.NewBuffer(toReq.Body))
if err == nil {
httpReq.Header = req.Headers
ctxhttp.Do(ctx, bidder.Client, httpReq)
// No validation yet on sending notifications
}
}

}

type httpCallInfo struct {
request *adapters.RequestData
response *adapters.ResponseData
Expand Down