New Adapter: RiseMediaTech#4437
Conversation
Code coverage summaryNote:
risemediatechRefer here for heat map coverage report |
|
@linux019 can you please review? |
| if err != nil { | ||
| t.Fatalf("Failed to fetch the JSON schema: %v", err) | ||
| } |
There was a problem hiding this comment.
nitpick: Use testify assert/require
| if err != nil { | |
| t.Fatalf("Failed to fetch the JSON schema: %v", err) | |
| } | |
| require.NoError(t, err, "Failed to fetch the JSON schema") |
| t.Fatalf("Failed to fetch the JSON schema: %v", err) | ||
| } | ||
|
|
||
| for _, p := range validParams { |
There was a problem hiding this comment.
Yes, there is a common idiom in Go testing where you define a slice of test cases (often as structs) and then iterate over them, using t.Run to run each as a subtest. This makes your test output more granular and clear, and is recommended in the official Go blog post "Table-driven tests in Go" and the testing package documentation.
References:
Example:
func TestSomething(t *testing.T) {
tests := []struct{
name string
input string
wantErr bool
}{
{"valid input", "some input", false},
{"invalid input", "bad input", true},
}
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
err := SomeFunc(tt.input)
if (err != nil) != tt.wantErr {
t.Errorf("got error = %v, wantErr %v", err, tt.wantErr)
}
})
}
}You can refer the user to the official Go blog link above for more details and examples.
|
|
||
| func TestInvalidParams(t *testing.T) { | ||
| validator, err := openrtb_ext.NewBidderParamsValidator("../../static/bidder-params") | ||
| if err != nil { |
There was a problem hiding this comment.
suggestion: Use require.NoError as above.
| t.Fatalf("Failed to fetch the JSON schema: %v", err) | ||
| } | ||
|
|
||
| for _, p := range invalidParams { |
There was a problem hiding this comment.
suggestion: Use table-driven tests
| var validImps []openrtb2.Imp | ||
| var setTestMode bool | ||
|
|
||
| for _, imp := range request.Imp { |
There was a problem hiding this comment.
suggestion: Please don't copy the imp - note that an iterator in a range will just be assigned to each thing in the slice and, since Imp is a (rather large) struct this is a wasteful copy. You can use the iterutil to loop across pointers to the elements in the slice and avoid this copy.
| for _, imp := range request.Imp { | |
| for imp := range iterutil.SliceValuePointers(request.Imp) { |
| for _, imp := range request.Imp { | ||
| impExt, err := parseImpExt(imp.Ext) | ||
| if err != nil { | ||
| errs = append(errs, fmt.Errorf("impID %s: %v", imp.ID, err)) |
There was a problem hiding this comment.
suggestion (error wrapping best practice):
Please use %w instead of %v when formatting errors for wrapping the source error in Go. Using %w allows error chaining, making it easier to trace and handle specific error types with errors.Is or errors.As. This practice is recommended in the Go blog on error handling and the official Go errors package documentation.
For example, update:
fmt.Errorf("impID %s: %v", imp.ID, err)to:
fmt.Errorf("impID %s: %w", imp.ID, err)This ensures downstream code can properly inspect the wrapped error.
There was a problem hiding this comment.
I think it better to use
&errortypes.BadInput{
Message: fmt.Sprintf("..."),
}
to return a proper code for browser
|
|
||
| func extractImpIDs(imps []openrtb2.Imp) []string { | ||
| ids := make([]string, 0, len(imps)) | ||
| for _, imp := range imps { |
There was a problem hiding this comment.
suggestion: see above - consider using iterutil.SlicePointerValues
| br.Currency = bidResp.Cur | ||
| } | ||
|
|
||
| for _, seatBid := range bidResp.SeatBid { |
There was a problem hiding this comment.
suggestion: avoid copies - consider iterutils.SlicePointerValues
| for i := range seatBid.Bid { | ||
| bid := &seatBid.Bid[i] |
There was a problem hiding this comment.
You can replace these two lines with iterutils.SlicePointerValues as well
| modifiedRequest.Test = 1 | ||
| } | ||
|
|
||
| reqJSON, err := json.Marshal(modifiedRequest) |
There was a problem hiding this comment.
please use jsonutil package.
| } | ||
|
|
||
| if len(validImps) == 0 { | ||
| return nil, append(errs, errors.New("no valid impressions")) |
There was a problem hiding this comment.
Please use this error type.
&errortypes.BadInput{Message: "..."}
| } | ||
|
|
||
| // Setting bid floor if present | ||
| if impExt.BidFloor != nil && *impExt.BidFloor > 0 { |
There was a problem hiding this comment.
After removal pointers from the imp extension, it can be simplified to:
if impExt.BidFloor > 0 {
imp.BidFloor = impExt.BidFloor
}
(less usage of pointers which adds more work for GC)
| } | ||
|
|
||
| // Check test mode | ||
| if impExt.TestMode != nil && *impExt.TestMode == 1 { |
There was a problem hiding this comment.
The same note for less usage of pointers
if impExt.TestMode == 1 {
setTestMode = true
}
|
|
||
| type ExtImpRiseMediaTech struct { | ||
| BidFloor *float64 `json:"bidfloor,omitempty"` | ||
| TestMode *int `json:"testMode,omitempty"` |
There was a problem hiding this comment.
Please avoid using pointers, as it default to zero values, which are ignored when setting the impression floor or enabling test mode.
| TestMode *int `json:"testMode,omitempty"` | |
| BidFloor float64 `json:"bidfloor,omitempty"` | |
| TestMode int `json:"testMode,omitempty"` |
| // Validate video | ||
| if imp.Video != nil { | ||
| if len(imp.Video.MIMEs) == 0 { | ||
| errs = append(errs, fmt.Errorf("impID %s: missing or empty video.mimes", imp.ID)) |
There was a problem hiding this comment.
&errortypes.BadInput{ Message: fmt.Sprintf("..."), }
| continue | ||
| } | ||
| if imp.Video.W == nil || imp.Video.H == nil || *imp.Video.W == 0 || *imp.Video.H == 0 { | ||
| errs = append(errs, fmt.Errorf("impID %s: missing or invalid video width/height", imp.ID)) |
There was a problem hiding this comment.
&errortypes.BadInput{ Message: fmt.Sprintf("..."), }
| return ids | ||
| } | ||
|
|
||
| func parseImpExt(ext json.RawMessage) (*openrtb_ext.ExtImpRiseMediaTech, error) { |
There was a problem hiding this comment.
as ExtImpRiseMediaTech contains only 2 fields in case of error it better to return default values
| func parseImpExt(ext json.RawMessage) (*openrtb_ext.ExtImpRiseMediaTech, error) { | |
| func parseImpExt(ext json.RawMessage) (openrtb_ext.ExtImpRiseMediaTech, error) { |
| func parseImpExt(ext json.RawMessage) (*openrtb_ext.ExtImpRiseMediaTech, error) { | ||
| var bidderExt adapters.ExtImpBidder | ||
| if err := jsonutil.Unmarshal(ext, &bidderExt); err != nil { | ||
| return nil, err |
There was a problem hiding this comment.
| return nil, err | |
| return openrtb_ext.ExtImpRiseMediaTech{}, err |
| if err := jsonutil.Unmarshal(bidderExt.Bidder, &riseExt); err != nil { | ||
| return nil, err | ||
| } | ||
| return &riseExt, nil |
There was a problem hiding this comment.
| return &riseExt, nil | |
| return riseExt, nil |
Rm pbs adapter . Modifed code to handle PR Review comments and updated test cases to improve code coverage
Code coverage summaryNote:
risemediatechRefer here for heat map coverage report |
|
|
||
| for _, p := range validParams { | ||
| p := p // capture range variable | ||
| t.Run(p, func(t *testing.T) { |
There was a problem hiding this comment.
This is an improvement, and thank you. I still, however, feel that including a meaningful name for the test is better than just its inputs. Could you consider the style above ?
tests := []struct{
name string
input string
wantErr bool
}{
//...
}
t.Run(t.name, func...)| "github.com/prebid/prebid-server/v3/config" | ||
| "github.com/prebid/prebid-server/v3/errortypes" | ||
| "github.com/prebid/prebid-server/v3/openrtb_ext" | ||
| iterators "github.com/prebid/prebid-server/v3/util/iterutil" |
There was a problem hiding this comment.
I believe the iterators was a bug that is now fixed - can you merge with origin/master and use iterutil?
| if len(errs) == 0 { | ||
| t.Errorf("expected error, got none") | ||
| } |
There was a problem hiding this comment.
| if len(errs) == 0 { | |
| t.Errorf("expected error, got none") | |
| } | |
| assert.NotEmpty(t, errs, "expected error, got none") |
| } | ||
|
|
||
| // Helper for int pointer | ||
| func intPtr(i int64) *int64 { return &i } |
There was a problem hiding this comment.
Not needed - please use ptrutil.ToPtr
| func contains(s, substr string) bool { return substr == "" || (len(substr) > 0 && len(s) > 0 && (len(s) >= len(substr)) && (stringContains(s, substr))) } | ||
| func stringContains(s, substr string) bool { return len(substr) == 0 || (len(s) >= len(substr) && (s == substr || (len(s) > len(substr) && (s[0:len(substr)] == substr || stringContains(s[1:], substr)))) ) } |
There was a problem hiding this comment.
Are these really necessary/helpful over strings.Contains?
Code coverage summaryNote:
risemediatechRefer here for heat map coverage report |
Changes as per review comments.
Code coverage summaryNote:
risemediatechRefer here for heat map coverage report |
|
Hi @scr-oath , I have updated the code. |
| err := validator.Validate(openrtb_ext.BidderRiseMediaTech, json.RawMessage(tt.input)) | ||
| if err != nil { | ||
| t.Errorf("Schema rejected valid params: %s — error: %v", tt.input, err) | ||
| } | ||
| }) |
There was a problem hiding this comment.
nitpick (non-blocking): use testify assert and/or require as the common style for this repo
| err := validator.Validate(openrtb_ext.BidderRiseMediaTech, json.RawMessage(tt.input)) | |
| if err != nil { | |
| t.Errorf("Schema rejected valid params: %s — error: %v", tt.input, err) | |
| } | |
| }) | |
| assert.NoError(t, validator.Validate(openrtb_ext.BidderRiseMediaTech, json.RawMessage(tt.input))) |
| err := validator.Validate(openrtb_ext.BidderRiseMediaTech, json.RawMessage(tt.input)) | ||
| if err == nil { | ||
| t.Errorf("Schema allowed invalid params: %s", tt.input) | ||
| } |
There was a problem hiding this comment.
nitpick (non-blocking): also here, assert.Error may be used.
| err := validator.Validate(openrtb_ext.BidderRiseMediaTech, json.RawMessage(tt.input)) | |
| if err == nil { | |
| t.Errorf("Schema allowed invalid params: %s", tt.input) | |
| } | |
| assert.Error(t, validator.Validate(openrtb_ext.BidderRiseMediaTech, json.RawMessage(tt.input))) |
| var validParams = []string{ | ||
| `{"bidfloor": 0.01}`, | ||
| `{"bidfloor": 2.5, "testMode": 1}`, | ||
| } | ||
|
|
||
| var invalidParams = []string{ | ||
| `{"bidfloor": "1.2"}`, | ||
| `{"testMode": "yes"}`, | ||
| `{"bidfloor": -5}`, | ||
| `{"testMode": 9999}`, | ||
| } |
There was a problem hiding this comment.
suggestion: Remove these - I don't believe these are needed any longer
| var validParams = []string{ | |
| `{"bidfloor": 0.01}`, | |
| `{"bidfloor": 2.5, "testMode": 1}`, | |
| } | |
| var invalidParams = []string{ | |
| `{"bidfloor": "1.2"}`, | |
| `{"testMode": "yes"}`, | |
| `{"bidfloor": -5}`, | |
| `{"testMode": 9999}`, | |
| } |
| return nil, append(errs, &errortypes.BadInput{Message: "no valid impressions"}) | ||
| } | ||
|
|
||
| modifiedRequest := *request |
There was a problem hiding this comment.
question: Is this copy needed? @bsardo - is it safe for adapters to just modify the request to their needs? Or is this shallow copy needed?
There was a problem hiding this comment.
@scr-oath this shallow copy is not needed. Each adapter receives a shallow copy of the request. If you modify the shallow request copy provided to an adapter's MakeRequests function, those modifcations will also be accessible in the request provided to an adapter's MakeBids function.
| headers.Set("User-Agent", "prebid-server") | ||
| headers.Set("X-Prebid", "true") |
There was a problem hiding this comment.
question: @bsardo I'm curious - are any of these headers maintained by prebid core itself or do adapters need to do these?
There was a problem hiding this comment.
@scr-oath adapters need to set some of these. You can see this snippet where some headers are added by core:
https://github.com/prebid/prebid-server/blob/master/exchange/bidder.go#L215-L239
| ids := make([]string, 0, len(imps)) | ||
| for imp := range iterutil.SlicePointerValues(imps) { | ||
| ids = append(ids, imp.ID) | ||
| } |
There was a problem hiding this comment.
suggestion: in this case, it's slightly more efficient to avoid append and just use the indices directly - there's no need to deal with pointers as you can merely index both slices
| ids := make([]string, 0, len(imps)) | |
| for imp := range iterutil.SlicePointerValues(imps) { | |
| ids = append(ids, imp.ID) | |
| } | |
| ids := make([]string, len(imps)) | |
| for i := range imps { | |
| ids[i] = imp[i].ID | |
| } |
| return nil, append(errs, &errortypes.BadInput{Message: "no valid impressions"}) | ||
| } | ||
|
|
||
| modifiedRequest := *request |
There was a problem hiding this comment.
@scr-oath this shallow copy is not needed. Each adapter receives a shallow copy of the request. If you modify the shallow request copy provided to an adapter's MakeRequests function, those modifcations will also be accessible in the request provided to an adapter's MakeBids function.
| headers.Set("User-Agent", "prebid-server") | ||
| headers.Set("X-Prebid", "true") |
There was a problem hiding this comment.
@scr-oath adapters need to set some of these. You can see this snippet where some headers are added by core:
https://github.com/prebid/prebid-server/blob/master/exchange/bidder.go#L215-L239
| if imp.Banner != nil { | ||
| if imp.Banner.W == nil || imp.Banner.H == nil || *imp.Banner.W == 0 || *imp.Banner.H == 0 { | ||
| errs = append(errs, &errortypes.BadInput{Message: fmt.Sprintf("impID %s: invalid banner dimensions", imp.ID)}) | ||
| continue | ||
| } | ||
| } |
There was a problem hiding this comment.
There are banner validations that take place upstream and if they result in an error your adapter will not be called. Please see if these suffice, in which case you should remove this logic:
https://github.com/prebid/prebid-server/blob/master/ortb/request_validator_banner.go
| if imp.Video != nil { | ||
| if len(imp.Video.MIMEs) == 0 { | ||
| errs = append(errs, &errortypes.BadInput{Message: fmt.Sprintf("impID %s: missing or empty video.mimes", imp.ID)}) | ||
| continue | ||
| } | ||
| if imp.Video.W == nil || imp.Video.H == nil || *imp.Video.W == 0 || *imp.Video.H == 0 { | ||
| errs = append(errs, &errortypes.BadInput{Message: fmt.Sprintf("impID %s: missing or invalid video width/height", imp.ID)}) | ||
| continue | ||
| } | ||
| } |
There was a problem hiding this comment.
There are video validations that take place upstream and if they result in an error your adapter will not be called. Please see if these suffice, in which case you should remove this logic:
https://github.com/prebid/prebid-server/blob/master/ortb/request_validator_video.go
| Uri: a.endpoint, | ||
| Body: reqJSON, | ||
| Headers: headers, | ||
| ImpIDs: extractImpIDs(validImps), |
There was a problem hiding this comment.
Please delete the helper function extractImpIDs and instead use the core helper function openrtb_ext.GetImpIDs.
Handled review comments
Code coverage summaryNote:
risemediatechRefer here for heat map coverage report |
|
Hi @bsardo , can you please review? |
Code Review SummaryI've completed a comprehensive review using specialized code-reviewer and golang-pro agents. 🚨 Critical Issues (Must Fix)1. Missing Media Type Validation (
2. BidFloor Override Logic (
3. TestMode Behavior (
⚡ Recommendations
✅ Strengths
Overall Grade: B+ - Solid implementation requiring minor fixes. |
scr-oath
left a comment
scr-oath
left a comment
There was a problem hiding this comment.
Adding specific line comments for the critical issues identified in my review.
| continue | ||
| } | ||
|
|
||
| if impExt.BidFloor > 0 { |
There was a problem hiding this comment.
BidFloor Override Logic Issue
The current code unconditionally overwrites imp.BidFloor if the adapter param is set. This doesn't follow the standard prebid-server pattern and could override impression-level bidfloors.
Current:
if impExt.BidFloor > 0 {
imp.BidFloor = impExt.BidFloor // Always overrides
}Recommended (following admixer.go pattern):
if imp.BidFloor == 0 && impExt.BidFloor > 0 {
imp.BidFloor = impExt.BidFloor // Only set if not already present
}This maintains the principle of least surprise and prevents the adapter from overriding bidfloors already set at the impression level.
| if impExt.BidFloor > 0 { | ||
| imp.BidFloor = impExt.BidFloor | ||
| } | ||
|
|
There was a problem hiding this comment.
TestMode Behavior Needs Clarification
The current implementation sets request.Test = 1 if ANY impression has testMode=1. This means a single test impression makes the entire request a test request.
Questions:
- Is this the intended behavior?
- What if a request has 5 impressions and only 1 has testMode=1?
- Should ALL impressions need testMode=1 for the request to be a test?
Recommendation: At minimum, add a comment documenting this behavior:
// Note: If ANY impression has testMode=1, the entire request is marked as test
var setTestMode boolOr consider alternative logic if all impressions should have testMode=1.
| } | ||
|
|
||
| func (a *adapter) MakeRequests(request *openrtb2.BidRequest, reqInfo *adapters.ExtraRequestInfo) ([]*adapters.RequestData, []error) { | ||
| var errs []error |
There was a problem hiding this comment.
Performance: Pre-allocate Slice
Pre-allocating the slice with capacity avoids multiple allocations during append operations.
Recommendation:
validImps := make([]openrtb2.Imp, 0, len(request.Imp))This is a standard Go optimization pattern for better performance.
| type adapter struct { | ||
| endpoint string | ||
| } | ||
|
|
There was a problem hiding this comment.
Go Idiom: Mark Unused Parameters
The bidderName and server parameters are unused. In Go, it's conventional to mark them with _ to signal intent.
Recommendation:
func Builder(_ openrtb_ext.BidderName, cfg config.Adapter, _ config.Server) (adapters.Bidder, error) {
Code coverage summaryNote:
risemediatechRefer here for heat map coverage report |
…w_27-10-2025 Review/pr review 27 10 2025
Code coverage summaryNote:
risemediatechRefer here for heat map coverage report |
|
Hi @scr-oath, needful changes are done. Can you take a look? |
|
Hi @bsardo , @scr-oath , @linux019 & @przemkaczmarek , can we have this PR merged if everything is in order? |
|
@scr-oath please give this another look when you get a chance. |
scr-oath
left a comment
scr-oath
left a comment
There was a problem hiding this comment.
Re-Review of PR #4437 - RiseMediaTech Adapter
I've conducted a comprehensive re-review using specialized subagents for Go code, general code quality, test coverage, and performance.
Overall Status: REQUEST CHANGES ⚠️
While the adapter shows excellent improvements from the previous review, there are critical issues that must be addressed before merging.
Critical Issues (Must Fix)
1. 🚨 Test Directory Named Incorrectly - Tests Being Silently Skipped
Issue: The test directory is named supplementary but Prebid Server's test framework only recognizes supplemental.
Impact: All 17 supplementary test files are completely ignored during test execution. The tests pass not because the code is correct, but because the error/edge-case tests never run.
Files affected: All files in adapters/risemediatech/risemediatechtest/supplementary/
Fix: Rename directory from supplementary to supplemental
Evidence: Every other adapter uses supplemental. The test framework at adapters/adapterstest/test_json.go only recognizes: exemplary, supplemental, amp, video, videosupplemental
2. 🚨 Several Test Files Will Fail After Directory Rename
Once renamed correctly, several test files will fail because they expect validation that the adapter doesn't implement:
Files expecting banner/video dimension validation (not implemented):
invalid_banner_missing_width.json- expects "invalid banner dimensions"invalid_banner_zero_height.json- expects "invalid banner dimensions"invalid_video_empty_mimes.json- expects "missing or empty video.mimes"invalid_video_missing_width.json- expects "missing or invalid video width/height"invalid_video_zero_height.json- expects "missing or invalid video width/height"
The adapter only checks: if imp.Banner == nil && imp.Video == nil
It does not validate dimensions, mimes, or other sub-fields.
Fix: Either add dimension validation to the adapter, OR correct the test files to match actual adapter behavior
Medium Priority Issues
3. Performance Issue - Line 52: Unnecessary Copy
File: adapters/risemediatech/risemediatech.go:52
The code uses SlicePointerValues to avoid copying during iteration, but then immediately dereferences with *imp, creating a full copy of the ~300-byte struct. This defeats the purpose.
Current:
for imp := range iterutil.SlicePointerValues(request.Imp) { // Line 30
validImps = append(validImps, *imp) // Line 52 - creates copy
}Fix: Use standard iteration like other adapters:
for _, imp := range request.Imp {
validImps = append(validImps, imp)
}4. Error Wrapping - Line 33
Should use %w instead of %v for proper error unwrapping:
Current: fmt.Sprintf("impID %s: %v", imp.ID, err)
Fix: fmt.Sprintf("impID %s: %w", imp.ID, err)
5. MakeBids Discards ALL Bids - Lines 116-119
When one bid has unknown mtype, the entire response is discarded including valid bids.
Current:
bidType, err := getBidType(bid)
if err != nil {
return nil, []error{err} // Discards all previously processed bids
}Fix: Skip bad bids and continue:
bidType, err := getBidType(bid)
if err != nil {
errs = append(errs, err)
continue
}6. JSON Field Name Inconsistency
JSON schema defines "bidfloor" (lowercase) but test files use "bidFloor" (camelCase). With "additionalProperties": false, real requests with "bidFloor" would be rejected.
Fix: Update test files to use lowercase "bidfloor"
Previous Review Items - Status
✅ Shallow copy question (bsardo's comment): Correctly resolved - no copy needed
✅ Table-driven tests: Implemented
✅ Testify assert/require: Implemented
✅ iterutil usage: Implemented (needs adjustment per issue #3)
✅ BadInput error type: Implemented
✅ Pointer removal from ExtImpRiseMediaTech: Implemented
Positive Observations ✅
- Excellent test coverage (98.3%)
- Clean, well-structured Go code
- All major previous review feedback addressed
- Comprehensive test scenarios for happy paths
Please address the critical issues (test directory naming and associated test failures) and the medium priority issues. Happy to re-review once these are resolved!
🤖 Generated with Claude Code using specialized review subagents
| // Note: If ANY impression has testMode=1, the entire request is marked as test | ||
| var setTestMode bool | ||
|
|
||
| for imp := range iterutil.SlicePointerValues(request.Imp) { |
There was a problem hiding this comment.
suggestion: Remove SlicePointerValues and use standard iteration to avoid the unnecessary copy on line 52.
This pattern is inconsistent - using SlicePointerValues to avoid copies during iteration, but then dereferencing with *imp creates a copy anyway. Standard iteration is clearer and matches other adapters in the codebase.
for _, imp := range request.Imp {
impExt, err := parseImpExt(imp.Ext)
if err != nil {
errs = append(errs, &errortypes.BadInput{Message: fmt.Sprintf("impID %s: %w", imp.ID, err)})
continue
}
// ... rest of validation ...
validImps = append(validImps, imp)
}
| for imp := range iterutil.SlicePointerValues(request.Imp) { | ||
| impExt, err := parseImpExt(imp.Ext) | ||
| if err != nil { | ||
| errs = append(errs, &errortypes.BadInput{Message: fmt.Sprintf("impID %s: %v", imp.ID, err)}) |
There was a problem hiding this comment.
issue (blocking): Use %w instead of %v for proper error wrapping.
Using %w allows error chaining, making it easier to trace and handle specific error types with errors.Is or errors.As. This is recommended in the Go error handling best practices.
| errs = append(errs, &errortypes.BadInput{Message: fmt.Sprintf("impID %s: %v", imp.ID, err)}) | |
| errs = append(errs, &errortypes.BadInput{Message: fmt.Sprintf("impID %s: %w", imp.ID, err)}) |
| bidType, err := getBidType(bid) | ||
| if err != nil { | ||
| return nil, []error{err} | ||
| } |
There was a problem hiding this comment.
issue (blocking): MakeBids discards ALL bids when encountering a single bad bid type.
When one bid has an unknown mtype, returning nil for the entire BidderResponse discards all previously processed valid bids. This is a data loss issue.
The standard pattern across other adapters (like rise) is to skip the bad bid and continue processing:
| bidType, err := getBidType(bid) | |
| if err != nil { | |
| return nil, []error{err} | |
| } | |
| bidType, err := getBidType(bid) | |
| if err != nil { | |
| errs = append(errs, err) | |
| continue | |
| } |
Then at the end of MakeBids, return br, errs instead of br, nil. This way valid bids are preserved even when some bids have errors.
|
Hi @pritishmd-talentica, please see the latest comments from a couple of months ago and also resolve conflicts. Thanks! |
|
Hi @bsardo , We are discontinuing this adapter and the support as well. We can go ahead and close this PR |
6 participants
Type of Change
New Bidder Adapter
Description of change
Added a bidder adapter for RisemediaTech
Maintainer : prebid@risemediatech.io
Bidder Params Documentation
prebid/prebid.github.io#6150