fix: prevent classic-editor fatal on voices API JSON error body#569
Draft
galbus wants to merge 1 commit into
Draft
fix: prevent classic-editor fatal on voices API JSON error body#569galbus wants to merge 1 commit into
galbus wants to merge 1 commit into
Conversation
get_voices_for_language() validated only the top level of the voices
API response. A non-2xx body in the API's documented error shape
({"message": "..."} -- e.g. a 429 or 5xx) decodes to
['message' => '<string>'], which passed the is_array() check but left
a scalar element. Under strict_types that scalar fataled in the
metabox render helpers (find_voice() / voice_model_key(), both typed
for arrays), taking down wp-admin/post.php for every classic-editor
load of a customized post for the duration of the API incident.
Fix:
- Harden shape validation to element level in
get_voices_for_language() and its sibling get_languages()
(array_values(array_filter($voices, 'is_array'))).
- Loosen the public voice_model_key() to accept mixed and bucket
non-array values as Standard, mirroring the tolerant JS
voiceModelKey() guard.
This also covers a malformed 2xx object body, which cached_get()
would otherwise cache and serve for 15 minutes.
Add a regression test reproducing the 429 error-object render, plus
scalar/null cases for voice_model_key().
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
✅ WordPress Plugin Check Report
📊 ReportAll checks passed! No errors or warnings found. 🤖 Generated by WordPress Plugin Check Action • Learn more about Plugin Check |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
On the classic edit screen, opening any customized post (one with a saved
beyondwords_language_code) throws an uncatchableTypeErrormid-metabox-render when the voices API answers with a JSON error object instead of a voice list.SelectVoice::get_voices_for_language()validated only the top level of the API response:Client::cached_get()returnsjson_decode( body, true )unconditionally, so a non-2xx response whose body is the BeyondWords API's own documented error shape —{"message": "..."}(a 429 rate-limit, 5xx maintenance, 401 after key revocation) — decodes to['message' => '<string>']. That passes the top-levelis_array()check, but the string element then flows into the render helpers underdeclare(strict_types=1):find_voice()returns the string against its?arrayreturn type (when the post has no saved voice id), orlanguage_models()→voice_model_key( array $voice )receives the string against itsarrayparameter (when a non-matching voice id is saved).Either way →
TypeError, andwp-admin/post.phpis left half-rendered and unusable. Because 429/5xx bodies aren't cached, every page load re-triggers it for the duration of the incident. A malformed 2xx object body is worse:cached_get()caches it, so the breakage sticks for the full 15-minute TTL.(The
{"errors": [...]}shape does not fatal — its elements are arrays — so only the common{"message": "..."}shape is affected.)Fix
Two independent layers, both defensible on their own:
get_voices_for_language()now filters to array elements —array_values( array_filter( $voices, 'is_array' ) )— so a scalar from an error body can never reach the render helpers. The same element-level filter is applied to the siblingget_languages().voice_model_key()is loosened fromarray $voicetomixed $voicewith anis_array()guard, bucketing any non-record value asstandard. This makes the PHP a faithful mirror of the tolerant JSvoiceModelKey()(voice?./voice &&) it already claims to mirror, and hardens the publiclanguage_models()helper for any caller.Net effect: on an API error the Model/Voice dropdowns degrade to empty instead of fataling.
Why the fix is at the consumer, not the cache
I deliberately left
Client::cached_get()alone. Restricting it to list-shaped payloads would break the object-returning endpoints that legitimately cache non-list objects (get_project(),get_video_settings(), …). The element-level invariant only makes sense for the list consumers (voices, languages), so the guard belongs there — and it covers the cached-object variant regardless.Tests
element_degrades_gracefully_when_voices_api_returns_error_object— mirrors the existing languages-failure precedent, intercepting the voices request with a429 {"message":"Too many requests"}body and asserting the render completes with both dropdowns hidden. Verified it fatals against the pre-fix code and passes with the fix.voice_model_keyunit test extended with scalar /nullcases.Reviewer notes
phpcs(WordPress-VIP-Go) clean, nophpcs:ignoreadded;SelectVoiceTest12 tests / 61 assertions, all green.voice?./voices ?? []; this was a PHP-onlystrict_typesfatal, so no JS changes.--no-verify: the grumphp pre-commit hook couldn't run in this throwaway worktree checkout, but its checks (phpcs,php -l, ≤72-char commit body) were all run manually.🤖 Generated with Claude Code