Dashboard crashes for sites marked as private on WP.com

nickdaugherty commented 4 years ago

Steps to reproduce the issue

Set site to private on WP.com (blog_public option to -1)
Load the JP dashboard at /wp-admin/admin.php?page=jetpack#/dashboard
Dashboard crashes b/c of this JS error:

TypeError: "bc(...).filter is not a function"
    yc admin.js:26550
    Ug admin.js:48881
    r admin.js:22627
    f admin.js:22717
    J admin.js:22720
    selectDerivedProps admin.js:22337
    renderWrappedComponent admin.js:22365
    indirectRenderWrappedComponent admin.js:22356
    ph react-dom.min.js:221
    lh react-dom.min.js:126
    O react-dom.min.js:121
    ze react-dom.min.js:118
    mg react-dom.min.js:53
    unstable_runWithPriority react.min.js:26
    Ma react-dom.min.js:52
    mg react-dom.min.js:52
    V react-dom.min.js:52
    Sb react-dom.min.js:213
    enqueueSetState react-dom.min.js:203
    setState react.min.js:21
    unsubscribe admin.js:22207
    d admin.js:22466
    Tt admin.js:24001
    dispatch admin.js:26849
    fetchSitePurchases admin.js:30965
react-dom.min.js:103:487

That ultimately comes from fetchSitePurchases which hits the wp-json/jetpack/v4/site/purchases endpoint, which has a bug where it returns an incorrect response for failed upstream requests (see #16067).

Regardless of #16067, the JS code here should be defensive and only call .filter() if the data is an array.

What I expected

To see the JP dashboard

What happened instead

The dashboard went kablamo.

kraftbj commented 4 years ago

~Marking this for 8.7 as we need to have a fix in the next release. This is causing issues for some VIP Go clients.~

We have a WP.com workaround for Go sites. The conditions needed (-1 blog_public option) is not yet possible for sites other than Atomic or Go, so this is not urgent.

Still need to fix it to support third-party private sites with a Jetpack connection.

kraftbj commented 4 years ago

If 16184 is merged, to test this, would need to mock a 200 status code response that does not return an array or, probably easier, revert 16184 for the sake of testing with the instructions above.

stale[bot] commented 3 years ago

This issue has been marked as stale. This happened because:

It has been inactive in the past 6 months.
It hasn’t been labeled `[Pri] Blocker`, `[Pri] High`.

No further action is needed. But it's worth checking if this ticket has clear reproduction steps and it is still reproducible. Feel free to close this issue if you think it's not valid anymore — if you do, please add a brief explanation.

anomiex commented 1 year ago

It appears that reproducing the error here can currently be done by changing https://github.com/Automattic/jetpack/blob/b927b0492c265811198f6d6f2a923763317a471c/projects/plugins/jetpack/_inc/lib/core-api/class.jetpack-core-api-site-endpoints.php#L58 to something like

'data'  => '"XXXX"',

The .filter() call in question appears to be https://github.com/Automattic/jetpack/blob/b927b0492c265811198f6d6f2a923763317a471c/projects/plugins/jetpack/_inc/client/state/site/reducer.js#L361-L363

OTOH, I'm not sure this is all that worth worrying about. The JS should be able to assume that the API endpoint returns a value structured according to its specification when it returns a successful response. Does that specification allow for not having whatever array it's trying to filter?

Automattic / jetpack