Skip to content
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

WPCOM memberships endpoint returns incorrect 500 status code #39153

Closed
sathyapulse opened this issue Aug 30, 2024 · 5 comments · Fixed by #40084
Closed

WPCOM memberships endpoint returns incorrect 500 status code #39153

sathyapulse opened this issue Aug 30, 2024 · 5 comments · Fixed by #40084
Assignees
@millerf
Labels
[Feature] WPCOM API General [Plugin] Jetpack Issues about the Jetpack plugin. https://wordpress.org/plugins/jetpack/ [Pri] Normal Triaged [Type] Bug When a feature is broken and / or not performing as intended

Comments

@sathyapulse
Copy link

Impacted plugin

Jetpack

Quick summary

One of our customers was concerned about the 500 errors in the application. After further investigation, we found that the membership status endpoint returns a 500 status code when the user account is not connected to WordPress.com. It looks like the intended status code is 404, but the API returns a 500 status code due to incorrect usage of status code in the third parameter.

In the same method, the third parameter is used correctly to return the intended 404 status code. Can we please audit the status codes in the file and fix the status codes?

Steps to reproduce

The requests to the endpoint are triggered from the Block Editor.

https://example.com/wp-json/wpcom/v2/memberships/status?source=gutenberg&type=all&is_editable=true&_locale=user

A clear and concise description of what you expected to happen.

The expected 404 status code should be returned from the API.

What actually happened

No response

Impact

All

Available workarounds?

There is no user impact

If the above answer is "Yes...", outline the workaround.

No response

Platform (Simple and/or Atomic)

No response

Logs or notes

No response
@sathyapulse sathyapulse added [Type] Bug When a feature is broken and / or not performing as intended Needs triage Ticket needs to be triaged labels Aug 30, 2024
@github-actions github-actions bot added the [Plugin] Jetpack Issues about the Jetpack plugin. https://wordpress.org/plugins/jetpack/ label Aug 30, 2024
@anomiex
Copy link
Contributor

anomiex commented Aug 30, 2024

Since it seemed likely we have more instances of this than just the one file, I used Phan to scan the entire monorepo.

It turned up 456 places where we pass an int to `WP_Error`'s data parameter, all of which look likely to be intended as an HTTP status code.

But on further review, I don't think all of these would be correctly changed to array( 'status' => X ). The ones in projects/plugins/jetpack/_inc/lib/core-api/ are probably going through Core's API and so are good to change, but on the other hand the ones in projects/plugins/jetpack/json-endpoints/ seem more likely to be going through this bit of code that wants either the bare int or array( 'status_code' => X )

jetpack/projects/plugins/jetpack/class.json-api.php

Lines 754 to 762 in cd0ac7a

$status_code = $error->get_error_data();
if ( is_array( $status_code ) && isset( $status_code['status_code'] ) ) {
$status_code = $status_code['status_code'];
}
if ( ! $status_code ) {
$status_code = 400;
}

We may need to check that neither of the APIs are using the other's classes as a backend. And we may need to check what code on wpcom Simple does with any of this too.

As for everything else, does it go through one of those APIs, or both, or neither?

That's as far as I have time to take this right now. Anyone else is free to pick it up.

@jartes jartes moved this from Needs Triage to Triaged in Automattic Prioritization: The One Board ™ Sep 4, 2024
@jartes jartes added Triaged and removed Needs triage Ticket needs to be triaged labels Sep 4, 2024
@ThirdEyeRose
Copy link

This seems to now be impacting multiple customers of WP VIP. It may be a change in the most recent update of Jetpack, or at least within the last few versions. Our latest customer report was after a Jetpack automatic upgrade to version 13.9.

@millerf millerf mentioned this issue Nov 7, 2024
Merged
3 tasks
@millerf
Copy link
Contributor

millerf commented Nov 7, 2024

@ThirdEyeRose @anomiex This is now fixed (at least for the Memberships status).

It turned up 456 places where we pass an int to WP_Error's data parameter, all of which look likely to be intended as an HTTP status code.

Maybe you should open a new issue regarding this.

CC @jeherve

@suyogyashukla
Copy link
Member

Hi @millerf - we have another customer seemingly observing the same issue in 197974-z - they're seeing 500 responses being returned for this endpoint:

/wp-json/wpcom/v2/memberships/status?source=gutenberg&type=all&is_editable=true&_locale=user

Is there a specific version of Jetpack that has the fix in place? Based on the PR, I think it should be fixed in 13.9.1? Fwiw, their WP version is currently 6.6, and Jetpack version is 14.0. Please also let me know if you need any further details here!

@millerf
Copy link
Contributor

millerf commented Dec 9, 2024 •
edited
Loading

Hi @millerf - we have another customer seemingly observing the same issue in 197974-z - they're seeing 500 responses being returned for this endpoint:

@suyogyashukla

Can you please tell me their blog ID or URL? I don't have access to the ticket... It would also help making sure their jetpack connection works fine with Pc9OEs-v-p2

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

@millerf millerf

Labels
[Feature] WPCOM API General [Plugin] Jetpack Issues about the Jetpack plugin. https://wordpress.org/plugins/jetpack/ [Pri] Normal Triaged [Type] Bug When a feature is broken and / or not performing as intended
Projects
Automattic Prioritization: The One Board ™
Archived in project
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Fix/status code Automattic/jetpack
7 participants
@jeherve @millerf @anomiex @jartes @ThirdEyeRose @sathyapulse @suyogyashukla