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

[enshrined]

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;
}

[enshrined]

Adding patch

[enshrined]

Added a patch that does a is_wp_error() check before trying to access the get_error_message() method on it.

[mukesh27]

Hi there! thanks for the ticket and patch.

Can you please add is_wp_error for below all occurrences so we don't get a similar error in any other code base?

​https://github.com/WordPress/wordpress-importer/blob/99462ec2d3390bc0de3bc5ffbe817f2160e5ef54/src/class-wp-import.php#L374-L376

​https://github.com/WordPress/wordpress-importer/blob/99462ec2d3390bc0de3bc5ffbe817f2160e5ef54/src/class-wp-import.php#L484-L486

​https://github.com/WordPress/wordpress-importer/blob/99462ec2d3390bc0de3bc5ffbe817f2160e5ef54/src/class-wp-import.php#L545-L547

​https://github.com/WordPress/wordpress-importer/blob/99462ec2d3390bc0de3bc5ffbe817f2160e5ef54/src/class-wp-import.php#L757-L759

​https://github.com/WordPress/wordpress-importer/blob/99462ec2d3390bc0de3bc5ffbe817f2160e5ef54/src/class-wp-import.php#L793-L795

[SergeyBiryukov]

Replying to mukesh27:

Hi there! thanks for the ticket and patch.

Can you please add is_wp_error for below all occurrences so we don't get a similar error in any other code base?

Thanks for the review! Unless I'm missing something, all of those instances already have an is_wp_error() check just a few lines above, so adding another check would be redundant.

The instance in the patch is different because it gets executed not only if $id is a WP_Error object, but also if it's 0:

$id = wp_insert_category( $data );
if ( ! is_wp_error( $id ) && $id > 0 ) {
	...
} else {
	printf( __( 'Failed to import category %s', 'wordpress-importer' ), esc_html( $cat['category_nicename'] ) );
	if ( defined( 'IMPORT_DEBUG' ) && IMPORT_DEBUG ) {
		echo ': ' . $id->get_error_message(); // The instance being patched here.
	}
	echo '<br />';
}

Instead of adding another is_wp_error() check, I think the correct fix here would be to pass true as the second parameter to wp_insert_category(), otherwise it can never return a WP_Error in the first place.

That would also be consistent with wp_insert_post( $postdata, true ) in ​line 746.

Moving the ticket to the WordPress.org milestone instead of 6.0, as this code is in the WordPress importer plugin and not in WordPress core.

[azouamauriac]

I question myself about the use of '0' returned by wp_insert_category, someone can help me?

Also I think it would be better to add 'true' as second arg to wp_insert_category( $data ) that will become wp_insert_category( $data, true );

[enshrined]

@SergeyBiryukov is correct; as far as I can see, this error won't occur with any of the other instances as they return WP_Error objects.

I've attached a new patch that means the wp_insert_category() call returns a WP_Error object. I agree; this approach makes more sense as it's more consistent with the rest of the script.

[ocean90]

@enshrined Could you please open an issue and pull request on ​https://github.com/WordPress/wordpress-importer for this change? This Trac isn’t really the best place for the WordPress Importer anymore. Thank you!

[enshrined]

Thanks, @ocean90!

PR and Issue opened on Github!

• ​https://github.com/WordPress/wordpress-importer/issues/122
• ​https://github.com/WordPress/wordpress-importer/pull/123

[ocean90]

Thank you! Closing as reported-upstream.