Skip to content

Conversation

@horo-t
Copy link
Member

@horo-t horo-t commented Jan 13, 2019

I want to introduce an OPTIONAL additional body to the algorithm of "Generate a network error report".

This will be used to send the signed exchange reports #99 from the spec of "Loading Signed Exchanges" WICG/webpackage#374.


Preview | Diff

index.html Outdated
</dl>
</li>
<li>
If <var>additional body</var> is present, set each properties of
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't precise enough. I think you intend to run approximately the algorithm from Object.assign, which nails down whether non-enumerable properties are included, what happens if the argument is a proxy, etc.

Using a WebIDL dictionary for additional body would be easier to specify precisely, and would satisfy @igrigorik's concern in #99 (comment).

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Uploaded 72f2bd6 which introduces AdditionalReportBody dictionary.
How about this?

@horo-t
Copy link
Member Author

horo-t commented Jan 25, 2019

@igrigorik, @dcreager Could you please review this PR?

I have been creating a chromium CL for the distributor side reporting.
https://crrev.com/c/1355020

@horo-t horo-t changed the title Introduce additional body of error report Introduce a new algorithm "Deliver a network report". Jan 28, 2019
Copy link
Member

@dcreager dcreager left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @horo-t, I think this is a much cleaner change now!

Copy link
Member

@igrigorik igrigorik left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

Ditto, great work @horo-t!

Copy link
Member

@jyasskin jyasskin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A couple nits, but overall this looks fine.

@dcreager dcreager merged commit 7214c61 into w3c:gh-pages Feb 13, 2019
@horo-t horo-t deleted the additionalbody branch February 28, 2019 03:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants