Spec: Support Private Aggregation aggregate error reporting #1427
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This [new feature] allows for aggregate measurement of certain error conditions. [new feature]: https://github.com/patcg-individual-drafts/private-aggregation-api/blob/main/error_reporting.md
This was referenced Apr 8, 2025
alexmturner
commented
Apr 8, 2025
spec.bs
Outdated
| |global|. | ||
| 1. If |result| is an [=ECMAScript/abrupt completion=], return « "null", null, |paContributions|, null ». | ||
|
|
||
| Issue: Callers don't seem to expect nulls in the return value. |
There was a problem hiding this comment.
Note: I updated the above line to pass on the private aggregation contributions, but the other fields might need updating too.
There was a problem hiding this comment.
yep, it needs to pass on the PAgg contributions.
- The return array didn't have correct number of elements. See the last step of the algorithm, or callers of the algorithm for what's the expected return value. (it would be good if we have defined the return value format in the algorithm's declaration, but that was still a TODO).
- It seems other fields can all be null, after checking the algorithm's callers
There was a problem hiding this comment.
- Ah true -- updated to have two extra nulls
- I think there is still an issue. In particular, |executionMetrics|'s [=script timed out=] is accessed by both callers, so |executionMetrics| being null isn't properly handled there (https://pr-preview.s3.amazonaws.com/alexmturner/turtledove/pull/1427.html#ref-for-evaluate-a-reporting-script and https://pr-preview.s3.amazonaws.com/alexmturner/turtledove/pull/1427.html#ref-for-evaluate-a-reporting-script%E2%91%A0). I guess that's not too major, though.
|
@qingxinwu, when you get a chance, could you PTAL at this too? Thanks! |
|
Just realizing that this PR might not treat timeouts correctly. I'll have a go fixing that. |
aarongable
pushed a commit
to chromium/chromium
that referenced
this pull request
Apr 22, 2025
Extends support for the new aggregate error reporting feature to Protected Audience. This cl detects when an uncaught exception or other error occurs and plumbs that to the PrivateAggregationBindings. A follow up cl will use this boolean to trigger contributions conditional on the event. This cl is a no-op, but is split out for ease of reviewing. Spec PR: WICG/turtledove#1427 Bug: 381788013 Change-Id: I0da8daf58a5893ed50d90cfbe960d82ae490126f Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6426087 Commit-Queue: Alex Turner <[email protected]> Reviewed-by: Qingxin Wu <[email protected]> Cr-Commit-Position: refs/heads/main@{#1449926}
aarongable
pushed a commit
to chromium/chromium
that referenced
this pull request
Apr 22, 2025
Extends support for the new aggregate error reporting feature to Protected Audience. This cl renames some types and helpers in unittests in advance of some type renames (in the subsequent cls). This cl is a no-op, but is split out for ease of reviewing. Spec PR: WICG/turtledove#1427 Bug: 381788013 Change-Id: I9574200c7993fb51db480c751a4622ff3e754c4c Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6438496 Reviewed-by: Qingxin Wu <[email protected]> Commit-Queue: Alex Turner <[email protected]> Cr-Commit-Position: refs/heads/main@{#1449927}
aarongable
pushed a commit
to chromium/chromium
that referenced
this pull request
Apr 22, 2025
Extends support for the new aggregate error reporting feature to Protected Audience. This cl adds support for the worklet/script runner parsing and forwarding, including only keeping contributions conditional on an uncaught exception/error if that event occurs. The browser-side handling is left for a future cl except for adding mojo validation. The validation rejects any request conditional on an error event if the feature flag is disabled. This support is conditional on the feature flag, which is disabled by default. So, this cl is (for now) a no-op. The feature flag will not be enabled until after the browser-side support is added in the following (and last) cl in this chain. Spec PR: WICG/turtledove#1427 Bug: 381788013 Change-Id: I61896a0ae0b715f3edcfbf2c43260d0e817f4053 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6438253 Reviewed-by: Qingxin Wu <[email protected]> Commit-Queue: Alex Turner <[email protected]> Reviewed-by: Ken Buchanan <[email protected]> Cr-Commit-Position: refs/heads/main@{#1449940}
aarongable
pushed a commit
to chromium/chromium
that referenced
this pull request
Apr 22, 2025
Extends support for the new aggregate error reporting feature to Protected Audience. This cl adds the browser-side handling for aggregate error reporting. This requires plumbing the event to the Private Aggregation layer and ensuring that error events are only triggered for one interest group per buyer and once per seller (just like for reserved.once). This support is conditional on the feature flag, which is disabled by default. So, this cl is (for now) a no-op. The feature flag will be enabled in a separate cl. Spec PR: WICG/turtledove#1427 Bug: 381788013 Change-Id: I5960f17b887bee868dd35de5ddf6b45adc072455 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6437255 Reviewed-by: Dominic Farolino <[email protected]> Reviewed-by: Giovanni Ortuno Urquidi <[email protected]> Commit-Queue: Alex Turner <[email protected]> Reviewed-by: Qingxin Wu <[email protected]> Cr-Commit-Position: refs/heads/main@{#1449941}
qingxinwu
reviewed
Apr 22, 2025
qingxinwu
approved these changes
May 2, 2025
|
Thanks for all the reviews! |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This new feature allows for aggregate measurement of certain error
conditions. This also does a drive-by fix of reserved.once behavior for
buyers.
Preview | Diff