weiqingy opened a new pull request, #989:
URL: https://github.com/apache/flink-agents/pull/989
Linked issue: #936
### Purpose of change
Case 3 of #936. The three OpenAI-family connections caught every exception
from a chat call and rethrew it as a generic `RuntimeException`, while the
Python connections let the SDK exception propagate unchanged. A caller writing
against both languages saw different failure types for the same provider
condition.
The SDK exceptions already carry the HTTP status and the provider's error
payload, so the wrapper only forced callers to inspect a cause. This removes it
from `OpenAICompletionsConnection`, `AzureOpenAIChatModelConnection` and
`OpenAIResponsesModelConnection`, which aligns Java with Python. Local
validation failures continue to raise `IllegalArgumentException`.
There is a second effect worth calling out. Each `try` also covered request
building, so a local validation failure was rewrapped too. A tool message
missing its `externalId` reached the caller as `RuntimeException("Failed to
call OpenAI chat completions API.")` rather than as the
`IllegalArgumentException` that `OpenAIChatCompletionsUtils` actually threw.
Azure already had a `catch (IllegalArgumentException e) { throw e; }` guarding
against exactly this, which is now unnecessary and is removed with the rest.
Out of scope: the wrappers around tool schema and tool argument JSON
handling stay. Those wrap a checked `JsonProcessingException` from local work
rather than a provider error, so they cannot propagate unwrapped.
### Tests
`OpenAICompletionsConnectionTest.testRequestBuildingFailurePropagatesUnwrapped`
and the same test in a new `OpenAIResponsesModelConnectionTest` call `chat`
with a tool message that has no `externalId` and assert the
`IllegalArgumentException` reaches the caller with its own type and message.
Request building throws before the client is touched, so neither test needs a
live API call.
I verified both tests discriminate rather than pass by construction:
restoring the old `catch` block fails
`testRequestBuildingFailurePropagatesUnwrapped` on the assertion, since the
wrapper produces a plain `RuntimeException` that is not an
`IllegalArgumentException`.
Azure gets no new test. Its `IllegalArgumentException` already propagated,
so the only behavior that changed there is what happens to a provider SDK
exception, and reaching that needs a live call through the final `OpenAIClient`.
`mvn -pl integrations/chat-models/openai test` passes, 72 tests. Spotless
and RAT are clean.
### API
No public API change. `chat` keeps its signature; only the exception type a
caller observes on failure changes, from `RuntimeException` wrapping the SDK
exception to the SDK exception itself.
### Documentation
- [ ] `doc-needed`
- [x] `doc-not-needed`
- [ ] `doc-included`
### Was this patch authored or co-authored using generative AI tooling?
- [x] Yes
- [ ] No
`Generated-by: Claude Code 2.1.226`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
