Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

get_the_excerpt is taking PHP out of memory in a block rendering #5572

Closed
2 of 3 tasks
maximebj opened this issue Mar 12, 2018 · 19 comments · Fixed by #8984
Closed
2 of 3 tasks

get_the_excerpt is taking PHP out of memory in a block rendering #5572

maximebj opened this issue Mar 12, 2018 · 19 comments · Fixed by #8984
Labels
[Type] Bug An existing feature does not function as intended

Comments

@maximebj
Copy link
Contributor

maximebj commented Mar 12, 2018

Issue Overview

I'm rendering a block in PHP that display another post preview card so I simply use (in the render_callabck) :

$id = $attributes['postID'];

$post = get_post( $id );
$excerpt = get_the_excerpt( $id );

I get a Fatal error: Allowed memory size of... from PHP from get_the_excerpt()
It look like it's too much for WP to parse the gutenberg saved markup

The targeted post only has a few paragraphs and a read more tag.

The issue does not happen if I set manually an excerpt in the inspector.

capture 2018-03-12 a 17 45 42

This memory issue does not occur outside a block rendered in PHP

Steps to Reproduce

  1. create a post with gutenberg
  2. add a read more block but no excerpt on inspector
  3. create a block rendering in PHP
  4. on the render hook, try to get_post and get_the_excerpt

Expected Behavior

display everything

Current Behavior

Allowed memory size of

Todos

  • Tests
  • Is it a legit issue?
  • find where occurs the memory leak and why
@Kluny
Copy link
Contributor

Kluny commented Mar 12, 2018

Hey Maxime, could you clarify what you're doing on this step?

create a block rendering in PHP

I'd like to try to test this.

@maximebj
Copy link
Contributor Author

Yes ! I'm creating some blocks (and making a plugin of it) and one of my block is about featuring another post like this :

capture 2018-03-13 a 09 18 01

So my block in html is just

<!-- wp:gutenblocks/post {"postID":158} /-->

And the whole thing is rendered via PHP.

When I'm in the render_callback, if I try to call get_excerpt() I got this error

@Theremingenieur
Copy link

And when might we expect a solution? Seems like I'm not the only one who sees this as an urgent and blocking problem...

@jomurgel
Copy link
Contributor

jomurgel commented Apr 25, 2018

@Theremingenieur are you working inside a new WP_Query loop or a going through wp_get_recent_posts results in a foreach loop?

I'm seeing the same behavior, but am also seeing some interesting differences depending on how I attempt to display results. Attempting to debug.

@Theremingenieur
Copy link

@jomurgel : I have no idea. I’m just eager to finally use the post block which is part of the plugin developed by @maximebj who opened this issue. He says he can’t fix his plugin until the underlying issue is solved...

@maximebj
Copy link
Contributor Author

The issue is not there if I make a standard WP_Query and use the_excerpt

The issue only occurs when I use get_post and get_the_excerpt( $id) with an infinite loop.

I don't understand why there is a difference, but if it can help.

Thank you!

@TimothyBJacobs
Copy link
Member

I think this happens when the excerpt is empty for a post because:

get_the_excerpt with a post ID ends up calling wp_trim_excerpt. If no excerpt, it tries to build one manually from the post content. This post content is accessed via get_the_content which uses the global $post. This doesn't take into account that get_the_excerpt can seemingly be used outside of the loop. This is kinda documented in the developer docs, https://developer.wordpress.org/reference/functions/get_the_excerpt/#user-contributed-notes.

It seems core ends up avoiding the fatal error because it calls strip_shortcodes on the content before passing it into the_content filter.

Stack trace ``` # | Time | Memory | Function | Location -- | -- | -- | -- | -- 1 | 0.2005 | 398640 | {main}( ) | .../index.php:0 2 | 0.2010 | 398928 | require( '/app/public/wp-blog-header.php' ) | .../index.php:17 3 | 0.2864 | 2905128 | require_once( '/app/public/wp-includes/template-loader.php' ) | .../wp-blog-header.php:19 4 | 0.2890 | 2947912 | include( '/app/public/wp-content/themes/twentysixteen/single.php' ) | .../template-loader.php:74 5 | 0.3255 | 3075008 | get_template_part( ) | .../single.php:19 6 | 0.3255 | 3075584 | locate_template( ) | .../general-template.php:155 7 | 0.3259 | 3075696 | load_template( ) | .../template.php:647 8 | 0.3262 | 3076056 | require( '/app/public/wp-content/themes/twentysixteen/template-parts/content-single.php' ) | .../template.php:690 9 | 0.3268 | 3076056 | the_content( ) | .../content-single.php:22 10 | 0.3268 | 3076192 | apply_filters( ) | .../post-template.php:240 11 | 0.3268 | 3076592 | WP_Hook->apply_filters( ) | .../plugin.php:203 12 | 0.3269 | 3078496 | do_blocks( ) | .../class-wp-hook.php:286 13 | 0.3270 | 3082240 | WP_Block_Type->render( ) | .../blocks.php:181 14 | 0.3270 | 3082240 | {closure:/app/public/wp-content/plugins/demo-talk/php/latest-author-post.php:5-45}( ) | .../class-wp-block-type.php:107 15 | 0.3294 | 3103048 | get_the_excerpt( ) | .../latest-author-post.php:35 16 | 0.3294 | 3103496 | apply_filters( ) | .../post-template.php:397 17 | 0.3294 | 3103896 | WP_Hook->apply_filters( ) | .../plugin.php:203 18 | 0.3294 | 3105400 | wp_trim_excerpt( ) | .../class-wp-hook.php:288 19 | 0.3294 | 3105512 | apply_filters( ) | .../formatting.php:3313 20 | 0.3294 | 3105912 | WP_Hook->apply_filters( ) | .../plugin.php:203 21 | 0.3294 | 3106664 | do_blocks( ) | .../class-wp-hook.php:286 22 | 0.3295 | 3110408 | WP_Block_Type->render( ) | .../blocks.php:181 23 | 0.3296 | 3110408 | {closure:/app/public/wp-content/plugins/demo-talk/php/latest-author-post.php:5-45}( ) | .../class-wp-block-type.php:107 24 | 0.3306 | 3129024 | get_the_excerpt( ) | .../latest-author-post.php:35 25 | 0.3307 | 3129472 | apply_filters( ) | .../post-template.php:397 26 | 0.3307 | 3129872 | WP_Hook->apply_filters( ) | .../plugin.php:203 27 | 0.3307 | 3130624 | wp_trim_excerpt( ) | .../class-wp-hook.php:288 28 | 0.3307 | 3130736 | apply_filters( ) | .../formatting.php:3313 29 | 0.3307 | 3131136 | WP_Hook->apply_filters( ) | .../plugin.php:203 30 | 0.3307 | 3131888 | do_blocks( ) | .../class-wp-hook.php:286 31 | 0.3307 | 3135632 | WP_Block_Type->render( ) | .../blocks.php:181 ```

@maximebj
Copy link
Contributor Author

Thank you for theses valuable informations

So as you say if I do a real WP Query there is no issue. And if a post has a manual excerpt there is no issue too.

I may close this issue as it is not really Gutenberg related, what do you think?

@TimothyBJacobs
Copy link
Member

Yeah, seems more like an issue with Core and the strip_shortcodes solution clearly won't work for Gutenberg.

@Luehrsen
Copy link
Contributor

Yeah, a 'normal' WP_Query doesn't help either. This is our current test setup:

        $args = array(
		'posts_per_page'   => $attributes['postsToShow'],
		'orderby'          => $attributes['orderBy'],
		'order'            => $attributes['order'],
		'post_type'        => 'post',
		'post_status'      => 'publish',
		'ignore_sticky'    => true,
	);

	// The Query
	$query = new WP_Query( $args );

	$block_content = '';

	// The Loop
	if ( $query->have_posts() ) {
		while ( $query->have_posts() ) {
			$query->the_post();
			// do something
			$block_content .= get_the_excerpt();
		}
	}
	// Restore original Post Data
	wp_reset_postdata();

	return $block_content;

It runs into memory issues as expected.

@chrisvanpatten
Copy link
Contributor

See also #7268.

@vguerrerobosch
Copy link

I came up with the same issue when creating a dynamic block and using get_the_excerpt in render block callback function. I solved it with

if ( doing_filter( 'get_the_excerpt' ) ) {
    return false;
}

which solves the infinite nesting and accomplishes my desired behavior.

@mtias mtias mentioned this issue Aug 1, 2018
16 tasks
@obenland
Copy link
Member

wp_trim_excerpt applies the the_content filter, which do_blocks() is hooked into, causing an infinite loop and the memory error.

We could probably just remove the filter at the beginning of do_blocks() to make sure it only runs once.

@TimothyBJacobs
Copy link
Member

I thought that wouldn't work because if you are trying to generate an excerpt from a post with blocks, without doing the blocks, you'll end up with block comments instead of the actual content.

@obenland
Copy link
Member

To my understanding, that would only apply to dynamic blocks. Shortcodes are currently stripped too, so there should be any inconsistency there. Regular block should display fine, based on their stored markup.

@TimothyBJacobs
Copy link
Member

Wouldn't you still end up with the comments at top? I guess, depending on how the person is using get_the_excerpt, they would be hidden from display because they are HTML comments. But I thought it was important that block comments were stripped completely, not just hidden from view.

@obenland
Copy link
Member

Not in my testing so far. wp_strip_all_tags() should take care of that

@TimothyBJacobs
Copy link
Member

Ah, awesome!

@obenland
Copy link
Member

A better fix could be to just strip dynamic blocks when trying to generate an excerpt

obenland added a commit that referenced this issue Aug 14, 2018
obenland added a commit that referenced this issue Aug 16, 2018
Initially used when generating excerpts to avoid infinite loops when dynamic blocks generate excerpts, and to add parity with `strip_shortcodes()`.

Fixes #5572, #7268.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Bug An existing feature does not function as intended
Projects
None yet
Development

Successfully merging a pull request may close this issue.