Implement Exception Hierarchy For Better Error Handling #78

Ref34t · 2025-09-01T13:19:00Z

Summary
Implements exception hierarchy to improve error handling across the WordPress AI Client ecosystem, directly addressing feedback from PR 2 in https://github.com/WordPress/wp-ai-client

Changes

New Exception Classes

AiClientExceptionInterface - Base interface for catch-all exception handling
NetworkException - HTTP transport errors, connection failures, timeouts
RequestException - API authentication failures, rate limiting, malformed requests
InvalidArgumentException - Project-scoped extension of \InvalidArgumentException
RuntimeException - Project-scoped extension of \RuntimeException

Updated Code

Updated existing exception usage in AiClient.php, GenerativeAiResult.php, FunctionResponse.php
Modified ResponseException to extend RequestException for better categorization
Added comprehensive test coverage

Benefits

Unified Exception Handling:

try {
    $result = AiClient::prompt('Generate content')->generateText();
} catch (AiClientExceptionInterface $e) {
    // Catches ALL AI Client exceptions
}

Better Error Categorization:

Network issues → NetworkException
API/Auth issues → RequestException
Invalid arguments → InvalidArgumentException

Related

Addresses feedback from 2 review comments about improving exception handling for wp-ai-client integration. https://github.com/WordPress/wp-ai-client

Ref34t · 2025-09-01T13:28:41Z

@felixarntz This implements the exception hierarchy you suggested in PR 2 in the wp-ai-client review. Now developers can catch all AI Client exceptions with catch (AiClientExceptionInterface $e) for unified error handling, while still having specific exception types for different error categories (network, request, validation). The implementation is minimal and focused - exactly what was requested for better WP-ai-client integration.

felixarntz

@Ref34t Thank you for getting this started!

A few points of feedback below, and one higher-level point here: I think we should put the exceptions into specific directories. For example:

InvalidArgumentException and RuntimeException into src/Common/Exception
NetworkException and RequestException into src/Providers/Http/Exception (where we already have ResponseException)

@JasonTheAdams It would be great to get your review as well - including regarding my own feedback.

src/Exceptions/NetworkException.php

src/Exceptions/RequestException.php

src/Providers/Http/Exception/ResponseException.php

JasonTheAdams · 2025-09-02T15:08:57Z

I agree with @felixarntz! I like the direction, but we need to see these exceptions being used in a few places to indicate their concrete usage.

github-actions · 2025-09-02T15:09:07Z

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 props-bot label.

Unlinked Accounts

The following contributors have not linked their GitHub and WordPress.org accounts: @[email protected].

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.

Unlinked contributors: [email protected].

Co-authored-by: felixarntz <[email protected]>
Co-authored-by: JasonTheAdams <[email protected]>
Co-authored-by: Ref34t <[email protected]>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

Ref34t · 2025-09-02T22:03:53Z

@felixarntz @JasonTheAdams
I've spent some time refactoring and improving the enhancements, you can see below what I did:

Architectural Reorganization

Moved exceptions to proper directories:

InvalidArgumentException & RuntimeException → src/Common/Exception/
NetworkException & RequestException → src/Providers/Http/Exception/
Updated 50+ import statements across the entire codebase

Concrete Usage Examples

NetworkException: PSR-18 network error handling in HttpTransporter::send()
RequestException: 400 Bad Request response handling in HttpTransporter::send()
ResponseException: Missing API data handling in OpenAiModelMetadataDirectory
InvalidArgumentException: 50+ validation locations throughout codebase
RuntimeException: 40+ operational error locations throughout codebase

Proper Inheritance

Fixed inheritance relationships:

RequestException extends InvalidArgumentException (for bad request data validation)
ResponseException extends RuntimeException (for unexpected provider responses)
NetworkException extends RuntimeException (for network connectivity issues)

Enhanced Developer Experience (Static factory methods suggestion)

"Simply adding static from* methods to the various *Exception classes"

RequestException (3 methods):

RequestException::fromInvalidParam('OpenAI', 'temperature', 'Must be between 0 and 2');
RequestException::fromBadRequestResponse('OpenAI', $response);
RequestException::fromBadRequestToUri($uri, 'Invalid  JSON payload');

ResponseException (4 methods):

ResponseException::fromMissingData('OpenAI', 'data');
ResponseException::fromUnexpectedStructure('OpenAI', 'array', 'string');
ResponseException::fromMalformedResponse('OpenAI', 'Invalid JSON format');
ResponseException::fromParsingFailure('OpenAI', 'model list', $jsonException);

NetworkException (5 methods):

NetworkException::fromPsr18NetworkException($uri, $networkException);
NetworkException::fromConnectionFailure($uri, 'Connection refused');
NetworkException::fromTimeout($uri, 'request', 30);
NetworkException::fromDnsFailure('api.openai.com');
NetworkException::fromSslError($uri, 'Certificate  validation failed');

Kindly review and let me know if you have any concerned

JasonTheAdams

Thanks, @Ref34t! I'm not sure I see the point in creating our own InvalidArgumentException and RuntimeException classes. Do you, @felixarntz?

felixarntz · 2025-09-02T23:20:03Z

@JasonTheAdams I do actually. It's simply about providing classes that can implement a common interface to identify "this is a specific exception from our PHP AI Client SDK".

Consuming code can then decide whether they want to catch e.g. RuntimeException and/or InvalidArgumentException or whether they want to do a single catch-all for "known" exceptions from our SDK only by catching AiClientExceptionInterface.

JasonTheAdams · 2025-09-03T15:44:17Z

I like that, @felixarntz! Admittedly, I hadn't noticed the common Interface. I like that idea of being able to catch exceptions thrown by this package.

As a note, I think we should update the AGENTS.md to include a note that all exceptions should use our own, including primitives, and if a primitive doesn't exist it should be created. Not sure if we should add this elsewhere, too. If we're going down this road it's important that we're consistent.

JasonTheAdams

I'm tracking now, @Ref34t! Good work!

I don't think we need all these static methods. As far as I can tell, there all used only one time. If that's the case, the abstraction isn't necessary and we can just let the implementing code determine the code, message, and such.

src/Common/Exception/InvalidArgumentException.php

felixarntz

@Ref34t @JasonTheAdams I do think some of the static methods are useful - just not all of them.

Per my previous feedback, @Ref34t it would be great if you could review the codebase for where exceptions are thrown and update these exceptions to use the new exception classes and new static methods if applicable.

We should not introduce any exception classes or any static methods that aren't used anywhere - I think that's a good guiding principle. Therefore it's important that we update any existing exceptions to use the new approach, and if there are none for a specific exception or static method, we know it's not useful, at least not at this point.

Ref34t · 2025-09-04T15:47:31Z

very interesting Convo Champs @felixarntz @JasonTheAdams 🥇
I'll work it this today and bring the best from it based on my idea and your thoughts

…and PromptBuilder now use project-scoped InvalidArgumentException and RuntimeException for better error categorization and unified exception handling

…hp and HTTP Request/Response DTOs now use project-scoped exception classes for unified error handling

…romBadResponse() based on real usage in ResponseUtil, remove unused static methods keeping only those actually used in codebase

Ref34t · 2025-09-05T16:34:36Z

Updated @SInCE tags from "0.2.0" to "n.e.x.t" in all exception classes
Migrated 30+ throw statements across 5 core files to use custom exception classes:
- ProviderRegistry.php: 5 exceptions converted
- PromptBuilder.php: 25+ exceptions converted
- Files/DTO/File.php: 5 exceptions converted
- Http/DTO/Request.php and Response.php: exceptions converted
Added ResponseException::fromBadResponse() static method based on actual usage
Simplified ResponseUtil from 30 lines to 1 line using new static method
Removed 9 unused static methods across NetworkException, RequestException, and ResponseException
All exceptions now use project's custom classes instead of PHP built-ins
Maintained AiClientExceptionInterface implementation for unified error handling
Added exception handling requirements to AGENTS.md coding standards

Ref34t · 2025-09-05T16:58:19Z

The remaining files that were updated

src/Providers/Http/Traits/WithHttpTransporterTrait.php
src/Providers/Http/Traits/WithRequestAuthenticationTrait.php
src/ProviderImplementations/OpenAi/OpenAiProvider.php
src/ProviderImplementations/Google/GoogleProvider.php
src/ProviderImplementations/Google/GoogleModelMetadataDirectory.php
src/ProviderImplementations/Anthropic/AnthropicProvider.php
src/ProviderImplementations/Anthropic/AnthropicModelMetadataDirectory.php
src/Providers/OpenAiCompatibleImplementation/AbstractOpenAiCompatibleTextGenerationModel.php
src/Providers/OpenAiCompatibleImplementation/AbstractOpenAiCompatibleImageGenerationModel.php
src/Providers/Models/DTO/ModelRequirements.php
src/Providers/Models/DTO/ModelMetadata.php
src/Providers/Models/DTO/ModelConfig.php
src/Providers/Models/DTO/SupportedOption.php
src/Providers/DTO/ProviderModelsMetadata.php
src/Providers/Contracts/ProviderOperationsHandlerInterface.php
src/Providers/Contracts/ProviderInterface.php
src/Providers/Contracts/ModelMetadataDirectoryInterface.php
src/Providers/ApiBasedImplementation/AbstractApiBasedModelMetadataDirectory.php
src/Tools/DTO/FunctionCall.php
src/Results/DTO/Candidate.php
src/Messages/DTO/MessagePart.php
src/Common/AbstractEnum.php
src/Common/AbstractDataTransferObject.php

JasonTheAdams

Awesome! Thanks for updating all those, @Ref34t! I left a couple comments and questions.

src/Files/DTO/File.php

JasonTheAdams · 2025-09-08T17:56:24Z

src/Providers/Http/Exception/NetworkException.php

+ *
+ * @since n.e.x.t
+ */
+class NetworkException extends RuntimeException


I wonder if this should implement \Psr\Http\Client\NetworkExceptionInterface?

I'll accept this suggestion! Implemented all three PSR-18 interfaces with getRequest() methods and updated HttpTransporter accordingly

resolved in 04e767f

JasonTheAdams · 2025-09-08T17:58:13Z

src/Providers/Http/Exception/RequestException.php

+ *
+ * @since n.e.x.t
+ */
+class RequestException extends InvalidArgumentException


Same, should implement \Psr\Http\Client\RequestExceptionInterface?

resolved 04e767f

JasonTheAdams · 2025-09-08T17:59:34Z

src/Providers/Http/Exception/ResponseException.php

 * @since 0.1.0
 */
-class ResponseException extends Exception
+class ResponseException extends RuntimeException


Similar, should this implement the PSR-18 ClientExceptionInterface?

resolved 04e767f

AGENTS.md

…nsions

felixarntz

@Ref34t This looks really close now. A few last points of feedback.

felixarntz · 2025-09-10T21:13:51Z

src/Exceptions/AiClientExceptionInterface.php

@@ -0,0 +1,19 @@
+<?php


This file should probably live in Common\Contracts, or in Common\Exception\Contracts - not in its own separate top-level namespace.

resolved here a6bbaf2

felixarntz · 2025-09-10T21:16:05Z

src/ProviderImplementations/OpenAi/OpenAiModelMetadataDirectory.php

-            throw new RuntimeException(
-                'Unexpected API response: Missing the data key.'
-            );
+            throw ResponseException::fromMissingData('OpenAI', 'data');


The same change still needs to be made in the AnthropicModelMetadataDirectory and in GoogleModelMetadataDirectory.

resolved here 7c742aa

felixarntz · 2025-09-10T21:21:42Z

src/Providers/Http/HttpTransporter.php

+        // Check for 400 Bad Request responses indicating invalid request data
+        if ($psr7Response->getStatusCode() === 400) {
+            $body = (string) $psr7Response->getBody();
+            $errorDetail = $body ? substr($body, 0, 200) : 'Invalid request parameters';
+            throw RequestException::fromBadRequest($psr7Request, $errorDetail);
+        }


This should not be handled here, because we want to make sure the caller of HttpTransporter::send is in full control over what to do with the response.

Therefore, this should be moved to ResponseUtil::throwIfNotSuccessful, so the caller can decide to use it, or not.

felixarntz · 2025-09-10T21:23:56Z

src/Providers/Http/Util/ResponseUtil.php

-        }
-
-        throw new ResponseException($errorMessage, $response->getStatusCode());
+        throw ResponseException::fromBadResponse($response);


I'm thinking that we may be want to move the fromBadResponse method from ResponseException to RequestException? Technically, the lines are blurry, but I think I have a good reason:

The only thing I personally consider a response exception is when something in the response itself is wrong (e.g. missing an expected field in a successful response), which should almost never happen. Everything else IMO is more suitable in RequestException - because if the response has an error status code, we can consider the request failed, not the response.

Would love to get your perspective on this too @JasonTheAdams!

So in frameworks like Symfony they distinguish this a bit differently, they use:

HttpExceptionInterface, which indicates a 4xx or 5xx response error

ClientExceptionInterface, which extends HttpExceptionInterface and is specific to 4xx response errors

ServerExceptionInterface, which extends HttpExceptionInterface and is specific to 5xx response errors

TransportExceptionInterface, which means there was a network failure

So they don't actually distinguish it in terms of "response" and "request", I think because of the ambiguity being described here. So if we like that convention better we could go that route.

I'm not opposed to doing it that way, however none of the exceptions you mention actually does what our ResponseException here is currently covering: It's the (very likely only hypothetical) scenario where a response does not include the data we expect. This can only happen if e.g. OpenAI made a breaking change to its API response shape tomorrow and we didn't know about it. So very unlikely to ever run into it, but possible - however it's only relevant if the API response itself came back successfully.

I see your point. I'm good with RequestException, or if we want something a touch more granular: UnsupportedRequestException.

Ok! @felixarntz and I chatted a bit and landed on this suggestion:

RequestException - base class and thrown within Providers

RedirectException extends RequestException - thrown for 3xx responses

ClientException extends RequestException - thrown for 4xx responses

ServerException extends RequestException - thrown for 5xx responses

NetworkException extends RequestException - thrown for network errors

Sound good to you, @Ref34t? 😄

I like the final outcome you decided upon. Will try to conclude it 👍

felixarntz · 2025-09-10T21:27:39Z

@Ref34t Note the merge conflict with ModelRequirements, it would be great if you could please fix that too now that #69 was merged.

felixarntz · 2025-09-10T21:33:25Z

src/Providers/Http/Exception/ResponseException.php

+     * @param string $context Additional context about where the field was expected.
+     * @return self
+     */
+    public static function fromMissingData(string $apiName, string $fieldName, string $context = ''): self


There are several other places where this should be used:

All exceptions thrown in AbstractOpenAiCompatibleTextGenerationModel::parseResponseToGenerativeAiResult

All exceptions thrown in AbstractOpenAiCompatibleTextGenerationModel::parseResponseChoiceToCandidate

All exceptions thrown in AbstractOpenAiCompatibleTextGenerationModel::parseResponseChoiceMessageParts

All exceptions thrown in AbstractOpenAiCompatibleImageGenerationModel::parseResponseToGenerativeAiResult

All exceptions thrown in AbstractOpenAiCompatibleImageGenerationModel::parseResponseChoiceToCandidate

For some of these where it's about something more specific than a missing field, we could have another public static function fromInvalidData(string $apiName, string $message): self here.

…erarchy

…eption

…rException

…rrors

Ref34t marked this pull request as ready for review September 1, 2025 13:28

felixarntz added the [Type] Enhancement A suggestion for improvement. label Sep 1, 2025

felixarntz reviewed Sep 1, 2025

View reviewed changes

src/Exceptions/NetworkException.php Outdated Show resolved Hide resolved

src/Exceptions/RequestException.php Outdated Show resolved Hide resolved

src/Providers/Http/Exception/ResponseException.php Outdated Show resolved Hide resolved

felixarntz mentioned this pull request Sep 2, 2025

Enhancement: Improve Error Handling Developer Experience #60

Closed

JasonTheAdams self-requested a review September 2, 2025 14:50

JasonTheAdams reviewed Sep 2, 2025

View reviewed changes

JasonTheAdams mentioned this pull request Sep 3, 2025

Development roadmap #8

Open

20 tasks

JasonTheAdams requested changes Sep 3, 2025

View reviewed changes

src/Common/Exception/InvalidArgumentException.php Outdated Show resolved Hide resolved

felixarntz reviewed Sep 3, 2025

View reviewed changes

Mohamed Khaled added 12 commits September 5, 2025 19:07

Implement simplified exception hierarchy for better error handling

19b0b50

Fix code style violations

f1b23d3

Implement exception hierarchy reorganization with static factory methods

a22c94e

Fix code style violations in exception classes

98a84cf

Fix line length violations in exception factory methods

28ce137

Fix function declaration brace placement

227498b

Update @SInCE tags from 0.2.0 to n.e.x.t

eb9686d

Update core files to use custom exception classes - ProviderRegistry …

f158e71

…and PromptBuilder now use project-scoped InvalidArgumentException and RuntimeException for better error categorization and unified exception handling

Update HTTP and File DTOs to use custom exceptions - Files/DTO/File.p…

9eac85b

…hp and HTTP Request/Response DTOs now use project-scoped exception classes for unified error handling

Implement usage-driven exception hierarchy - Add ResponseException::f…

1f2fc0e

…romBadResponse() based on real usage in ResponseUtil, remove unused static methods keeping only those actually used in codebase

Add exception handling requirements to coding standards

c1aede5

Implement usage-driven exception hierarchy with custom classes

f901fce

Ref34t force-pushed the feature/exception-hierarchy branch from 722138b to f901fce Compare September 5, 2025 16:15

Fix File exception handling to use specific MimeType exceptions

048d889

Complete exception hierarchy implementation across entire codebase

499b5ab

Ref34t requested review from felixarntz and JasonTheAdams September 5, 2025 17:02

JasonTheAdams requested changes Sep 8, 2025

View reviewed changes

Mohamed Khaled added 5 commits September 10, 2025 17:56

Fix accidentally removed try-catch for unrecognized file extensions

469f46e

Update test to expect correct fallback error message for unknown exte…

c0d3b74

…nsions

Implement PSR-18 exception interfaces for HTTP client compliance

04e767f

Generalize exception handling guidelines in AGENTS.md

d2f5d62

Fix code style violations in HTTP exception classes

9330edd

Ref34t requested a review from JasonTheAdams September 10, 2025 15:22

JasonTheAdams added this to the 0.2.0 milestone Sep 10, 2025

felixarntz mentioned this pull request Sep 10, 2025

Adds HTTP Client Adapter WordPress/wp-ai-client#2

Open

felixarntz reviewed Sep 10, 2025

View reviewed changes

Mohamed Khaled added 9 commits September 11, 2025 17:20

Merge remote-tracking branch 'origin/trunk' into feature/exception-hi…

34fec79

…erarchy

Restore missing ModelRequirements methods from trunk

5fe7df9

Fix code style: add missing newline at end of file

23ceb28

Implement exception hierarchy improvements

a6bbaf2

Update Anthropic and Google ModelMetadataDirectory to use ResponseExc…

7c742aa

…eption

Implement ClientException for 4xx responses in ResponseUtil

4a69461

Implement complete exception hierarchy with ClientException and Serve…

8ab52c3

…rException

Update tests to expect new exception hierarchy

beb17e1

Enhance ResponseException with semantic factory methods for parsing e…

44c8a4f

…rrors

Implement Exception Hierarchy For Better Error Handling #78

Are you sure you want to change the base?

Implement Exception Hierarchy For Better Error Handling #78

Conversation

Ref34t commented Sep 1, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Ref34t commented Sep 1, 2025

Uh oh!

felixarntz left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

JasonTheAdams commented Sep 2, 2025

Uh oh!

github-actions bot commented Sep 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Unlinked Accounts

Uh oh!

Ref34t commented Sep 2, 2025

Uh oh!

JasonTheAdams left a comment

Choose a reason for hiding this comment

Uh oh!

felixarntz commented Sep 2, 2025

Uh oh!

JasonTheAdams commented Sep 3, 2025

Uh oh!

JasonTheAdams left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

felixarntz left a comment

Choose a reason for hiding this comment

Uh oh!

Ref34t commented Sep 4, 2025

Uh oh!

Ref34t commented Sep 5, 2025

Uh oh!

Ref34t commented Sep 5, 2025

Uh oh!

JasonTheAdams 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!

Choose a reason for hiding this comment

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!

felixarntz Sep 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

JasonTheAdams Sep 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Ref34t commented Sep 1, 2025 •

edited

Loading

github-actions bot commented Sep 2, 2025 •

edited

Loading

felixarntz Sep 10, 2025 •

edited

Loading

JasonTheAdams Sep 10, 2025 •

edited

Loading

JasonTheAdams Sep 10, 2025 •

edited

Loading