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

unmarshal of imp.ext.{bidder} fails in dealTier flow after addition of tid field in impression #2647

Closed
AvinashKapre opened this issue Mar 16, 2023 · 1 comment · Fixed by #2829
Assignees
Labels

Comments

@AvinashKapre
Copy link
Contributor

In the PR: #2412 a mandatory field tid was introduced at imp.ext due to which ReadDealTiersFromImp() function fails to unmarshal the ext for usecase imp.ext.{bidder}

i.e: This sample input fails to unmarshal into type impExt

"imp": [
    {
        "ext": {
            "appnexus": {
                "dealtier": {
                    "prefix": "apnx",
                    "mindealtier": 4
                }
            }
            "tid": "1234"
        }
    }
]
// for imp.ext.{bidder}
var impExt map[string]struct {
		DealTier *DealTier `json:"dealTier"`
}

Current behaviour: ReadDealTiersFromImp() returns without dealtier specified in the new location imp.ext.prebid.{bidder}.
Expected behaviour: As imp.ext.{bidder} is deprecated, we should ignore the unmarhal error for it and proceed to check for dealtier in the new location imp.ext.prebid.{bidder}.

Shall I go ahead and submit a fix for the same do we want to support the new field tid for deprecated field as well?. I'll update the code accordingly.

@bsardo
Copy link
Collaborator

bsardo commented Jun 7, 2023

Hi @AvinashKapre,

You're correct that there is a problem here as I was able to reproduce the issue. The deal tiers are not being captured from the request due to the presence of the tid field or any other primitive type in imp.ext.

I'll push up a fix that removes the logic that looks for deal tiers at imp.ext.BIDDER since all bidder information at imp.ext.BIDDER is migrated to imp.ext.prebid.bidder.BIDDER upstream during request validation in validateImpExt before ReadDealTiersFromImp is executed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: Done
Development

Successfully merging a pull request may close this issue.

2 participants