API requests use cache for service interruptions

[robmadole]

A service disruption on api.fontawesome.com took down several sites that were using Beaver Builder where those sites were querying Kits.

PHP Fatal error: Uncaught FortAwesome\ApiResponseException: An unexpected response
  was received from the Font Awesome API server.
    in /nas/content/live/REDACTED/wp-content/plugins/font-awesome/includes/class-fontawesome-exception.php:68

  Stack trace:
   #0  font-awesome/includes/class-fontawesome-metadata-provider.php(150)
         FortAwesome\FontAwesome_Exception::with_wp_response(Array)
   #1  font-awesome/includes/class-fontawesome.php(2919)
         FortAwesome\FontAwesome_Metadata_Provider->metadata_query('{ me { kit(toke...')
   #2  bb-plugin/classes/class-fl-builder-font-awesome.php(261)
         FortAwesome\FontAwesome->query('{ me { kit(toke...')
   #3  bb-plugin/classes/class-fl-builder-icons.php(243)
         FLBuilderFontAwesome::get_kit_data()
   #4  bb-plugin/classes/class-fl-builder-icons.php(46)
         FLBuilderIcons::register_custom_sets()
   #5  bb-plugin/classes/class-fl-builder-icons.php(505)
         FLBuilderIcons::get_sets()
   #6  bb-plugin/classes/class-fl-builder-icons.php(454)
         FLBuilderIcons::enqueue_styles_for_icon('', Array, Object(stdClass))
   #7  bb-plugin/classes/class-fl-builder-icons.php(425)
         FLBuilderIcons::enqueue_styles_for_nested_module_form(
             Object(FLContentSliderModule), Array, 'slides')
   #8  bb-plugin/classes/class-fl-builder-module.php(399)
         FLBuilderModule->enqueue_icon_styles()
   #9  bb-plugin/classes/class-fl-builder.php(638)
         FLBuilder::enqueue_module_layout_styles_scripts(Object(FLContentSliderModule))
   #10 bb-plugin/classes/class-fl-builder.php(616)
         FLBuilder::enqueue_layout_styles_scripts()
   #11 bb-plugin/classes/class-fl-builder.php(535)
         FLBuilder::enqueue_all_layouts_styles_scripts('')
   #12 wp-includes/class-wp-hook.php(341)
         WP_Hook->apply_filters(NULL, Array)
   #13 wp-includes/class-wp-hook.php(365)
         WP_Hook->do_action(Array)
   #14 wp-includes/plugin.php(522)
         do_action('wp_enqueue_scri...')
   #15 wp-includes/script-loader.php(2311)
         wp_enqueue_scripts('')
   #16 wp-includes/class-wp-hook.php(341)
         WP_Hook->apply_filters(NULL, Array)
   #17 wp-includes/class-wp-hook.php(365)
         WP_Hook->do_action(Array)
   #18 wp-includes/plugin.php(522)
         do_action('wp_head')
   #19 wp-includes/general-template.php(3197)
         wp_head()
   #20 themes/bb-theme/header.php(11)
         require_once('...')
   #21 wp-includes/template.php(814)
         load_template('...', true, Array)
   #22 wp-includes/template.php(749)
         locate_template(Array, true, true, Array)
   #23 wp-includes/general-template.php(48)
         get_header()
   #24 themes/bb-theme/page.php(1)
         include('...')
   #25 wp-includes/template-loader.php(132)
         require_once('...')
   #26 wp-blog-header.php(19)
         require('...')
   #27 index.php(17)
         {main}

  thrown in font-awesome/includes/class-fontawesome-exception.php on line 68

@mlwilkerson I think a better fix for this might be to chat with BB and let them control the behavior but this PR adds a "use cache on origin error" pattern that provides some insurance.

What do you think about this idea?

[mlwilkerson]

If I understand correctly, this may have unintended results some authorized queries. This function is a general purpose PHP interface for sending GraphQL queries to the API service. It should work for:

1. authorized queries as well as public/unauthorized ones
2. queries whose results change seldomly (like releases metadata) and frequently (like querying the properties of a kit that may have just changed)
3. queries across different access tokens or with the same query body

This would always cache any successful response for a month for an authorized query, but the cache key doesn't specify who its value is authorized for.

So if a WP user uses an API token for Account X, and does a query that lists their kits, and that's cached, and then changes to an API token for Account Y, and hits this cached response, they're going to get Account X's kits.

There's also the concern that a one month cache is pretty long. Font Awesome releases don't change very often, so that makes intuitive sense there. But kits might change much more often as users change settings, subsets, or custom icons. However, since the cache is only used when there's a failure, and is always updated on a successful query, then it may not matter that the cache expiring is one month, since it will effectively always just produce the value of the most recent successful query, prior to the failed request.

Another consideration is growing the cache. Because this is a general purpose interface for submitting any query documents--not just the canned query for Font Awesome releases--this could cache many queries with large responses. And it would cache ALL of them for a whole month. Imagine if this interface were used to make queries on the search field and it selects the first 100 hits, and lots of SVG icon data. Those responses would be large-ish, differentiated by arbitrary query strings (so, different cache keys) and cached for a whole month. That might really add up.

Also, in the PR description, you mentioned that the API outage "took down" several sites. That might mean that end user facing front page loads stopped working (major problem), or that admins or content editors on the back end couldn't make FA-specific changes during the outage (much smaller problem).

I would think this API service should never be queried on front end page loads. That's obviously a choice that Beaver Builder would have to make. But that's a commitment I've held in our official plugin. In our plugin, the only time the API is queried is when:

1. an admin on the back end makes configuration changes
2. a content creator/editor on the back end makes content changes (using search, for example--and in the latest version of our plugin, those searches go directly from the user's browser to the API service, not through through the back end)

So even if there were an API outage, or degraded performance, only those back end user scenarios would be impacted. Never the whole site. Never front end page loads.

[robmadole]

Also, in the PR description, you mentioned that the API outage "took down" several sites. That might mean that end user facing front page loads stopped working (major problem), or that admins or content editors on the back end couldn't make FA-specific changes during the outage (much smaller problem).

@mlwilkerson I checked with a few of the affected WP admins and it was the entire site, user-facing pages as well. The only way to restore the site was to disable the Font Awesome plugin. (This disabled the Beaver Builder integration I'm assuming.)

So if a WP user uses an API token for Account X, and does a query that lists their kits, and that's cached, and then changes to an API token for Account Y, and hits this cached response, they're going to get Account X's kits.

We could add the API token to the cache partition, but doing that might actually lead to a failure condition this technique is actively trying to guard against. The API key would be rotated out, the API server is down, and now there is no fallback cache since the key changed.

So I talked myself out of making the cache key too unique.

There's also the concern that a one month cache is pretty long.

Agreed. This was a first pass and I think something like 3 hours makes more sense. Just long enough to survive an outage.

Another consideration is growing the cache.

Reducing the TTL to 3 hours will help but it could still get rather large. @mlwilkerson looking at some information I can find about the transients it's not quite as advanced as I would have hoped. There doesn't seem to be a cache strategy (LFU or LRU) unless the site has enabled Redis or Memcached. And even at that point it's using eviction policies that the user doesn't control (just config for those servers I'm guessin).

This bums me out a bit. I think it might make this idea impractical.

I would think this API service should never be queried on front end page loads. That's obviously a choice that Beaver Builder would have to make. But that's a commitment I've held in our official plugin.

I thought I remembered you making that choice so I was really surprised when support tickets started pouring in. I eventually saw the pattern that the stacktraces all contained "bb-plugin" and pieced it together.

But if BB can fall into this trap, what other plugins may have? And is a wordpress-fontawesome plugin defensive layer warranted?

[mlwilkerson]

But if BB can fall into this trap, what other plugins may have?

Well, that's an interesting question. I shouldn't be a trap. Of course, the API service should be reliable and never have an outage. And it is actually much more reliable today than it was years ago when I chose not to rely on it for front end page loads in our plugin.

But the final reason not to rely on it is just because of performance. The API service broadly provides:

1. metadata
2. SVG icon data

I can't think of why metadata would need to be fetched on a front end page load.

And if its SVG icon data that are needed for front end page loads, then our Kits CDN should be preferred for that.

Whether our official plugin had this extra insurance or not, other plugins--and clients outside the WordPress ecosystem entirely--could use the API service in this way that I think is an anti-pattern.

So it shouldn't be a "trap" at all. But if it is, then there are a thousand ways to fall into it, and this particular function in our particular WordPress official is only one of them.

I'm still on the fence as to whether I think is a good idea. It seems to add cache state complexity to solve a problem in anti-pattern usage.

[robmadole]

I'm still on the fence as to whether I think is a good idea. It seems to add cache state complexity to solve a problem in anti-pattern usage.

@mlwilkerson agreed. I don't know that I'm convinced either. But we had real customers with real downtime and it happened through a plugin integration using a questionable pattern. Maybe we just have to address them as they come up though. We can let this PR stew and I can reach out to Beaver Builder to discuss why user-facing pages can be affected by our API being down.