Skip to content

Enhance exception API for response exceptions from invalid values by including concrete field name #102

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Oct 8, 2025
Merged

Enhance exception API for response exceptions from invalid values by including concrete field name #102

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 3 additions & 2 deletions src/Providers/Http/Exception/ResponseException.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,11 +39,12 @@ public static function fromMissingData(string $apiName, string $fieldName): self
* @since n.e.x.t
*
* @param string $apiName The name of the API service (e.g., 'OpenAI', 'Anthropic').
* @param string $fieldName The field that was invalid.
* @param string $message The specific error message describing the invalid data.
* @return self
*/
public static function fromInvalidData(string $apiName, string $message): self
public static function fromInvalidData(string $apiName, string $fieldName, string $message): self
{
return new self(sprintf('Unexpected %s API response: %s', $apiName, $message));
return new self(sprintf('Unexpected %s API response: Invalid "%s" key: %s', $apiName, $fieldName, $message));
}
}
15 changes: 10 additions & 5 deletions ...Providers/OpenAiCompatibleImplementation/AbstractOpenAiCompatibleImageGenerationModel.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -302,20 +302,22 @@ protected function parseResponseToGenerativeAiResult(
if (!is_array($responseData['data'])) {
throw ResponseException::fromInvalidData(
$this->providerMetadata()->getName(),
'The data key must contain an array.'
'data',
'The value must be an array.'
);
}

$candidates = [];
foreach ($responseData['data'] as $choiceData) {
foreach ($responseData['data'] as $index => $choiceData) {
if (!is_array($choiceData) || array_is_list($choiceData)) {
throw ResponseException::fromInvalidData(
$this->providerMetadata()->getName(),
'Each element in the data key must be an associative array.'
"data[{$index}]",
'The value must be an associative array.'
);
}

$candidates[] = $this->parseResponseChoiceToCandidate($choiceData, $expectedMimeType);
$candidates[] = $this->parseResponseChoiceToCandidate($choiceData, $index, $expectedMimeType);
}

$id = isset($responseData['id']) && is_string($responseData['id']) ? $responseData['id'] : '';
Expand Down Expand Up @@ -352,12 +354,14 @@ protected function parseResponseToGenerativeAiResult(
* @since 0.1.0
*
* @param ChoiceData $choiceData The choice data from the API response.
* @param int $index The index of the choice in the choices array.
* @param string $expectedMimeType The expected MIME type the response is in.
* @return Candidate The parsed candidate.
* @throws RuntimeException If the choice data is invalid.
*/
protected function parseResponseChoiceToCandidate(
array $choiceData,
int $index,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not crazy about requiring this parameter that's strictly used for the exception. This method otherwise isn't coupled to the concept that there's an index somehow associated with the choice. If we feel the index is really important (and the $choiceData isn't sufficient), I'd rather throw the exception where the loop is happening — even if it means catching this exception in the loop and throwing a new one.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's totally fair. I'll take another look later to see how this could be restructured.

Copy link
Member Author

@felixarntz felixarntz Oct 3, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@JasonTheAdams Looking into this, I'm not sure what's the best alternative - everything ends up being hacky somehow.

You could do something where the inner methods (without awareness of $index) simply pass the innermost field name (e.g. message instead of choices[{$index}].message), and then the outer method catches it and runs a regular expression on $e->getMessage() to replace "([A-Za-z0-9_]+)" with "choices[{$index}].$1", but that's hacky because it is based on the specific message shape of the ResponseException class. - I don't love it.

Other ideas I thought about is to store the API name and field name as separate properties in ResponseException so you can read them from the caught instance, but then you also still need to somehow extract the part of the message that's dynamic and doesn't just come from the API name and field name. Not a great solution either.

Let me know if you have other ideas.

Otherwise, I think the current approach of passing something to the inner methods is the most straightforward. An idea that might alleviate your concern would be that we don't just pass the index, but the full "prefix" identifier of the current context that is being parsed, e.g. instead of $index (integer) we would pass choices[{$index}] (string). This not only clarifies the purpose of why this is passed, it also avoids the problem of the inner methods having some awareness of their context (e.g. right now the methods must know that the outer field is called choices, which is not ideal) - by passing the context down in its entirety the responsibility remains entirely with the outer method, and as such, in a single place within the class.

WDYT?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, I'm starting to back down on this one. I admit I didn't notice the first time that this is a protected method (and could maybe even be private). If this is just an internal method, then I don't think passing the index is a big deal. It feels a little odd since it's just used for exceptions, but that would be more strange if it was public.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

OK, sounds good. I think it still needs to be protected so that custom tools in e.g. a specific provider can be supported (by overriding some of these methods in a child class), but yeah the risk should be rather low. Also as long as we're in such an early version, for such low-level semi-internal APIs we can still make breaking changes if we later find there's a problem.

LMK if this works for you.

string $expectedMimeType = 'image/png'
): Candidate {
if (isset($choiceData['url']) && is_string($choiceData['url'])) {
Expand All @@ -367,7 +371,8 @@ protected function parseResponseChoiceToCandidate(
} else {
throw ResponseException::fromInvalidData(
$this->providerMetadata()->getName(),
'Each choice must contain either a url or b64_json key with a string value.'
"choices[{$index}]",
'The value must contain either a url or b64_json key with a string value.'
);
}

Expand Down
20 changes: 13 additions & 7 deletions src/Providers/OpenAiCompatibleImplementation/AbstractOpenAiCompatibleTextGenerationModel.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -565,7 +565,8 @@ protected function parseResponseToGenerativeAiResult(Response $response): Genera
if (!is_array($responseData['choices'])) {
throw ResponseException::fromInvalidData(
$this->providerMetadata()->getName(),
'The choices key must contain an array.'
'choices',
'The value must be an array.'
);
}

Expand All @@ -574,7 +575,8 @@ protected function parseResponseToGenerativeAiResult(Response $response): Genera
if (!is_array($choiceData) || array_is_list($choiceData)) {
throw ResponseException::fromInvalidData(
$this->providerMetadata()->getName(),
'Each element in the choices key must be an associative array.'
"choices[{$index}]",
'The value must be an associative array.'
);
}

Expand Down Expand Up @@ -640,7 +642,7 @@ protected function parseResponseChoiceToCandidate(array $choiceData, int $index)
}

$messageData = $choiceData['message'];
$message = $this->parseResponseChoiceMessage($messageData);
$message = $this->parseResponseChoiceMessage($messageData, $index);

switch ($choiceData['finish_reason']) {
case 'stop':
Expand All @@ -658,6 +660,7 @@ protected function parseResponseChoiceToCandidate(array $choiceData, int $index)
default:
throw ResponseException::fromInvalidData(
$this->providerMetadata()->getName(),
"choices[{$index}].finish_reason",
sprintf('Invalid finish reason "%s".', $choiceData['finish_reason'])
);
}
Expand All @@ -671,15 +674,16 @@ protected function parseResponseChoiceToCandidate(array $choiceData, int $index)
* @since 0.1.0
*
* @param MessageData $messageData The message data from the API response.
* @param int $index The index of the choice in the choices array.
* @return Message The parsed message.
*/
protected function parseResponseChoiceMessage(array $messageData): Message
protected function parseResponseChoiceMessage(array $messageData, int $index): Message
{
$role = isset($messageData['role']) && 'user' === $messageData['role']
? MessageRoleEnum::user()
: MessageRoleEnum::model();

$parts = $this->parseResponseChoiceMessageParts($messageData);
$parts = $this->parseResponseChoiceMessageParts($messageData, $index);

return new Message($role, $parts);
}
Expand All @@ -690,9 +694,10 @@ protected function parseResponseChoiceMessage(array $messageData): Message
* @since 0.1.0
*
* @param MessageData $messageData The message data from the API response.
* @param int $index The index of the choice in the choices array.
* @return MessagePart[] The parsed message parts.
*/
protected function parseResponseChoiceMessageParts(array $messageData): array
protected function parseResponseChoiceMessageParts(array $messageData, int $index): array
{
$parts = [];

Expand All @@ -705,11 +710,12 @@ protected function parseResponseChoiceMessageParts(array $messageData): array
}

if (isset($messageData['tool_calls']) && is_array($messageData['tool_calls'])) {
foreach ($messageData['tool_calls'] as $toolCallData) {
foreach ($messageData['tool_calls'] as $toolCallIndex => $toolCallData) {
$toolCallPart = $this->parseResponseChoiceMessageToolCallPart($toolCallData);
if (!$toolCallPart) {
throw ResponseException::fromInvalidData(
$this->providerMetadata()->getName(),
"choices[{$index}].message.tool_calls[{$toolCallIndex}]",
'The response includes a tool call of an unexpected type.'
);
}
Expand Down
4 changes: 3 additions & 1 deletion tests/mocks/MockOpenAiCompatibleImageGenerationModel.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -91,13 +91,15 @@ public function exposeParseResponseToGenerativeAiResult(
* Exposes the protected parseResponseChoiceToCandidate method.
*
* @param array<string, mixed> $choiceData
* @param int $index
* @param string $expectedMimeType
* @return \WordPress\AiClient\Results\DTO\Candidate
*/
public function exposeParseResponseChoiceToCandidate(
array $choiceData,
int $index,
string $expectedMimeType = 'image/png'
): \WordPress\AiClient\Results\DTO\Candidate {
return $this->parseResponseChoiceToCandidate($choiceData, $expectedMimeType);
return $this->parseResponseChoiceToCandidate($choiceData, $index, $expectedMimeType);
}
}
16 changes: 9 additions & 7 deletions ...iders/OpenAiCompatibleImplementation/AbstractOpenAiCompatibleImageGenerationModelTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -741,7 +741,9 @@ public function testParseResponseToGenerativeAiResultInvalidDataType(): void
$model = $this->createModel();

$this->expectException(ResponseException::class);
$this->expectExceptionMessage('Unexpected TestProvider API response: The data key must contain an array.');
$this->expectExceptionMessage(
'Unexpected TestProvider API response: Invalid "data" key: The value must be an array.'
);

$model->exposeParseResponseToGenerativeAiResult($response);
}
Expand All @@ -758,7 +760,7 @@ public function testParseResponseToGenerativeAiResultInvalidChoiceElementType():

$this->expectException(ResponseException::class);
$this->expectExceptionMessage(
'Unexpected TestProvider API response: Each element in the data key must be an associative array.'
'Unexpected TestProvider API response: Invalid "data[0]" key: The value must be an associative array.'
);

$model->exposeParseResponseToGenerativeAiResult($response);
Expand All @@ -775,7 +777,7 @@ public function testParseResponseChoiceToCandidateValidUrlData(): void
'url' => 'https://example.com/image.png',
];
$model = $this->createModel();
$candidate = $model->exposeParseResponseChoiceToCandidate($choiceData, 'image/png');
$candidate = $model->exposeParseResponseChoiceToCandidate($choiceData, 0, 'image/png');

$this->assertInstanceOf(Candidate::class, $candidate);
$this->assertEquals(
Expand All @@ -799,7 +801,7 @@ public function testParseResponseChoiceToCandidateValidB64JsonData(): void
'b64_json' => $base64Image,
];
$model = $this->createModel();
$candidate = $model->exposeParseResponseChoiceToCandidate($choiceData, 'image/png');
$candidate = $model->exposeParseResponseChoiceToCandidate($choiceData, 0, 'image/png');

$this->assertInstanceOf(Candidate::class, $candidate);
$this->assertEquals($base64Image, $candidate->getMessage()->getParts()[0]->getFile()->getBase64Data());
Expand All @@ -822,10 +824,10 @@ public function testParseResponseChoiceToCandidateMissingUrlOrB64Json(): void

$this->expectException(ResponseException::class);
$this->expectExceptionMessage(
'Unexpected TestProvider API response: Each choice must contain either a url or b64_json key with a ' .
'string value.'
'Unexpected TestProvider API response: Invalid "choices[0]" key: The value must contain either a ' .
'url or b64_json key with a string value.'
);

$model->exposeParseResponseChoiceToCandidate($choiceData);
$model->exposeParseResponseChoiceToCandidate($choiceData, 0);
}
}
21 changes: 14 additions & 7 deletions ...viders/OpenAiCompatibleImplementation/AbstractOpenAiCompatibleTextGenerationModelTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -972,7 +972,9 @@ public function testParseResponseToGenerativeAiResultInvalidChoicesType(): void
$model = $this->createModel();

$this->expectException(ResponseException::class);
$this->expectExceptionMessage('Unexpected TestProvider API response: The choices key must contain an array.');
$this->expectExceptionMessage(
'Unexpected TestProvider API response: Invalid "choices" key: The value must be an array.'
);

$model->parseResponseToGenerativeAiResult($response);
}
Expand All @@ -989,7 +991,7 @@ public function testParseResponseToGenerativeAiResultInvalidChoiceElementType():

$this->expectException(ResponseException::class);
$this->expectExceptionMessage(
'Unexpected TestProvider API response: Each element in the choices key must be an associative array.'
'Unexpected TestProvider API response: Invalid "choices[0]" key: The value must be an associative array.'
);

$model->parseResponseToGenerativeAiResult($response);
Expand Down Expand Up @@ -1098,7 +1100,12 @@ public function testParseResponseChoiceToCandidateInvalidFinishReason(): void
$model = $this->createModel();

$this->expectException(ResponseException::class);
$this->expectExceptionMessage('Unexpected TestProvider API response: Invalid finish reason "unknown".');
$this->expectExceptionMessage(
sprintf(
'Unexpected TestProvider API response: Invalid "%s" key: Invalid finish reason "unknown".',
'choices[0].finish_reason'
)
);

$model->exposeParseResponseChoiceToCandidate($choiceData);
}
Expand All @@ -1115,7 +1122,7 @@ public function testParseResponseChoiceMessageAssistant(): void
'content' => 'Assistant response',
];
$model = $this->createModel();
$message = $model->exposeParseResponseChoiceMessage($messageData);
$message = $model->exposeParseResponseChoiceMessage($messageData, 0);

$this->assertEquals(MessageRoleEnum::model(), $message->getRole());
$this->assertCount(1, $message->getParts());
Expand All @@ -1134,7 +1141,7 @@ public function testParseResponseChoiceMessageUser(): void
'content' => 'User response',
];
$model = $this->createModel();
$message = $model->exposeParseResponseChoiceMessage($messageData);
$message = $model->exposeParseResponseChoiceMessage($messageData, 0);

$this->assertEquals(MessageRoleEnum::user(), $message->getRole());
$this->assertCount(1, $message->getParts());
Expand All @@ -1153,7 +1160,7 @@ public function testParseResponseChoiceMessagePartsContentAndReasoning(): void
'content' => 'Final answer',
];
$model = $this->createModel();
$parts = $model->exposeParseResponseChoiceMessageParts($messageData);
$parts = $model->exposeParseResponseChoiceMessageParts($messageData, 0);

$this->assertCount(2, $parts);
$this->assertEquals('Thinking process', $parts[0]->getText());
Expand Down Expand Up @@ -1182,7 +1189,7 @@ public function testParseResponseChoiceMessagePartsToolCalls(): void
],
];
$model = $this->createModel();
$parts = $model->exposeParseResponseChoiceMessageParts($messageData);
$parts = $model->exposeParseResponseChoiceMessageParts($messageData, 0);

$this->assertCount(1, $parts);
$this->assertInstanceOf(FunctionCall::class, $parts[0]->getFunctionCall());
Expand Down
8 changes: 4 additions & 4 deletions ...unit/Providers/OpenAiCompatibleImplementation/MockOpenAiCompatibleTextGenerationModel.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -163,14 +163,14 @@ public function exposeParseResponseChoiceToCandidate(array $choiceData, int $ind
return $this->parseResponseChoiceToCandidate($choiceData, $index);
}

public function exposeParseResponseChoiceMessage(array $messageData): Message
public function exposeParseResponseChoiceMessage(array $messageData, int $index = 0): Message
{
return $this->parseResponseChoiceMessage($messageData);
return $this->parseResponseChoiceMessage($messageData, $index);
}

public function exposeParseResponseChoiceMessageParts(array $messageData): array
public function exposeParseResponseChoiceMessageParts(array $messageData, int $index = 0): array
{
return $this->parseResponseChoiceMessageParts($messageData);
return $this->parseResponseChoiceMessageParts($messageData, $index);
}

public function exposeParseResponseChoiceMessageToolCallPart(array $toolCallData): ?MessagePart
Expand Down
Loading