Enqueued Assets audit: treat zero-size and no-store responses as errors#2481
Enqueued Assets audit: treat zero-size and no-store responses as errors#2481Gilmoursa wants to merge 3 commits into
Conversation
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
In perflab_aea_get_asset_size(), two cases that should have been
treated as errors were silently passing the audit:
- A 200 response with an empty body now returns WP_Error('zero_size').
An asset that returns nothing is effectively broken, regardless of the
status code.
- A 200 response with Cache-Control: no-store now returns
WP_Error('not_cacheable'). An asset that browsers are forbidden from
caching is a clear performance problem and should surface in the audit.
Cache-Control: no-cache (revalidate) is intentionally left as passing
since the asset can still be served from cache after revalidation.
Both cases were already noted as TODOs in the original code. Removes
those TODO comments and adds tests covering zero-size, no-store,
no-cache (should pass), and normal cacheable responses. Also updates
the mock class to pass response headers through so header-dependent
tests can work end-to-end.
Fixes WordPress#2417.
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
7168784 to
4bcfe31
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## trunk #2481 +/- ##
==========================================
+ Coverage 69.33% 69.39% +0.05%
==========================================
Files 90 90
Lines 7749 7763 +14
==========================================
+ Hits 5373 5387 +14
Misses 2376 2376
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
wp_remote_retrieve_header() returns string|array (array when the same header appears multiple times in the response). Joining array values with implode() before passing to strtolower() resolves the PHPStan error and also correctly handles multi-value Cache-Control headers. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
This PR updates the Enqueued Assets Site Health audit so empty asset responses and Cache-Control: no-store responses are surfaced as asset errors instead of passing as successful zero-byte/cacheable assets.
Changes:
- Returns
WP_Errorfor zero-size asset bodies andCache-Control: no-store. - Adds tests for zero-size and cache-control handling.
- Updates the audit asset test mock to pass response headers through.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
plugins/performance-lab/includes/site-health/audit-enqueued-assets/helper.php |
Adds empty-body and no-store error handling in asset size retrieval. |
plugins/performance-lab/tests/includes/site-health/audit-enqueued-assets/test-audit-enqueued-assets-helper.php |
Updates and expands coverage for the new asset-size error behavior. |
plugins/performance-lab/tests/data/class-audit-assets-mock-assets.php |
Includes mocked headers in HTTP responses for header-dependent tests. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…ze check Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Summary
Fixes #2417.
perflab_aea_get_asset_size()had two TODOs in the code for cases that silently passed the audit when they should be flagged as errors:Cache-Control: no-store— an asset that browsers are forbidden from caching is a performance problem and should surface in the audit. The audit was reporting these as OK as long as the status was 200.Behaviour change
Zero-size: Returns
WP_Error('zero_size')instead of0.Non-cacheable: Returns
WP_Error('not_cacheable')when the response has aCache-Control: no-storedirective.Cache-Control: no-cache(revalidate) is intentionally left as passing — the asset can still be served from cache after revalidation, so it is not a hard performance problem in the same way asno-store.Both
WP_Errorresults feed into the existing error display path in the audit table (red row, error message shown,recommendedstatus), so no additional UI changes are needed.Changes
plugins/performance-lab/includes/site-health/audit-enqueued-assets/helper.php: implement the two checks, remove the TODO commentsplugins/performance-lab/tests/includes/site-health/audit-enqueued-assets/test-audit-enqueued-assets-helper.php: update the zero-size assertion to expectWP_Error; addtest_perflab_aea_get_asset_size_not_cacheable()coveringno-store,no-storewith extra directives,no-cache(should pass), and a normal cacheable responseplugins/performance-lab/tests/data/class-audit-assets-mock-assets.php: passheadersfrom mocked responses through thepre_http_requestfilter so header-dependent tests work end-to-endTest plan
composer test:integration -- --filter test_perflab_aea_get_asset_sizecomposer test:integration -- --filter test_perflab_aea_get_asset_size_not_cacheableCache-Control: no-storeshows as an error row in the tableCache-Control: no-cachestill passes the audit