Make WordPress Core

Opened 3 years ago

Closed 2 years ago

Last modified 2 years ago

#53056 closed defect (bug) (fixed)

REST API json_encode error returns 500 data->status in body(Correct) but 200 in actual status(Incorrect).

Reported by: lalitjalandhar's profile lalitjalandhar Owned by: timothyblynjacobs's profile TimothyBlynJacobs
Milestone: 5.9 Priority: normal
Severity: normal Version: 4.4
Component: REST API Keywords: good-first-bug has-patch has-unit-tests dev-feedback
Focuses: rest-api Cc:

Description

<?php
add_action( 'rest_api_init', function() {
        register_rest_route(
                'v3',
                '/my-custom-route',
                array(
                        array(
                                'methods'             => \WP_REST_Server::READABLE,
                                'callback'            => function() {
                                        return new \WP_REST_Response(INF);
                                },
                                'permission_callback' => '__return_true',
                                'args'                => array(),
                        ),
                )
        );
});

Attachments (2)

nan.PNG (19.1 KB) - added by lalitjalandhar 3 years ago.
postman screen shot
53056.diff (2.4 KB) - added by hermpheus 3 years ago.

Download all attachments as: .zip

Change History (11)

@lalitjalandhar
3 years ago

postman screen shot

#1 @desrosj
3 years ago

  • Focuses rest-api added
  • Keywords needs-patch good-first-bug needs-unit-tests added
  • Milestone changed from Awaiting Review to 5.8

Thanks for this one, @lalitjalandhar. And welcome to Trac!

It seems that there is a missing set_status() call here. Would you be interested in creating a patch?

It would also be nice to add a few tests to confirm the desired behavior after these changes.

This ticket was mentioned in PR #1264 on WordPress/wordpress-develop by hmfs.


3 years ago
#2

  • Keywords has-patch has-unit-tests added; needs-patch needs-unit-tests removed

Fixes issue in the REST API where a JSON encode failure of the response body does not result in a 500 error.

An automated test is included for this case, requiring an expansion of the test spy.

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

@hermpheus
3 years ago

#3 @hermpheus
3 years ago

Thank you @lalitjalandhar for the excellent example, and thank you @desrosj for the excellent suggestion.

I have a patch available that should address this issue. It has an automated test and also works for me in the browser and Postman. Please let me know if any changes should be made. Thanks!

#4 @hellofromTonya
3 years ago

  • Milestone changed from 5.8 to 5.9

Today is 5.8 Beta 1. Ran out of time for this one to land. Punting to 5.9 to give it time for code and test review and testing.

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


3 years ago

#6 @chaion07
3 years ago

  • Keywords dev-feedback added

Thanks @lalitjalandhar for reporting the issue. We reviewed this ticket during a [recent triage session at Core]https://wordpress.slack.com/archives/C02RQBWTW/p1623733904418100. As @hellofromTonya mentioned in here last comment that we need test review and testing as well as coding. We think that having an owner for this would also help determine the future of the ticket better. Also adding the dev-feedback keyword. Thanks!

#7 @TimothyBlynJacobs
2 years ago

  • Component changed from General to REST API
  • Version set to 4.4

#8 @TimothyBlynJacobs
2 years ago

  • Owner set to TimothyBlynJacobs
  • Resolution set to fixed
  • Status changed from new to closed

In 51960:

REST API: Send a 500 status code when JSON encoding fails.

Previously, a 200 status code would be sent despite the 500 status code present in the response body.

Props hermpheus, lalitjalandhar.
Fixes #53056.

TimothyBJacobs commented on PR #1264:


2 years ago
#9

Thanks for the patch @hermpheus! Merged in https://core.trac.wordpress.org/changeset/51960.

Note: See TracTickets for help on using tickets.