"Uncached elements inside cached" errors after update to 2.6+

[hugopeek]

This is a belated response to the changes introduced in this PR: https://github.com/modxcms/revolution/pull/13530

I'm running into a lot of "You should not call uncached elements inside cached" errors now after updating all my sites to 2.6.5. I was holding off the 2.5 -> 2.6 update for a while now because I tried it on a few sites back then, got these errors, and decided to deal with this later.

My use case involves ContentBlocks. For most blocks, you can enter the HTML for the output in a Template field. Works fine, but I wanted to maintain this HTML in a static file instead, to improve portability across projects and make maintenance a little easier. So in my ContentBlocks template fields, I often call chunks like this:

[[$headingBasicSubtitle?
    &value=`[[+value]]`
    &level=`[[+level]]`
    &subtitle=`[[+subtitle]]`
    &alignment=`[[+alignment]]`
    &unique_idx=`[[+unique_idx]]`
]]

As of ContentBlocks 1.7, this use case is also supported natively:

Field/layout templates now support @CHUNK to create chunk tags with all available properties [#461]

But if I understand correctly, this means that after the caching update, these chunks cannot contain any uncached placeholders anymore.. This would also mean that any values entered by the editor cannot contain uncached tags. In the above example, I ran into a situation where the subtitle property contained a username. This results in the following error:

[2018-07-23 07:29:00] (ERROR @ /home/hugo/Localhost/romanesco-nursery/core/model/modx/modparser.class.php : 452) You should not call uncached elements inside cached!
Outer tag: [[$headingBasicSubtitle?
    &value=`Your website is ready!<br>`
    &level=`h1`
    &subtitle=`What shall we do now [[!+modx.user.id:userinfo=`username`]]? `
    &alignment=`center aligned`
    &unique_idx=`1`
]]
Inner tag $headingBasicSubtitle?
    &value=`Your website is ready!<br>`
    &level=`h1`
    &subtitle=`What shall we do now [[!+modx.user.id:userinfo=`username`]]? `
    &alignment=`center aligned`
    &unique_idx=`1`

Using the @CHUNK option has the same effect. And since I'm using this pattern a lot, combined with the fact that the error message is quite elaborate, my error logs are all growing like mushrooms on a fresh pile of steamy horse poo right now.

I'm all for this PR and making the caching more predictable, but to me this update feels a little too intrusive now. Yes, I understand that what I'm doing is "wrong", technically speaking, but if it doesn't slow down my sites too much (above example takes just 0.0023179s to parse), then why roast me this harshly for it in the error log? And secondly: why shouldn't I be free to (ab)use MODX in any way I prefer? If this works for me, then why not let me be?

So I second the motion of opengeek to reduce this to a warning. Or alternatively, create a system setting instead (similar to log_snippet_not_found) to suppress the errors?

[bezumkin]

If this works for me, then why not let me be?

Because this not works for you or anybody else. If you call any uncached tag inside cached - the whole parent tag will be processed as uncached. You will not cache anything, this is a mistake.

So, you just need to make your call uncacheable and get rid of this errors in your log.

[Jako]

It is maybe some sort of misunderstanding.

As far as I know, the following use case is problematic. Uncached snippet/chunk/placeholder tags inside of property values of a cached snippet/chunk call. Like [[snippet? &property=`[[!+placeholder]]`]]

A cached chunk call could contain uncached tags inside of the called chunk itself.

Or am I wrong with that?

[bezumkin]

Or am I wrong with that?

If it contains at least one uncached tag, the whole parent tag will be processed as uncached. That is what about this record in error log.

This is from 2.6.0

[hugopeek]

Or am I wrong with that?

@Jako No, you are correct. Uncached tags inside the chunk itself remain unaffected. It's the uncached tags inside a chunk call that trigger the error, like the property value in your example.

If you call any uncached tag inside cached - the whole parent tag will be processed as uncached. You will not cache anything, this is a mistake.

@bezumkin Ok, I understand that this is how it works now. My problem is: I often don't know what content is inside the property values. That's mostly user input, coming from ContentBlocks fields. In my above example, value and subtitle are user input.

And this is just a simple example. There are also scenarios with more complex content being passed as property value, or even nested ContentBlocks layouts that by themselves can contain a wide range of input fields. Some of them containing uncached chunk calls or values. This results in error messages with up to a few hundred lines of code, "highlighting" the offending chunk call:

[2018-07-24 04:01:14] (ERROR @ /home/hugo/Localhost/netmedia/core/model/modx/modparser.class.php : 452) You should not call uncached elements inside cached!
Outer tag: [[$flexibleRowInnerWrapper? &content=`<div class=" row">
    <div class="column">[[$headingBasic?
    &value=`[[%romanesco.patterns.assigned_templates]]`
    &level=`h5`
    &alignment=``
    &unique_idx=`3862`
]]

[[$referringPatternsOuter? 
    &unique_idx=`3863`
    &layout_id=`12`
    &layout_column=`col_1`
    &layout_idx=`0`
    &pattern_list=`assignedTemplates`
    &pattern_template=`patternLayoutElectronTV`
    &pattern_name=`organization_id`
    ]]
</div>
    <div class="column">[[$headingBasic?
    &value=`[[%romanesco.patterns.included_patterns]]`
    &level=`h5`
    &alignment=``
    &unique_idx=`3864`
]]

[[!cbRenderCodeField?
    &valueRendered=`<div class="ui list">
    [[$includedPatternsRow?
        &name=`tvSelectClient`
        &category=`174`
    ]]
</div>`
    &valueRaw=`<div class="ui list">
    [[$includedPatternsRow?
        &name=`tvSelectClient`
        &category=`174`
    ]]
</div>`
    &renderTag=`ignore`
]]

[[$headingBasic?
    &value=`[[%romanesco.patterns.referring_patterns]]`
    &level=`h5`
    &alignment=``
    &unique_idx=`3866`
]]

[[$referringPatternsOuter? 
    &unique_idx=`3867`
    &layout_id=`12`
    &layout_column=`col_2`
    &layout_idx=`0`
    &pattern_list=`referringChunks,referringTemplates`
    &pattern_template=`patternLayoutElectronTV`
    &pattern_name=`organization_id`
    ]]
</div>
</div>`]]
Inner tag $flexibleRowInnerWrapper? &content=`<div class=" row">
    <div class="column"><h5 class="ui  header">Assigned to the following templates</h5>

<div class="ui list">

        <a class="item" href="patterns/templates/confirmation#testimonial">
            <span class="ui circular label">T</span>
            Testimonial

        </a>

        <a class="item" href="patterns/templates/confirmation#portfolioproject">
            <span class="ui circular label">T</span>
            PortfolioProject

        </a>

</div>
</div>
    <div class="column"><h5 class="ui  header">Contains the following patterns</h5>

[[!cbRenderCodeField?
    &valueRendered=`<div class="ui list">

        <a class="item" href="patterns/atoms/select#tvselectclient">
            <span class="ui circular label">A</span>
            tvSelectClient

        </a>

</div>`
    &valueRaw=`<div class="ui list">

        <a class="item" href="patterns/atoms/select#tvselectclient">
            <span class="ui circular label">A</span>
            tvSelectClient

        </a>

</div>`
    &renderTag=`ignore`
]]

<h5 class="ui  header">Included in the following patterns</h5>

<div class="ui list">

            [[+referring_tvs]]

        <a class="item" href="patterns/molecules/overviews/review#overviewrowreviewlogo">
            <span class="ui circular label">M</span>
            overviewRowReviewLogo

        </a>

            [[+referring_pages]]

</div>
</div>
</div>`

Annoying huh, all that scrolling ;) Needless to say that on some bigger sites, the logs quickly become unusable.

You say that in order to get rid of the errors, I should simply call the parent chunks uncached. This would basically result in my entire framework being rendered uncached, which would also render it useless. All to anticipate for some edge cases, for which I'm more than happy to live with the resulting flaw of the parent element(s) not being cached. And as I said: it doesn't slow my sites down considerably and I'm aware of good caching practices and custom caching, which I also use whenever applicable and beneficial.

So again: could there be an easy way to suppress the errors, or turn them into warnings? I don't know if I'm the only one in the MODX universe who's affected, but please consider that right now I have no choice but to accept massive error logs or hack the core. Yeah, or restructure my entire framework and update all existing projects.. But all that doesn't quite rhyme with creative freedom I feel, and I also wouldn't expect it on a minor version update.

[bezumkin]

This would basically result in my entire framework being rendered uncached, which would also render it useless.

It is already processed uncached. You just have messages about it in your log.

If you don't care about what is happening on your site and what users put into values of your chunks - just make error.log unwritable.

So again: could there be an easy way to suppress the errors, or turn them into warnings?

There is no other way, but make pull request with this changes, so nobody other will know about wrong use of cached elements on their sites.

Let's make them think, that everything in cache, when it runs uncached.

[alroniks]

My vote for the warning against the error. The error means that something totally has broken and the user should have a look and fix it asap. But in this case code still working, but a bit incorrect, so warning will show it but without a huge alerting.

[hugopeek]

Some of the cached chunk calls are uncached now, depending on the input. Not all. I was referring to calling all chunks uncached to prevent issues with some of these cases, which would be a little drastic.

And my point is that I do care about what's happening on the site. But when 98% of the log is filled with this issue, it becomes a bit harder to scoop out the other errors..

Anyway, I agree with you that this is a good exercise in learning how the caching works. For me at least ;)

[nicar]

+1 what @hugopeek said.

This is highly unpractical indeed (for me as well). I see the advantage and are for sure positive towards that it is pointing out those "errors", but it becomes a pain in the ass if you dump the code for a whole site into the logs as seen above (or take for example a site running Jako's SwitchTemplate) ... mainly right now when people are urged to update because of security vulnerabilities but don't have the time to dig in those things for now.

What about truncating the Message? In several instances it isn't any more precise or giving to display the whole tag code - in most cases you'll have to search and analyse for yourself anyway.

[Jako]

Change

xPDO::LOG_LEVEL_ERROR

to

xPDO::LOG_LEVEL_WARN

In https://github.com/modxcms/revolution/blob/2.x/core/model/modx/modparser.class.php#L452

[alroniks]

Thank you @Jako I've prepared PR https://github.com/modxcms/revolution/pull/14017

[alroniks]

PR is merged, so closing as solved.

[hugopeek]

Thanks everyone, much appreciated!