thebatclaudio / wp-rest-api-v2-menus

Adding menus endpoints on WP REST API v2
47 stars 23 forks source link

Fatal error when get_term() fails #43

Closed darylldoyle closed 1 year ago

darylldoyle commented 1 year ago

When get_term() fails to return what's expected it will return a WP_Error object or null.

We're currently seeing it return null which is causing a fatal error as the code is then trying to assign a property to it.

https://github.com/thebatclaudio/wp-rest-api-v2-menus/blob/4ea466b042c698c7a9a6e261389a04015d83ddd1/wp-rest-api-v2-menus.php#L70-L71

We should probably do something like:

$menu        = get_term( $locations[ $data['id'] ] );

if ( is_wp_error( $menu ) || null === $menu ) {
    return new \WP_Error('some error message');
}

$menu->items = wp_api_v2_menus_get_menu_items( $locations[ $data['id'] ] );

☝️ this is untested

thebatclaudio commented 1 year ago

Hello @darylldoyle, can you help me to reproduce this bug?

I'm trying to reproduce this error, but I can't.

Thank you!

darylldoyle commented 1 year ago

Hey @thebatclaudio 👋

We're running PHP 8. I've dug in a little today, and the root cause seems to be that a menu was set up and therefore saved into the theme_mods option, but the associated WP_Term has been deleted somehow.

This means that get_nav_menu_locations() returns the location as active, but when we call get_term() with the ID, it doesn't exist.

This seems like something has gone awry with our data, which has caused this issue, but it might be worth fixing in case others also run into it.

I'll open a PR shortly; if you don't want to merge it, that's no issue. We can patch it locally to ensure it's not an issue for us.

thebatclaudio commented 1 year ago

Hey @darylldoyle, thank you for the PR. I will merge it ASAP

thebatclaudio commented 1 year ago

I just released a new version with your fix! Thank you @darylldoyle!