WordPress / wordpress-importer

The WordPress Importer
https://wordpress.org/plugins/wordpress-importer/
GNU General Public License v2.0
78 stars 76 forks source link

Failed wp_insert_category() when IMPORT_DEBUG is set to true produces a fatal error #122

Closed darylldoyle closed 2 years ago

darylldoyle commented 2 years ago

This issue is copied over from this Trac issue.

When calling wp_insert_category(), setting the second parameter to true lets us return a WP_Error. In this case, we don't, so 0 is returned in the case of a failure.

If a WP_Error or 0 value is returned, it will echo that it failed to import the category. If we also have IMPORT_DEBUG set to true, it will try and fetch the error message from the returned value. This is fine, if we're returning a WP_Error, but in the case that it returns 0, it causes a fatal error.

See: https://github.com/WordPress/wordpress-importer/blob/99462ec2d3390bc0de3bc5ffbe817f2160e5ef54/src/class-wp-import.php#L426-L438

Fatal error: Uncaught Error: Call to a member function get_error_message() on int

I suggest that we add an additional check, to see if we have a WP_Error, before trying to render the error message:

$id = wp_insert_category( $data );
if ( ! is_wp_error( $id ) && $id > 0 ) {
        if ( isset( $cat['term_id'] ) ) {
                $this->processed_terms[ intval( $cat['term_id'] ) ] = $id;
        }
} else {
        printf( __( 'Failed to import category %s', 'wordpress-importer' ), esc_html( $cat['category_nicename'] ) );
        if ( defined( 'IMPORT_DEBUG' ) && IMPORT_DEBUG && is_wp_error( $id ) ) {
                echo ': ' . $id->get_error_message();
        }
        echo '<br />';
        continue;
}
darylldoyle commented 2 years ago

Further to the above, in the Trac comments, we decided that a better approach than the above was to set the second argument of wp_insert_category() to true so that it returns a WP_Error object.