Make WordPress Core

Opened 3 years ago

Closed 2 years ago

#54807 closed defect (bug) (maybelater)

URL Details request returns a 404 status for inaccessible pages

Reported by: talldanwp's profile talldanwp Owned by:
Milestone: Priority: normal
Severity: normal Version: 5.9
Component: REST API Keywords: needs-patch
Focuses: rest-api Cc:

Description

When the new URL details endpoint is called with a URL that doesn't exist or is inaccessible, it returns a 404 status.

I'm not sure that this is correct. The 404 indicates that the URL Details endpoint itself isn't present. But that's not true, the URL Details endpoint did receive the request and responded, but instead the URL it was trying to access was not found.

My feeling is this information about the missing or inaccessible page that the user requested details on should instead be returned via the url details response payload along with a success code. Another good reason for this is that browsers log 404 responses to the console as an error. To anyone looking, it seems as though WordPress has errors, but in fact the website is behaving correctly.

I might be incorrect in my interpretation, so look forward to the discussion on this. Another consideration is that maybe it's too late to address this now, as it would likely be a breaking change.

Steps to reproduce:

  1. Open the Gutenberg editor and open dev tools to the network tab
  2. Add the navigation block and choose Start Empty
  3. Click the + button in the navigation block
  4. When the URL Popover shows, type in the name of a page that you want to create
  5. Click the 'Create draft page' option that shows up in the search results
  6. In the network tab, observe the 404. This is because the draft that URL Details attempted to get details for was publicly inaccessible. However the 404 seems to indicate that URL Details itself can't be found.

Change History (7)

#1 @get_dave
3 years ago

Yes this does seem a little unusual. When I implemented this I was thinking that we needed some way to communicate to the client that the remote URL was inaccessible.

Perhaps instead we return a "success" code such as 204 to indicate the lack of content.

Be aware that any change here will need to be accompanied by changes to the handler code in Gutenberg Core because we'll need to add a special case for the 204 and reflect that in the UI as required.

#2 @hellofromTonya
3 years ago

  • Milestone changed from Awaiting Review to 5.9.1

Thanks @talldanwp for raising this discussion. Moving to 5.9.1 for continued discussion and consensus on how to communicate the remote URL is inaccessible or non-existent.

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


2 years ago

#4 @audrasjb
2 years ago

  • Keywords needs-patch added

This issue was mentioned during today's bug scrub and there's a consensus about this not being a 404. @peterwilsoncc will try to work on a patch to try to fix this for 5.9.1.

#5 @peterwilsoncc
2 years ago

I compared this with the oembed proxy endpoint in WP_oEmbed_Controller introduced in WordPress 4.8.0. It too returns a 404 when attempting to access a resource that doesn't exist.

Testing steps:

  1. Open your browsers developer tools to the network tab
  2. Edit a new page or post
  3. Attempt to embed the non-existent URL https://twitter.com/wordpress/status/40454807
  4. In developer tools, observe the request to http://wp-dev.local/wp-json/oembed/1.0/proxy?url=https%3A%2F%2Ftwitter.com%2Fwordpress%2Fstatus%2F40454807&_locale=user returns a 404 File not found response.

Testing with https://httpstat.us/500 (which returns a 500 Internal Server Error) also returns a 404 response on both the oembed proxy and URL details endpoint.

I think @talldanwp is correct that a 200 OK is a more accurate response because the REST API URLs are working as intended but I would prefer the two endpoints provide a consistent behavior.

#6 @talldanwp
2 years ago

I think @talldanwp is correct that a 200 OK is a more accurate response because the REST API URLs are working as intended but I would prefer the two endpoints provide a consistent behavior.

That seems fair enough. It may also be difficult to update in terms of back compat. Should we close this?

#7 @peterwilsoncc
2 years ago

  • Milestone 5.9.1 deleted
  • Resolution set to maybelater
  • Status changed from new to closed

Thanks for your understanding, Dan.

I considered this overnight and am going to close it as maybelater, with any change to be consistent for the two proxying endpoints: URL details and oembeds.

Note: See TracTickets for help on using tickets.