fix: bound blocking editor-dropdown API calls on the metabox render path

src/api/class-client.php

@@ -46,6 +46,17 @@ class Client {
 	 */
 	const CACHE_TTL = 15 * MINUTE_IN_SECONDS;
 
+	/**
+	 * How long to negative-cache a *failed* editor-dropdown fetch (WP_Error /
+	 * timeout, 5xx, 4xx, non-JSON). Much shorter than CACHE_TTL so a transient
+	 * outage self-heals within minutes, but long enough that a slow or
+	 * unreachable API is probed at most once per interval rather than blocking
+	 * every admin edit-screen render.
+	 *
+	 * @since 7.0.0
+	 */
+	const CACHE_TTL_ON_ERROR = 2 * MINUTE_IN_SECONDS;
+
 	/**
 	 * Default timeout, in seconds, for a BeyondWords API request.
 	 *
@@ -70,6 +81,19 @@ class Client {
 	 */
 	const DELETE_TIMEOUT = 3;
 
+	/**
+	 * Timeout, in seconds, for a cached render-path GET (editor dropdowns).
+	 *
+	 * These run inline on every classic-editor metabox render, so a long
+	 * blocking timeout would tie up a PHP-FPM worker for the length of an admin
+	 * page load. Short, per VIP guidance (see `vip_safe_wp_remote_get`), and
+	 * paired with negative caching ({@see CACHE_TTL_ON_ERROR}) to bound repeat
+	 * failures.
+	 *
+	 * @since 7.0.0
+	 */
+	const RENDER_TIMEOUT = 3;
+
 	/**
 	 * Register WordPress hooks.
 	 *
@@ -529,7 +553,8 @@ public static function call_api( string $method, string $url, string $body = '',
 	 * @param string               $body    Request body.
 	 * @param array<string,string> $headers Extra per-request headers.
 	 * @param int                  $timeout Request timeout in seconds. Defaults to REQUEST_TIMEOUT;
-	 *                                      DELETEs pass the shorter DELETE_TIMEOUT.
+	 *                                      DELETEs pass the shorter DELETE_TIMEOUT and cached
+	 *                                      render-path GETs the shorter RENDER_TIMEOUT.
 	 *
 	 * @return array<string,mixed>
 	 */
@@ -565,17 +590,25 @@ private static function cache_key( string $suffix ): string {
 	}
 
 	/**
-	 * GET an endpoint, caching successful list/object responses.
+	 * GET an editor-render-path endpoint, caching both hits and failures.
+	 *
+	 * Successful 2xx array responses are cached for {@see CACHE_TTL}. Failures
+	 * (WP_Error / timeout, 5xx, 4xx, non-JSON) are negative-cached for the much
+	 * shorter {@see CACHE_TTL_ON_ERROR}, so a slow or unreachable API is probed
+	 * at most once per interval instead of re-issuing a blocking request on
+	 * every admin render. The request itself uses the short
+	 * {@see RENDER_TIMEOUT} for the same reason.
 	 *
-	 * Only 2xx array responses are cached, so a transient API error retries on
-	 * the next call rather than caching an empty/error payload for the full TTL.
+	 * A 401 additionally self-heals: call_api() clears
+	 * `beyondwords_valid_api_connection`, which ungates the metabox next load.
 	 *
 	 * @since 7.0.0
 	 *
 	 * @param string $suffix Cache-key suffix (include any project/language id).
 	 * @param string $url    Absolute endpoint URL.
 	 *
-	 * @return array<mixed>|null|false
+	 * @return array<mixed>|null|false Decoded body on the fetching call; the cached
+	 *                                 value ([] after a cached failure) thereafter.
 	 */
 	private static function cached_get( string $suffix, string $url ): array|null|false {
 		$key    = self::cache_key( $suffix );
@@ -585,7 +618,7 @@ private static function cached_get( string $suffix, string $url ): array|null|fa
 			return $cached;
 		}
 
-		$response = self::call_api( 'GET', $url );
+		$response = self::call_api( 'GET', $url, '', false, [], self::RENDER_TIMEOUT );
 		$decoded  = json_decode( wp_remote_retrieve_body( $response ), true );
 
 		if (
@@ -594,8 +627,14 @@ private static function cached_get( string $suffix, string $url ): array|null|fa
 			&& is_array( $decoded )
 		) {
 			set_transient( $key, $decoded, self::CACHE_TTL );
+
+			return $decoded;
 		}
 
+		// Negative-cache the failure so the next render is served from the
+		// transient instead of blocking on another remote round-trip.
+		set_transient( $key, [], self::CACHE_TTL_ON_ERROR );
+
 		return $decoded;
 	}
 

tests/phpunit/api/test-client.php

@@ -541,12 +541,15 @@ public function editor_dropdown_responses_are_cached()
      * @test
      * @group settings
      *
-     * A non-2xx response is not cached, so the next call retries the API rather
-     * than serving a cached error for the full TTL.
+     * A failed editor-dropdown fetch is negative-cached for a short TTL, so a
+     * slow or unreachable API is probed at most once per interval instead of
+     * blocking every admin edit-screen render. The second call within the TTL
+     * is served from the transient (the empty-array sentinel) and makes no HTTP
+     * request.
      */
-    public function failed_responses_are_not_cached()
+    public function failed_responses_are_negative_cached()
     {
-        // No API key → the mock returns a 401, which must not be cached.
+        // No API key → the mock returns a 401, which is negative-cached.
         $calls   = 0;
         $counter = function ($preempt, $args, $url) use (&$calls) {
             if (str_contains((string) $url, '/summarization_settings_templates')) {
@@ -556,12 +559,58 @@ public function failed_responses_are_not_cached()
         };
         add_filter('pre_http_request', $counter, 0, 3);
 
-        Client::get_summarization_settings_templates();
-        Client::get_summarization_settings_templates();
+        $first  = Client::get_summarization_settings_templates();
+        $second = Client::get_summarization_settings_templates();
 
         remove_filter('pre_http_request', $counter, 0);
 
-        $this->assertSame(2, $calls, 'Errors should not be cached');
+        $this->assertSame(1, $calls, 'A failed response should be negative-cached, so the second call is served from the transient');
+        $this->assertSame([], $second, 'The negative-cache sentinel is an empty array');
+    }
+
+    /**
+     * @test
+     * @group settings
+     *
+     * Cached editor-dropdown GETs render on the admin edit screen, so they use
+     * a short (<= 3s) timeout to avoid tying up a PHP worker when the API is
+     * slow. Write-path (and other direct call_api) requests keep the longer
+     * timeout so user-initiated saves aren't dropped mid-generation.
+     */
+    public function render_path_gets_use_a_short_timeout()
+    {
+        update_option('beyondwords_api_key', BEYONDWORDS_TESTS_API_KEY);
+        update_option('beyondwords_project_id', BEYONDWORDS_TESTS_PROJECT_ID);
+
+        $timeouts = [];
+        $capture  = function ($preempt, $args, $url) use (&$timeouts) {
+            $timeouts[(string) $url] = $args['timeout'] ?? null;
+            return $preempt;
+        };
+        add_filter('pre_http_request', $capture, 0, 3);
+
+        Client::get_summarization_settings_templates();      // cached_get → short timeout
+        Client::get_content(BEYONDWORDS_TESTS_CONTENT_ID);   // call_api  → long timeout
+
+        remove_filter('pre_http_request', $capture, 0);
+
+        $render_timeout = null;
+        $write_timeout  = null;
+        foreach ($timeouts as $url => $timeout) {
+            if (str_contains($url, '/summarization_settings_templates')) {
+                $render_timeout = $timeout;
+            } elseif (str_contains($url, '/content/')) {
+                $write_timeout = $timeout;
+            }
+        }
+
+        $this->assertNotNull($render_timeout, 'Expected a render-path request to be captured');
+        $this->assertSame(Client::RENDER_TIMEOUT, $render_timeout, 'Render-path GETs should use the short RENDER_TIMEOUT');
+        $this->assertLessThanOrEqual(3, $render_timeout, 'RENDER_TIMEOUT must stay within VIP guidance (<= 3s)');
+        $this->assertSame(Client::REQUEST_TIMEOUT, $write_timeout, 'Direct/write-path requests keep the longer default timeout');
+
+        delete_option('beyondwords_api_key');
+        delete_option('beyondwords_project_id');
     }
 
     /**