Skip to content

Fix #692 [Bug]: Passing invalid parameters to gemini throws Undefined… - #693

Open
alxlab-zone66x wants to merge 3 commits into
openai-php:mainfrom
alxlab-zone66x:issue-692
Open

Fix #692 [Bug]: Passing invalid parameters to gemini throws Undefined…#693
alxlab-zone66x wants to merge 3 commits into
openai-php:mainfrom
alxlab-zone66x:issue-692

Conversation

@alxlab-zone66x

@alxlab-zone66x alxlab-zone66x commented Oct 2, 2025

Copy link
Copy Markdown

… array key "choices" /www/htdocs/vendor/openai-php/client/src/Responses/Chat/CreateResponse.php 54

Change-Id: Ibbf64cc554ef68844fd8338583f3fc8ec5fde413

What:

  • Bug Fix

Description:

Related:

Fix issue #692

@ammarcoux
Fix openai-php#692 [Bug]: Passing invalid parameters to gemini throws…
273cc37 
… Undefined array key "choices" /www/htdocs/vendor/openai-php/client/src/Responses/Chat/CreateResponse.php 54

Change-Id: Ibbf64cc554ef68844fd8338583f3fc8ec5fde413
iBotPeaches
iBotPeaches reviewed Oct 2, 2025
View reviewed changes

@iBotPeaches iBotPeaches left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Could you add a test to confirm this? I think the HttpTransporter tests is the area to make a formulated Gemini exception to confirm this.

@alxlab-zone66x

alxlab-zone66x commented Oct 2, 2025

Copy link
Copy Markdown
Author

Yeah sure. That would make sense.

@alxlab-zone66x

alxlab-zone66x commented Oct 3, 2025

Copy link
Copy Markdown
Author

Hmm for the ErrorException type what do you thing would be best? Google returns:

[[
            'error' => [
                'code' => 400,
                'message' => 'Invalid JSON payload received. Unknown name "ddd": Cannot find field.',
                'status' => 'INVALID_ARGUMENT',
                'details' => [
                    [
                        '@type' => 'type.googleapis.com/google.rpc.BadRequest',
                        'fieldViolations' => [
                            [
                                'description' => 'Invalid JSON payload received. Unknown name "ddd": Cannot find field.',
                            ],
                        ],
                    ],
                ],
            ],
        ]]

type.googleapis.com/google.rpc.BadRequest would be more in line with OpenAI's type invalid_request_error, but INVALID_ARGUMENT is more informative imho.

@iBotPeaches

iBotPeaches commented Oct 3, 2025

Copy link
Copy Markdown
Collaborator

type.googleapis.com/google.rpc.BadRequest would be more in line with OpenAI's type invalid_request_error, but INVALID_ARGUMENT is more informative imho.

Probably the message/status of the higher object. I think you can ignore inner details stuff.

@ammarcoux
Fix openai-php#692 [Bug]: Passing invalid parameters to gemini throws…
6591d36 
… Undefined array key "choices" /www/htdocs/vendor/openai-php/client/src/Responses/Chat/CreateResponse.php 54

Add tests for gemini with invalid parameter.
Set ErrorException type to return error status for invalid gemini parameter.

Change-Id: I076016d0470eb3060d3f972702e4e3701b52207d
iBotPeaches
iBotPeaches reviewed Oct 9, 2025
View reviewed changes
Comment thread src/Transporters/HttpTransporter.php
Comment on lines +181 to +183
if (! isset($data[0]['error']['type']) && isset($data[0]['error']['status'])) {
$data[0]['error']['type'] = $data[0]['error']['status'];
}

@iBotPeaches iBotPeaches Oct 9, 2025

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

This check is the only thing I'm iffy on right now. I'd rather the ErrorException class navigate the water of different error structures to normalize them. So the HTTPTransporter isn't required to formulate things perfectly for an exception class.

@iBotPeaches iBotPeaches mentioned this pull request Nov 28, 2025
Closed
1 of 2 tasks
@krowinski

krowinski commented Dec 16, 2025

Copy link
Copy Markdown

Hi! Any update on this? Maybe Google should fix it ? I suppose every SDK has issues with error payloads ?

@iBotPeaches

iBotPeaches commented Dec 16, 2025

Copy link
Copy Markdown
Collaborator

I wish Google would fix it. If you are making a parity API for OpenAI, at least make it match. My last comment is still outstanding.

IamYashKapoor pushed a commit to IamYashKapoor/client that referenced this pull request Nov 6, 2026
Yash Kapoor @alxlab-zone66x
fix(OpenAI): fall back to the error status when the payload has no type
ec71cad 
Gemini's OpenAI-compatible endpoints name the kind of error `status`
(`INVALID_ARGUMENT`), not `type`, so `ErrorException::getErrorType()`
returned null for every error they raise. `type` keeps precedence when
a payload carries both.

Normalize it in the exception rather than in the transporter, as asked
on openai-php#693, so both construction sites are covered: the whole-response one
in HttpTransporter and the SSE frame one in StreamResponse. Widen the
error payload shapes at those two sites, which declared `type` as
required and knew nothing of `status`.

Co-authored-by: alxlab-zone66x <85719219+alxlab-zone66x@users.noreply.github.com>
IamYashKapoor pushed a commit to IamYashKapoor/client that referenced this pull request Nov 6, 2026
Yash Kapoor @alxlab-zone66x
fix(OpenAI): fall back to the error status when the payload has no type
49be636 
Gemini's OpenAI-compatible endpoints name the kind of error `status`
(`INVALID_ARGUMENT`), not `type`, so `ErrorException::getErrorType()`
returned null for every error they raise. `type` keeps precedence when
a payload carries both, and a `status` that is not a string is ignored:
plenty of providers put the HTTP status there as an integer, and this
getter returns `?string`.

Normalize it in the exception rather than in the transporter, as asked
on openai-php#693, so both construction sites are covered: the whole-response one
in HttpTransporter and the SSE frame one in StreamResponse. Widen the
error payload shapes at those two sites, which declared `type` as
required and knew nothing of `status`.

Co-authored-by: alxlab-zone66x <85719219+alxlab-zone66x@users.noreply.github.com>
IamYashKapoor pushed a commit to IamYashKapoor/client that referenced this pull request Nov 6, 2026
Yash Kapoor @alxlab-zone66x
fix(OpenAI): fall back to the error status when the payload has no type
bce7e6a 
Gemini's OpenAI-compatible endpoints name the kind of error `status`
(`INVALID_ARGUMENT`), not `type`, so `ErrorException::getErrorType()`
returned null for every error they raise. `type` keeps precedence when
a payload carries both, and a `status` that is not a string is ignored:
plenty of providers put the HTTP status there as an integer, and this
getter returns `?string`.

Normalize it in the exception rather than in the transporter, as asked
on openai-php#693, so both construction sites are covered: the whole-response one
in HttpTransporter and the SSE frame one in StreamResponse. Widen the
error payload shapes at those two sites, which declared `type` as
required and knew nothing of `status`.

Co-authored-by: alxlab-zone66x <85719219+alxlab-zone66x@users.noreply.github.com>
@IamYashKapoor IamYashKapoor mentioned this pull request Nov 6, 2026
Open
1 of 2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@iBotPeaches iBotPeaches iBotPeaches left review comments
+1 more reviewer
@prince-alipclinton prince-alipclinton prince-alipclinton approved these changes
Reviewers whose approvals may not affect merge requirements

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

5 participants

@alxlab-zone66x @iBotPeaches @krowinski @prince-alipclinton @ammarcoux