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

Don't cache exceptions nor WP_Error objects #1

Merged
merged 3 commits into from
Apr 14, 2018

Conversation

stevegrunwell
Copy link
Owner

We should do our best to avoid caching WP_Error objects or PHP exceptions, as these are typically not things we want to cache. Explicitly checking for WP_Error objects (via is_wp_error() also enables developers to short-circuit the cache, if necessary.

For example, assume we have the following function:

remember_transient( 'some-cache-key', function () {
  $response = wp_remote_get( 'https://example.com' );

  // Something went wrong with the request.
  if ( is_wp_error( $response ) ) {
    return $response;
  } elseif ( 200 !== wp_remote_retrieve_response_code( $response ) ) {
    return new WP_Error( 'api-error', 'Received a non-200 status code.' );
  }

  return wp_remote_retrieve_body( $response );
}, DAY_IN_SECONDS );

If the response failed or returned a non-200 status code, that value could be cached for a day; temporary latency, a now-fixed bug, or any other number of reasons could result in stale errors being returned to users.

With this PR, the returned value is run through is_wp_error() before being stored in the object cache, transients, or site transients.

@stevegrunwell stevegrunwell added the enhancement New feature or request label Apr 14, 2018
@coveralls
Copy link

Pull Request Test Coverage Report for Build 22

  • 6 of 6 (100.0%) changed or added relevant lines in 1 file are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage remained the same at 100.0%

Totals Coverage Status
Change from base Build 20: 0.0%
Covered Lines: 38
Relevant Lines: 38

💛 - Coveralls

@stevegrunwell stevegrunwell merged commit 1492646 into develop Apr 14, 2018
@stevegrunwell stevegrunwell deleted the fix/no-caching-on-error branch April 14, 2018 18:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants