Bidder-specific imp FPD #3704
Bidder-specific imp FPD #3704
Conversation
| "durfloors": [{ | ||
| "mindur": 1, | ||
| "maxdur": 30, | ||
| "bidfloor": 10.0 | ||
| }], |
There was a problem hiding this comment.
Why is the merge function merging objects inside of an array instead of just selecting the overriding array? I was expecting this to be:
"durfloors": [{
"maxdur": 30
}],
There was a problem hiding this comment.
This is a difference between merging approaches from an earlier PR. Good find. Let's address separately.
| for _, err := range errs { | ||
| if errortypes.ReadCode(err) == errortypes.InvalidImpFirstPartyDataErrorCode { | ||
| return nil, err | ||
| } | ||
| } |
There was a problem hiding this comment.
Consider replacing this loop to existing ContainsFatalError function, it is also used in Line325
if errortypes.ContainsFatalError([]error{err}) {
return nil, err
}There was a problem hiding this comment.
Ah this is the direction I was originally going in but figured that it could change the existing behavior for other types of errors and could also introduce a scenario where there are multiple fatal errors that need to be aggregated. As a result, I decided to take an approach that did not disrupt current behavior (even though it is currently flawed) and just handle this one scenario. In that case we can tackle error handling as a whole at some point and address these cases properly.
There was a problem hiding this comment.
Ok, this works. this is just a minor optional suggestion. Approved!
Implements #2335
This PR merges bidder-specific imp first party data into the applicable bidder request. After merging the imps are revalidated. If there is a validation error, it is considered a fatal error which means the auction is halted and the error is reported back to the client.
While attempting to implement merge and validation error handling, I noticed that several preexisting errors captured in
cleanOpenRTBRequestsare actually swallowed and never reported to the client. Consequently, this also effects errors from failed imp FPD merges. In order to ensure imp FPD merge errors are made known to the client while avoiding a large general error handling refactor, I created an imp FPD merge error type with a fatal severity and detect when it occurs so I can halt the auction and send the error to the client.