Repository navigation
Conversation
Logging_Http_Transporter::send() initialized $status to 'success' and
only changed it in the catch block for thrown exceptions. The SDK's
HttpTransporterInterface::send() only throws for PSR-18 network or
client exceptions; a non-2xx response (429, 503, etc.) comes back as
an ordinary Response and is rejected later by the caller, after the
log row was already written as 'success' with no status code or error
message.
Check Response::isSuccessful() after the transporter returns and log
'error' with the HTTP status code and, when present, the provider's
JSON error message. The status code is also recorded in the log
context so error rows are filterable.
Also fix detect_request_kind() mislabeling every Gemini text
generation request as 'metadata': Google appends the method name to
the model resource path as `:generateContent` or
`:streamGenerateContent` (e.g. `/v1beta/models/{model}:generateContent`),
which matched the '/models/' metadata rule before this fix.
Fixes WordPress#1032
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the Unlinked AccountsThe following contributors have not linked their GitHub and WordPress.org accounts: @nielash. Contributors, please read how to link your accounts to ensure your work is properly credited in WordPress releases. If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #1040 +/- ##
=============================================
+ Coverage 80.83% 81.33% +0.50%
- Complexity 3007 3013 +6
=============================================
Files 125 125
Lines 12011 12025 +14
=============================================
+ Hits 9709 9781 +72
+ Misses 2302 2244 -58
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
What?
Closes #1032
Fixes
Logging_Http_Transporterso a non-2xx HTTP response from a provider (429, 503, etc.) is logged aserrorwith the status code and message, instead ofsuccess. Also fixesLog_Data_Extractor::detect_request_kind()mislabeling every Gemini text generation request asMetadata.Why?
Logging_Http_Transporter::send()initializes$statusto'success'and only changes it in thecatch ( Throwable $e )branch. The SDK'sHttpTransporterInterface::send()only throws for PSR-18 network or client exceptions; a non-2xx response comes back as an ordinaryResponse, and the provider rejects it later (e.g.GoogleTextGenerationModel::generateTextResult()callsResponseUtil::throwIfNotSuccessful()right aftersend()returns). By the time that exception surfaces to the user, the log row has already been written assuccess, with no status code, no error message, no token counts, and no output preview — so the AI Request Logs screen looks healthy while a feature is failing on every request.A second, smaller problem on the same rows:
detect_request_kind()returnsmetadatafor any path containing/models/. Every Gemini generation URL has the form.../v1beta/models/{model}:generateContent, so all Google text requests were labeled Metadata instead of Text.How?
includes/Logging/Logging_Http_Transporter.php: after the transporter returns a response, checkResponse::isSuccessful(). When it's false, set$status = 'error', build an error message withdescribe_error_response()(the HTTP status code, plus the provider'serror.messagefrom the JSON body when present), and record the status code in$log_data['context']['http_status']so error rows are filterable.includes/Logging/Log_Data_Extractor.php:detect_request_kind()now checks for a:generateContent/:streamGenerateContentsuffix before applying the/models/→metadatarule, so Gemini generation requests fall through to the normal text/embeddings/image/audio detection (and default totext) instead of being misclassified.Logging_Http_TransporterTest(non-2xx response with a JSON error body, non-2xx without a body, a normal 2xx success, and the pre-existing thrown-exception path) and inLog_Data_ExtractorTest(:generateContentand:streamGenerateContentpaths).This only touches
Logging_Http_Transporter::send(), the same method #757 edits for its event-listener fallback; the two PRs will need to be rebased against each other, whichever lands second.Testing Instructions
composer installvendor/bin/phpunit --filter "Logging_Http_TransporterTest|Log_Data_ExtractorTest"Note: I wasn't able to execute the WP-integrated PHPUnit suite in my local environment — my local WordPress core checkout has a pre-existing, unrelated fatal error (a signature mismatch between
wp-includes/ai-client'sWP_AI_Client_HTTP_Clientandwp-includes/php-ai-client'sClientWithOptionsInterface) that breaks the bootstrap for this plugin's test suite entirely. I confirmed this reproduces identically on a cleandevelopcheckout with none of this PR's changes, so it's an environment issue, not something introduced here. I did verifyphp -l,phpcs --standard=phpcs.xml.dist, andphpstan analyseall pass cleanly on the changed files, and read through the new/updated test bodies carefully against the SDK's actualResponseDTO (isSuccessful(),getStatusCode(),getData()) to confirm they exercise the fix correctly. It'd be great if a maintainer (or CI) could confirm the test suite passes in a working environment.Manual repro steps from the issue, to confirm end-to-end:
error, the HTTP status code, and the provider's error message, with Request KindTextrather thanMetadata.Changelog Entry