Make WordPress Core

Opened 4 months ago

Closed 3 months ago

Last modified 3 months ago

#65421 closed defect (bug) (fixed)

AI Client: sendRequestWithOptions omits HTTP method from NetworkException message

Reported by: sagardeshmukh Owned by: gziolo
Priority: normal Milestone: 7.1
Component: AI Version: 7.0
Severity: normal Keywords: has-patch has-unit-tests
Cc: Focuses:

Description

Problem

WP_AI_Client_HTTP_Client has two methods for sending HTTP requests:
sendRequest() and sendRequestWithOptions(). When a WordPress HTTP error
occurs, both throw a NetworkException - but their error messages are
inconsistent.

sendRequest() includes the HTTP method, URL, and underlying error:

Network error occurred while sending POST request to ​https://…: Connection refused

sendRequestWithOptions() omits the HTTP method:

Network error occurred while sending request to ​https://…: Connection refused

When debugging a failed AI request, the HTTP method is an important detail
(e.g. knowing whether it was a GET or POST helps narrow down the issue).
Both code paths should produce equivalent diagnostic messages.

Relevant code

src/wp-includes/ai-client/adapters/class-wp-ai-client-http-client.php

sendRequest() — line ~76:

$message = sprintf(

/* translators: 1: HTTP method (e.g. GET, POST). 2: Request URL. 3: Error message. */
( 'Network error occurred while sending %1$s request to %2$s: %3$s' ),
$request->getMethod(),
$url,
$response->get_error_message()

);

sendRequestWithOptions() — line ~107:

$message = sprintf(

/* translators: 1: Request URL. 2: Error message. */
( 'Network error occurred while sending request to %1$s: %2$s' ),
$url,
$response->get_error_message()

);

Proposed fix

Update sendRequestWithOptions() to match sendRequest()'s format:

$message = sprintf(

/* translators: 1: HTTP method (e.g. GET, POST). 2: Request URL. 3: Error message. */
( 'Network error occurred while sending %1$s request to %2$s: %3$s' ),
$request->getMethod(),
$url,
$response->get_error_message()

);

A test should also be added to assert the method, URL, and underlying
error message all appear in the thrown NetworkException.

Introduced in

This code was introduced in #64591.

Change History (6)

This ticket was mentioned in ​PR #12108 on ​WordPress/wordpress-develop by ​@sagardeshmukh.


4 months ago
#1

  • Keywords has-unit-tests added

## Summary

  • WP_AI_Client_HTTP_Client::sendRequestWithOptions() omitted the HTTP method from its NetworkException message, while sendRequest() included it. Both now use the same three-argument format so error output carries consistent diagnostic context (method + URL + underlying error).
  • Adds a test asserting the method, URL, and WP_Error message all appear in the exception thrown by sendRequestWithOptions.
  • Adds a missing test for WP_AI_Client_Prompt_Builder::generate_result() when wp_supports_ai() returns false - this code path produced a distinct error message ("AI features are not supported in this environment.") that was untested.

## Changes

  • src/wp-includes/ai-client/adapters/class-wp-ai-client-http-client.php
  • tests/phpunit/tests/ai-client/wpAiClientHttpClient.php
  • tests/phpunit/tests/ai-client/wpAiClientPromptBuilder.php

## Test coverage

Run: phpunit --group ai-client

https://core.trac.wordpress.org/ticket/65421
https://core.trac.wordpress.org/ticket/65422

#2 @jorbin
4 months ago

  • Component REST API → AI
  • Milestone Awaiting Review → 7.1

Hi @sagardeshmukh, welcome to WordPress Core Trac!

Moving to 7.1 for consideration.

I think it would be helpful to also include a test that checks and ensures the strings stay consistent.

Adds a missing test for WP_AI_Client_Prompt_Builder::generate_result() when wp_supports_ai() returns false - this code path produced a distinct error message ("AI features are not supported in this environment.") that was untested.

This should get handled on it's own. Can you put this piece in its own PR attached to #64894?

​@gziolo commented on ​PR #12108:


4 months ago
#3

@sagarsdeshmukh – I updated the description and removed the reference to https://core.trac.wordpress.org/ticket/65422. It triggered the automation and closed this PR when another ticket was closed. Can you refresh this PR to exclude the test that was merged with that ticket?

#4 @gziolo
4 months ago

  • Owner set to gziolo
  • Status new → reviewing

#5 @gziolo
3 months ago

  • Resolution → fixed
  • Status reviewing → closed

In 62753:

AI Client: Include the HTTP method in the sendRequestWithOptions() network error message.

WP_AI_Client_HTTP_Client::sendRequestWithOptions() omitted the HTTP method from its NetworkException message, while sendRequest() already included it. Both now use the same format, and a new test covers it.

Props sagarsdeshmukh, jorbin, gziolo.
Fixes #65421.

Note: See TracTickets for help on using tickets.