Adds HTTP Client Adapter #2

JasonTheAdams · 2025-08-30T19:38:23Z

This PR will add the Client Adapter so that the PHP AI Client uses the WP functions to fulfill requests.

includes/http/class-wp-ai-client-client-adapter.php

dkotter

In testing out implementing the PHP AI Client package in one of our plugins, ran into an issue where our E2E tests all fail. The issue for us is we install a test plugin during our E2E tests that hooks into pre_http_request and returns mocked responses (so we don't have to hit actual APIs every time we run tests).

This doesn't work when requests are sent by the PHP AI Client package as it doesn't use the WordPress HTTP API. I had thought that this PR might solve that but still getting the same problems while testing this. Apologies if I'm missing something obvious here but are there additional steps required to ensure this adapter is used?

felixarntz · 2025-09-04T18:05:59Z

@dkotter You're on the bleeding edge 😆

This is all early WIP. This adapter is indeed supposed to fix the problem you mentioned. It's not a bug in the PHP AI Client SDK since it is WordPress agnostic, but that kind of thing is exactly why this package exists.

As far as I can tell, this PR implements the adapter but doesn't use it yet, but that will be added. Apologies for the confusion, but it might be a bit too early to test this package in particular as it's not meant to be usable just yet. That'll hopefully be different in a week or so :)

dkotter · 2025-09-04T18:19:53Z

It's not a bug in the PHP AI Client SDK since it is WordPress agnostic, but that kind of thing is exactly why this package exists.

Correct, not calling that out as a bug, just hadn't thought of that until I opened a PR and saw all our tests fail 😭

As far as I can tell, this PR implements the adapter but doesn't use it yet, but that will be added. Apologies for the confusion, but it might be a bit too early to test this package in particular as it's not meant to be usable just yet. That'll hopefully be different in a week or so

No worries, sounds good. Realize I'm jumping in early here, was just trying to help do some testing on this PR to help get it merged but realize there's lots of things still in progress across all these work streams

JasonTheAdams · 2025-09-09T23:16:18Z

@felixarntz This is back to you for review! A couple things to note:

First, I updated the class names and such to reflect PSR-4, and I updated some types stuff to pass PHPStan. We'll simply have more use of @var and ignoring in this project to get things to play nicely between PSR and WordPress. It is what it is. 🤷‍♂️

Second, I added a DiscoveryStrategy implementing class. As far as I can tell, as it's not well documented, this is how to make this discoverable to the HTTPlug system. You'll notice WP_AI_Client_Discovery_Strategy has an init() method, which is what would be called to prepend this to the discovery system. I believe you said you'd like this to be ready for another system to call said methods instead of this package bootstrapping itself?

Third, I added the nyholm/psr7 package to provide a PSR17 factory. It seems to be the de facto lightweight solution and is PHP 7.2 compatible. My only concern is this issue (Nyholm/psr7#256) which points out some basic PHP 8.4 deprecation fixes that's been neglected.

What do you think? 😄

felixarntz

@JasonTheAdams Overall, this all makes sense to me. A bit of feedback regarding the WordPress specific implementation.

includes/http/WP_AI_Client_Client_Adapter.php

felixarntz · 2025-09-10T20:40:19Z

includes/http/WordPress_HTTP_Client.php

+			// TODO: Update to use PHP AI Client exceptions.
+			throw new \Exception(
+				$response->get_error_message(), // phpcs:ignore WordPress.Security.EscapeOutput.ExceptionNotEscaped
+				$response->get_error_code() ? (int) $response->get_error_code() : 0


This doesn't work in 99% of scenarios, WordPress error codes are almost always strings with text.

What I would suggest is:

Either introduce something like a WP_Error_Exception that allows to maintain the WordPress error codes too, via an additional property.

Or rely on one of the HTTP request/response specific exceptions that are being added as part of Implement Exception Hierarchy For Better Error Handling php-ai-client#78.

Let's see if we can get the exception PR done soon and I'll use that.

Finally resolved in de7de24!

My comment here is still relevant I think: What I'm saying is it doesn't make sense to interpret a WP_Error's error code as an integer, since it's meant to be a string.

Per the above, I think we should simply remove this line. There's no reasonable way to maintain the WordPress error code within the exception, unless we come up with a new exception class that can also store a string-shape as error code. But that's overkill IMO.

includes/http/WordPress_HTTP_Client.php

felixarntz

@JasonTheAdams This looks excellent, great work!

A few questions, and two small things to ideally address before merge.

felixarntz · 2025-10-29T22:23:36Z

includes/http/WP_AI_Client_Discovery_Strategy.php

+		// PSR-17 factories - Nyholm's Psr17Factory implements all of them.
+		$psr17_factories = array(


This is mostly a question for my own understanding: Why do we need this? Why wouldn't we be able to only inject our WordPress HTTP Client and leave the rest?

felixarntz · 2025-10-29T22:24:32Z

includes/http/WP_AI_Client_Discovery_Strategy.php

+			return array(
+				array(
+					'class'     => Psr17Factory::class,
+					'condition' => Psr17Factory::class,


What is the condition field for? And why is it simply the same as class?

felixarntz · 2025-10-29T22:25:16Z

includes/http/WP_AI_Client_Discovery_Strategy.php

+	 * @since n.e.x.t
+	 * @return void
+	 */
+	public static function init() {


I assume this method is the entry point to all this? As in: Consuming code will need to call this static method to wire up our WordPress HTTP Client with the PHP AI Client SDK, and nothing else, right?

felixarntz · 2025-10-29T22:31:16Z

includes/http/WordPress_HTTP_Client.php

+			$message = sprintf(
+				'Network error occurred while sending request to %s: %s',
+				$url,
+				$response->get_error_message()
+			);


Minor suggestion: Include the request method here as well.

Suggested change

$message = sprintf(

'Network error occurred while sending request to %s: %s',

$url,

$response->get_error_message()

);

$message = sprintf(

'Network error occurred while sending %s request to %s: %s',

$request->getMethod(),

$url,

$response->get_error_message()

);

felixarntz · 2025-10-29T22:32:18Z

includes/http/WordPress_HTTP_Client.php

+			// TODO: Update to use PHP AI Client exceptions.
+			throw new \Exception(
+				$response->get_error_message(), // phpcs:ignore WordPress.Security.EscapeOutput.ExceptionNotEscaped
+				$response->get_error_code() ? (int) $response->get_error_code() : 0


Per the above, I think we should simply remove this line. There's no reasonable way to maintain the WordPress error code within the exception, unless we come up with a new exception class that can also store a string-shape as error code. But that's overkill IMO.

feat: starts client adapter work

485cd6a

JasonTheAdams self-assigned this Aug 30, 2025

JasonTheAdams mentioned this pull request Aug 30, 2025

Project MVP Roadmap #1

Open

6 tasks

felixarntz reviewed Aug 30, 2025

View reviewed changes

includes/http/class-wp-ai-client-client-adapter.php Outdated Show resolved Hide resolved

Merge branch 'trunk' into http-adapter

182e32a

dkotter reviewed Sep 4, 2025

View reviewed changes

JasonTheAdams added 7 commits September 9, 2025 11:00

Merge branch 'trunk' into http-adapter

51a5130

refactor: rename file to use PSR-4 autoloading

c06c468

chore: adds WordPress stubs to PHPStan

845407a

fix: corrects type issues

d0b9208

chore: cleans up phpstan file

d7ef812

feat: adds nyholm/psr7 package for psr17 factory

f7ec38f

feat: adds HTTPlug discovery strategy

8010654

JasonTheAdams marked this pull request as ready for review September 9, 2025 23:07

JasonTheAdams requested a review from felixarntz September 9, 2025 23:07

feat: adds PSR17 discoverability

4e0247f

neillmcshea added this to the 0.1.0 milestone Sep 10, 2025

felixarntz requested changes Sep 10, 2025

View reviewed changes

JasonTheAdams added 2 commits September 22, 2025 10:51

refactor: renames class

3998eb7

refactor: removes stream args

581de9e

felixarntz mentioned this pull request Sep 24, 2025

Add streaming support to WordPress_HTTP_Client #11

Open

JasonTheAdams added 3 commits October 21, 2025 16:18

feat: updates to PHP AI Client 0.2.0

5c66fde

feat: implements ClientWithOptionsInterface

4e80b89

refactor: switches to using NetworkException

de7de24

JasonTheAdams requested a review from felixarntz October 21, 2025 22:25

felixarntz approved these changes Oct 29, 2025

View reviewed changes

felixarntz added the [Feature] New feature to highlight in changelogs. label Oct 29, 2025

		// PSR-17 factories - Nyholm's Psr17Factory implements all of them.
		$psr17_factories = array(

Adds HTTP Client Adapter #2

Are you sure you want to change the base?

Adds HTTP Client Adapter #2

Uh oh!

Conversation

JasonTheAdams commented Aug 30, 2025

Uh oh!

Uh oh!

dkotter left a comment

Choose a reason for hiding this comment

Uh oh!

felixarntz commented Sep 4, 2025

Uh oh!

dkotter commented Sep 4, 2025

Uh oh!

JasonTheAdams commented Sep 9, 2025

Uh oh!

felixarntz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

felixarntz left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants