-
Notifications
You must be signed in to change notification settings - Fork 233
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
runAdAuction Input Validation for Multi-Seller #1168
Comments
You can call runAdAuction on a component with an empty set of bidders and see if it throws, or even with an already aborted abort signals. Admittedly, the latter option probably won't prevent loading all interest groups from disk, so is not entirely free - so ideally would only resort to that if the main auction throws. |
Iteratively modifying each component seller auction configs to (temporarily) remove the interest group buyers sounds like a potentially undesirable top-level seller behavior; we generally view these configs as opaque JSON blobs passed verbatim from third-parties. Awaiting multiple async runAdAuction calls for a single auction also sounds problematic for latency. However, this approach would be better than abandoning the auction entirely or maintaining a series of user-space input validations. We'll experiment with an integration, but it would still be useful if Chrome explored more explicit validation checks long-term, especially considering the level of complexity and room for error in this config object. Tangentially, this problem reminds me of #759. Microsoft has some great code generation that converts IDLs into TypeScript types. I would love to see PAAPI reach that level of support and improved web platform ergonomics as well. |
I agree that this might be a relatively slow affair, since each individual component would be tested in its own async operation. So this is surely something to only do once the real auction has failed and you're trying to diagnose an error. And I certainly understand why you don't want to run a real auction with modified configs. As Matt said, emptying out the To make this explicit, here's the kind of debugging function that I think we're suggesting could help for now: // Call this function with a config that made runAdAuction throw an error.
// You will get back a Promise for an object with two properties:
// 'bad' : A list of all the component configs that caused errors
// 'good' : A new auction config that does not include the bad components
// If 'good' is null, then the original auction config has problems of its own,
// that are not caused by component auctions.
async function debugConfig(config) {
if (!config['componentAuctions'] || !config['componentAuctions'].length) {
return {'good' : null, 'bad' : null};
}
goodComponentConfigs = [];
badComponentConfigs = [];
miniConfig = structuredClone(config);
await Array.fromAsync(config['componentAuctions'], async (component) => {
// Make an auction config containing only one component seller...
miniConfig['componentAuctions'] = [ structuredClone(component) ];
// ...and zero buyers, so that we just error-check the component config.
miniConfig['componentAuctions'][0]['interestGroupBuyers'] = [];
return navigator.runAdAuction(miniConfig).then(
()=>goodComponentConfigs.push(component),
()=>badComponentConfigs.push(component))
});
miniConfig['componentAuctions'] = goodComponentConfigs;
return {'good' : miniConfig,
'bad' : badComponentConfigs};
} |
Worth noting that "structuredClone()" would need to be smart enough to handle input arguments that are promises (probably by passing them in unmodified). I think [{ ... , component }] would be sufficient here - no need to clone the entire thing, since we don't modify the input auction, just need to copy the component auction itself so can clear buyers. Could even stash buyers in a temporary, start the promise, restore it, and then await the promise. |
One interesting oddity: If a promise resolves to an invalid value, only the component with the bad-valued promise will fail. The rest of the auction will run and complete without issue, I believe, as long as it's not the top-level auction with the bad value. |
In multi-seller auctions, the top-level seller is responsible for assembling all sellers' component auction configs into a single
runAdAuction
call. These component auction configs often come directly from different sellers and may accidentally include malformed fields.Upon receiving a malformed field anywhere inside of the auction config (including component auctions),
runAdAuction
currently rejects the entire auction. This is illustrated in the example below[1], which causes Chrome to throw an exception and prevents any valid, well-formed auction configs from executing which diminishes PAAPI utility.Top-level sellers may instead choose to implement their own input validation on component auction configs and surface errors to third-parties directly over their own interfaces. This is problematic due to the top-level seller burden/risk of porting and maintaining internal browser logic to catch invalid inputs early.
One option would be for Chrome to only warn if an invalid component auction config is provided and still continue executing all valid component-level auctions and the top-level auction. This is problematic due to a lack of feedback loop for broken component sellers and reduced JS error observability, thus concealing potential errors.
Another option would be for Chrome to provide an explicit auction config validation API that a top-level seller may use to filter out each invalid component auction config prior to invoking the auction. To avoid incurring additional latency for the valid majority of auction configs, this API could return synchronously or instead be invoked lazily iff
runAdAuction
rejects with aTypeError
.Example of an Invalid Auction Config
The text was updated successfully, but these errors were encountered: