Skip to content

[speculation-rules] Fire error events for invalid speculation rules #53821

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jul 22, 2025
Merged

[speculation-rules] Fire error events for invalid speculation rules #53821

merged 1 commit into from
Jul 22, 2025

Conversation

@chromium-wpt-export-bot
Copy link
Collaborator

@chromium-wpt-export-bot chromium-wpt-export-bot commented Jul 18, 2025 •
edited by jonathan-j-lee
Loading

Change-Id: I9b0776b86059f6c8734d57c17f50ed26e89215da

@chromium-wpt-export-bot chromium-wpt-export-bot marked this pull request as ready for review July 18, 2025 04:33
wpt-pr-bot
wpt-pr-bot approved these changes Jul 18, 2025
View reviewed changes
Copy link
Collaborator

@wpt-pr-bot wpt-pr-bot left a comment

Choose a reason for hiding this comment

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

The review process for this patch is being conducted in the Chromium project.

@domenic @jonathan-j-lee
Fire error events for invalid speculation rules
0d8d00f 
This CL introduces error handling for <script type=speculationrules>,
mirroring the behavior of <script type=importmap>.

Specifically, it adds error events for two cases:
- Inline speculation rules with unparsable JSON.
- External speculation rules, which are not yet supported.

This is verified by new web platform tests.

This follows the spec change at
whatwg/html#11426.

Change-Id: I9b0776b86059f6c8734d57c17f50ed26e89215da
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/6758353
Reviewed-by: Hiroki Nakagawa <[email protected]>
Commit-Queue: Domenic Denicola <[email protected]>
Cr-Commit-Position: refs/heads/main@{#1488632}
@jonathan-j-lee jonathan-j-lee force-pushed the chromium-export-a354fb49e4 branch from 250d2b5 to 0d8d00f Compare July 19, 2025 03:27
@jonathan-j-lee
Copy link
Contributor 

INFO:root:Scheduling sink-task
Traceback (most recent call last):
  File "/home/test/web-platform-tests/_venv3/lib/python3.10/site-packages/taskcluster/client.py", line 503, in _makeHttpRequest
    response.raise_for_status()
  File "/home/test/web-platform-tests/_venv3/lib/python3.10/site-packages/requests/models.py", line 1024, in raise_for_status
    raise HTTPError(http_error_msg, response=self)
requests.exceptions.HTTPError: 406 Client Error: Not Acceptable for url: http://taskcluster/api/queue/v1/task/NNVRw6neTF-0PsO0SnYuAw

This PR doesn't change anything infra-related, so I have no idea why the decision task for this PR and #53822 in particular are failing, and no other PRs are. I've already tried rebasing/retrying CI at different times, but the failure is consistent.

@jcscottiii Should we just try merging anyway?

@jonathan-j-lee jonathan-j-lee mentioned this pull request Jul 21, 2025
Merged
@jcscottiii
Copy link
Contributor

jcscottiii commented Jul 21, 2025 •
edited
Loading

I agree that these two PRs do not look infra related. I merged one just for now. We can watch the checks for this commit 8c89451.

If all goes well, I will merge this one too.

@jcscottiii jcscottiii mentioned this pull request Jul 21, 2025
Closed
@jonathan-j-lee jonathan-j-lee changed the title Fire error events for invalid speculation rules [speculation-rules] Fire error events for invalid speculation rules Jul 22, 2025
@jcscottiii
Copy link
Contributor

Hey @jonathan-j-lee, I think it is actually safe to merge. I created a rebased version here: #53879

And it works.

@chromium-wpt-export-bot chromium-wpt-export-bot merged commit 1d91c36 into master Jul 22, 2025
72 checks passed
@chromium-wpt-export-bot chromium-wpt-export-bot deleted the chromium-export-a354fb49e4 branch July 22, 2025 18:45
@jonathan-j-lee
Copy link
Contributor

Thanks @jcscottiii! My goal here was actually to try and isolate what about this PR was causing the decision task failure by mutating this PR's title/description until it matched your passing PR #53879.

It looks like the CI for this PR was fixed by scrubbing the PR description. There might be a connection between <script type=speculationrules> (in the description for this PR and #53822) and the scheduling failure. I'll work on relanding #53822 and continue the investigation there.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jcscottiii jcscottiii jcscottiii approved these changes

@wpt-pr-bot wpt-pr-bot wpt-pr-bot approved these changes

Assignees

No one assigned

Labels

chromium-export speculation-rules wg-cg_wicg

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@chromium-wpt-export-bot @jonathan-j-lee @jcscottiii @wpt-pr-bot @domenic