Core: fpd validation library#15333
Conversation
|
Whoa there partner! This project is migrating to typescript. Consider changing the new JS files to TS, with well-defined types for what interacts with the prebid public API (for example: bid params and configuration). Thanks!
Tread carefully! This PR adds 2 linter errors (possibly disabled through directives):
|
|
Some adapters in this PR do not follow Prebid naming conventions.
|
![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: f4a7a41802
ℹ️ About Codex in GitHub
Your team has set up Codex to 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 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| function fpdWarn(...args) { | ||
| // eslint-disable-next-line no-console | ||
| console.warn(...args); |
There was a problem hiding this comment.
When validationFpdModule sees invalid ortb2 on a normal publisher page, this helper now calls console.warn directly, whereas the extracted code previously used logWarn. That bypasses Prebid's debug gating and auctionDebug emission, so validation warnings become unconditional console noise in production and listeners no longer receive the standard warning event; route these warnings through logWarn or an injectable logger instead.
Useful? React with 👍 / 👎.
|
Whoa there partner! This project is migrating to typescript. Consider changing the new JS files to TS, with well-defined types for what interacts with the prebid public API (for example: bid params and configuration). Thanks!
Tread carefully! This PR adds 2 linter errors (possibly disabled through directives):
|
Barecheck - Code coverage reportTotal: 96.58%Your code coverage diff: 0.00% ▴ Uncovered files and lines |
|
Whoa there partner! This project is migrating to typescript. Consider changing the new JS files to TS, with well-defined types for what interacts with the prebid public API (for example: bid params and configuration). Thanks!
Tread carefully! This PR adds 2 linter errors (possibly disabled through directives):
|
|
Some adapters in this PR do not follow Prebid naming conventions.
|
|
This PR introduces changes that may not work on all browsers. According to Babel, the following polyfills may be needed, and they are not automatically included:
The best way to address this is to provide good test coverage, as normal PR checks run unit tests on older browsers. |
There was a problem hiding this comment.
can we keep new files in ts pls?
|
Whoa there partner! This project is migrating to typescript. Consider changing the new JS files to TS, with well-defined types for what interacts with the prebid public API (for example: bid params and configuration). Thanks!
Tread carefully! This PR adds 1 linter error (possibly disabled through directives):
|
|
Some adapters in this PR do not follow Prebid naming conventions.
|
|
This PR introduces changes that may not work on all browsers. According to Babel, the following polyfills may be needed, and they are not automatically included:
The best way to address this is to provide good test coverage, as normal PR checks run unit tests on older browsers. |
|
This PR includes an adapter whose code does not match its file name. Bid adapter modules should be named |
|
Some adapters in this PR do not follow Prebid naming conventions.
|
|
This PR introduces changes that may not work on all browsers. According to Babel, the following polyfills may be needed, and they are not automatically included:
The best way to address this is to provide good test coverage, as normal PR checks run unit tests on older browsers. |
| } | ||
| } | ||
|
|
||
| export function validateInterceptedBidFpd(bidRequest) { |
There was a problem hiding this comment.
My understanding is that currently the FPD validation module can be useful to check your setup, but once you found / fixed any issues there's no point to leaving it in production, hence why we want it as part of debugging. That way you could just turn debugging on temporarily to find any issues.
I am not sure about doing it as part of bid interception:
- to do that kind of check you would need to intercept all bids and most issues would be duplicated for each one. Even then there may be some FPD that doesn't run through the validation (if for example some bidders do not run on every auction).
- if you are intercepting bids for other reasons (and you don't know or care about FPD) it may also be surprising to see them being modified, you don't typically expect troubleshooting tools to change the behavior of the thing they're troubleshooting.
IMO this should do something similar to the module, running on global config and bidderConfig rather than bids, and not modify it (which could be achieved just by cloning it, although the wording of the logs it generates may need to be adjusted as well).
3 participants
Type of change
Description of change
This PR extracts the FPD validation logic from validationFpdModule into a shared library under
libraries/fpdUtils/.The validation code is no longer tied to a single module and can be reused by other parts of Prebid — including bid adapters — that need to validate ortb2 first-party data.The same library is also integrated into the debugging module. When bids are intercepted, FPD validation runs against
bidRequest.ortb2and invalid fields are reported via console warnings, which helps developers spot FPD issues during local testing without relying on a live bidder response.validationFpdModuleremains responsible for running validation at auction time via the FPD pipeline; it now delegates to the shared library instead of owning the validation implementation.Other information
fixes #14968