Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 0cfc83ed48
ℹ️ About Codex in GitHub
Codex has been enabled to automatically review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
When you sign up for Codex through ChatGPT, Codex can also answer questions or update the PR, like "@codex address that feedback".
modules/aceexBidAdapter.js
Outdated
| if (!serverResponse || !serverResponse.body) return []; | ||
|
|
||
| const repackedBids = []; | ||
|
|
||
| serverResponse.body.seatbid.forEach(seatbidItem => { |
There was a problem hiding this comment.
This response handler only checks for serverResponse.body and then immediately calls serverResponse.body.seatbid.forEach(...). OpenRTB responses are allowed to omit seatbid entirely when there are no bids, and some exchanges return {} or { seatbid: null } for no-fill. In those cases this will throw and break the auction instead of returning an empty bid array. Consider defaulting to an empty array or early-returning when seatbid is not an array.
Useful? React with 👍 / 👎.
| ttl: 1200, | ||
| meta: { | ||
| advertiserDomains: [ 'aaa.com' ] | ||
| }, |
There was a problem hiding this comment.
The adapter hard-codes meta.advertiserDomains to ['aaa.com'] for every bid. This mislabels the advertiser for all responses, which can bypass publisher blocklists or incorrectly block legitimate ads, and it also violates the expectation that this field reflects adomain/advertiser domains from the exchange. Use the response-provided domain list (e.g., bid.adomain) or leave it empty when it’s missing.
Useful? React with 👍 / 👎.
|
Tread carefully! This PR adds 4 linter errors (possibly disabled through directives):
|
Pull Request Test Coverage Report for Build 21267619242Details
💛 - Coveralls |
|
@gwhigs hi, just a gentle reminder about this PR) |
|
@gwhigs Hi! Just a gentle reminder — could you please take a look at this merge request when you have a moment? |
4 participants
Type of change
Bugfix
Feature
New bidder adapter
Updated bidder adapter
Code style update (formatting, local variables)
Refactoring (no functional changes, no api changes)
Build related changes
CI related changes
Does this change affect user-facing APIs or examples documented on http://prebid.org?
Other
Description of change
Other information