WordPress.org

Make WordPress Core

Opened 3 months ago

Last modified 12 days ago

#50145 new defect (bug)

Site Health - Defensively handle empty/improperly formed test results

Reported by: dogwithblog Owned by:
Milestone: 5.6 Priority: normal
Severity: normal Version:
Component: Site Health Keywords: has-screenshots has-patch needs-refresh
Focuses: Cc:

Description

Hi team,

I am getting the message, "results are still loading" on Site Health even after being on wait for over half an hour. This is for my blog: https://dogwithblog.in

Attachments (2)

dwbw.jpg (40.1 KB) - added by dogwithblog 3 months ago.
Site health error
50145.diff (3.0 KB) - added by SergeyBiryukov 5 weeks ago.

Download all attachments as: .zip

Change History (14)

@dogwithblog
3 months ago

Site health error

#1 @davidbaumwald
3 months ago

  • Component changed from General to Site Health
  • Keywords has-screenshots added

#2 @kraftbj
3 months ago

  • Milestone changed from Awaiting Review to 5.5
  • Summary changed from Site Health - Results are still loading to Site Health - Defensively handle empty/improperly formed test results

Hi @dogwithblog,

Welcome to Trac. Generally, checking in for support at https://wordpress.org/support/ is a good step before filing a bug report since the issue may not be an issue with WordPress itself.

Checking out your site, I believe you have Jetpack installed. The recent version has a bug that is preventing the site health area from reporting a "grade".

Resolving this on the Jetpack side is being tracked at https://github.com/Automattic/jetpack/issues/15709

On the WordPress side, @clorith will be working on a patch to better handle a situation where a plugin returns an unexpected value.

This ticket was mentioned in Slack in #forums by kraft. View the logs.


3 months ago

This ticket was mentioned in Slack in #core-site-health by afragen. View the logs.


6 weeks ago

This ticket was mentioned in PR #349 on WordPress/wordpress-develop by Clorith.


6 weeks ago

  • Keywords has-patch added

Trac ticket: https://core.trac.wordpress.org/ticket/50145

This PR adds a validator to the issue respones for Site Health tests, which will discard any invalid responses instead of causing fatal errors.

It also makes badges optional, on the same basis as actions are optional, it's expected, but there may be situations where they're not present.

This ticket was mentioned in PR #349 on WordPress/wordpress-develop by Clorith.


6 weeks ago

Trac ticket: https://core.trac.wordpress.org/ticket/50145

This PR adds a validator to the issue respones for Site Health tests, which will discard any invalid responses instead of causing fatal errors.

It also makes badges optional, on the same basis as actions are optional, it's expected, but there may be situations where they're not present.

#7 @Clorith
5 weeks ago

  • Keywords commit added

I've been doing some testing over the last few days, and this seems to be handling things well.

It also makes the badge optional. This is intentional. I like the badges as they give some context, especially the way large plugins have used them to flag where tests come from, but I can see scenarios where a badge may not make sense as well, so just future-proofing here.

#8 @SergeyBiryukov
5 weeks ago

  • Keywords needs-refresh added; commit removed

50145.diff is a refresh of https://github.com/WordPress/wordpress-develop/pull/349.diff.

It seems to work well in my testing, however it also introduces some JSHint issues that would result in test failures:

Running "jshint:core" (jshint) task
src/js/_enqueues/admin/site-health.js
108 |    Object.entries( minimumExpected ).forEach( ( [ key, value ] ) => {
                                                    ^ 'destructuring binding' is available in ES6 (use 'esversion: 6') or Mozilla JS extensions (use moz).
108 |    Object.entries( minimumExpected ).forEach( ( [ key, value ] ) => {
                                                                     ^ 'arrow function syntax (=>)' is only available in ES6 (use 'esversion: 6').
110 |            Object.entries( value ).forEach( ( [ subKey, subValue ] ) => {
                                                  ^ 'destructuring binding' is available in ES6 (use 'esversion: 6') or Mozilla JS extensions (use moz).
110 |            Object.entries( value ).forEach( ( [ subKey, subValue ] ) => {
                                                                         ^ 'arrow function syntax (=>)' is only available in ES6 (use 'esversion: 6').

Core currently uses the "esversion": 3 (ECMAScript 3) setting in .jshintrc.

Would it be possible to rewrite the patch for compatibility with the current core standard?

Last edited 5 weeks ago by SergeyBiryukov (previous) (diff)

@SergeyBiryukov
5 weeks ago

This ticket was mentioned in Slack in #core by david.baumwald. View the logs.


3 weeks ago

This ticket was mentioned in Slack in #core by whyisjake. View the logs.


12 days ago

#11 @whyisjake
12 days ago

Also seeing this:

 {
  filename: 'site-health.js',
  line: 108,
  col: 64,
  pos: 3278
}
>> Uglifying source build/wp-admin/js/site-health.js failed.
Warning: Uglification failed.
Invalid assignment. 
Line 108 in build/wp-admin/js/site-health.js

Warning: Cannot read property 'min' of undefined

#12 @SergeyBiryukov
12 days ago

  • Milestone changed from 5.5 to 5.6
Note: See TracTickets for help on using tickets.