humanmade / altis-cloud

Cloud Module for Altis
https://docs.altis-dxp.com/cloud/
9 stars 2 forks source link

Batcache notice: "Trying to access array offset on value of type bool" #484

Closed roborourke closed 3 years ago

roborourke commented 3 years ago

It's not entirely clear why this happens but Batcache should be able to cope with this notice.

It might be related to it failing to set / calculate cache keys properly - the affected site has Arabic language URLs so we should confirm any fix works unicode URL paths.

E_NOTICE Trying to access array offset on value of type bool
/usr/src/app/vendor/humanmade/batcache/advanced-cache.php:577
robindevitt commented 3 years ago

Hey @roborourke

The error shown

Trying to access array offset on value of type bool
vendor/humanmade/batcache/advanced-cache.php:577

The if statement from line 577

if ( $batcache->requests >= $batcache->times &&
    time() >= $batcache->cache['time'] + $batcache->cache['max_age']
) {
    wp_cache_delete( $batcache->req_key, $batcache->group );
    $batcache->do = true;
} else {
    $batcache->do = false;
}

I have done a var_dump on $batcache->cache:

array(9) { ["output"]=> string(2114132) "

Noting from line 577 it's looking for $batcache->cache['time'] & $batcache->cache['max_age']

Perhaps we need to include the following on the initial if statement:

if ( $batcache->requests >= $batcache->times &&
    isset($batcache->cache['time']) && 
    isset($batcache->cache['max_age']) && 
    time() >= $batcache->cache['time'] + $batcache->cache['max_age']
) {
    wp_cache_delete( $batcache->req_key, $batcache->group );
    $batcache->do = true;
} else {
    $batcache->do = false;
}

I notice that this is currently listed in humanmade/altis-cloud, should the issue not be in humanmade/batcache ?

roborourke commented 3 years ago

It's fine to track it here though the PR will be to the batcache repo. We might still need to update the package version here after that's done.

roborourke commented 3 years ago

Did you find a way to reproduce the error?

I think regarding the if statement the problem is that $batcache->cache can be a boolean, ie true or false, so we could just check if it's an array before checking the array keys.

robindevitt commented 3 years ago

I am battling to bypass this if statement on my local where is is setting the X-Batcache: BYPASS

// Never batcache when cookies indicate a cache-exempt visitor.
if ( is_array( $_COOKIE) && ! empty( $_COOKIE ) ) {
    var_dump( $_COOKIE );
    foreach ( array_keys( $_COOKIE ) as $batcache->cookie ) {
        if ( ! in_array( $batcache->cookie, $batcache->noskip_cookies ) && ( substr( $batcache->cookie, 0, 2 ) == 'wp' || substr( $batcache->cookie, 0, 9 ) == 'wordpress' || substr( $batcache->cookie, 0, 14 ) == 'comment_author' ) ) {
            batcache_stats( 'batcache', 'cookie_skip' );
            if ( $batcache->cache_control ) {
                header( 'Cache-Control: no-cache, must-revalidate, max-age=0' );
            }
            if ( $batcache->add_hit_status_header ) {
                header( 'X-Batcache: BYPASS' );
                header( 'X-Batcache-Reason: Cookies' );
            }

            return;
        }
    }
}

I tried being logged out as well as being logged in and using different browsers with no luck.

So I commented out the above if statement temporarily.

I am then able to reproduce the error. Sometime I do need to refresh multiple times to get it to show.

roborourke commented 3 years ago

You'd probably just have to clear cookies I think but no worries

Can we confirm it's because the URL contains unicode characters that this happens or if there are cases when the $batcache->cache property is expected to be false. We need to handle that case but it seems we might also need to fix the reason the property can be false in the first place.

robindevitt commented 3 years ago

I am seeing the error both with an Arabic and an English word in the URL so I don't entirely believe it's to do with the unicode.

roborourke commented 3 years ago

@robindevitt that's good at least. So does the error happen once and then the cache value is set or does it keep happening?

roborourke commented 3 years ago

Fixed by humanmade/batcache#23