WordPress.org

Make WordPress Core

Opened 3 months ago

Last modified 6 weeks ago

#53056 new defect (bug)

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

Reported by: lalitjalandhar Owned by:
Milestone: 5.9 Priority: normal
Severity: normal Version:
Component: General 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 months ago.
postman screen shot
53056.diff (2.4 KB) - added by hermpheus 2 months ago.

Download all attachments as: .zip

Change History (8)

@lalitjalandhar
3 months ago

postman screen shot

#1 @desrosj
3 months 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.


2 months ago

  • 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
2 months ago

#3 @hermpheus
2 months 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
7 weeks 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.


6 weeks ago

#6 @chaion07
6 weeks 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!

Note: See TracTickets for help on using tickets.