Refactor negative scenario tests. Add exception mapping to OpenAiOfficialChatModel #3310

kpavlov · 2025-07-06T00:23:41Z

Issue

Reuse test logic by introducing abstract tests. Catch and fix missing exception mapping in OpenAiOfficialChatModel.

Change

Add error handling tests for AbstractChatModel and its implementations. Remove duplictie tests
Add test for streaming request in GoogleAiGeminiStreamingChatModelTest
Add OpenAiOfficialChatModelErrorsTest
Implemented missing OpenAiOfficialExceptionMapper to handle OpenAI API exceptions
Enhance HttpException constructors to carry underlying exception (cause)
Remove tests duplicating AbstractChatModelErrorsTest and it's children

General checklist

There are no breaking changes
I have added unit and/or integration tests for my change
The tests cover both positive and negative cases
I have manually run all the unit and integration tests in the module I have added/changed, and they are all green
I have manually run all the unit and integration tests in the core and main modules, and they are all green

I have added/updated the documentation
I have added an example in the examples repo (only for "big" features)
I have added/updated Spring Boot starter(s) (if applicable)

Checklist for adding new maven module

I have added my new module in the root pom.xml and langchain4j-bom/pom.xml

Checklist for adding new embedding store integration

I have added a {NameOfIntegration}EmbeddingStoreIT that extends from either EmbeddingStoreIT or EmbeddingStoreWithFilteringIT
I have added a {NameOfIntegration}EmbeddingStoreRemovalIT that extends from EmbeddingStoreWithRemovalIT

Checklist for changing existing embedding store integration

I have manually verified that the {NameOfIntegration}EmbeddingStore works correctly with the data persisted using the latest released version of LangChain4j

…istency in AbstractChatModelErrorsTest

…penAiOfficialExceptionMapper` and adjusting maxRetries logic

…3316)

…3313) This PR contains the following updates: | Package | Change | Age | Confidence | |---|---|---|---| | [com.google.protobuf:protobuf-java-util](https://developers.google.com/protocol-buffers/) ([source](https://redirect.github.com/protocolbuffers/protobuf)) | `3.25.5` -> `3.25.8` | [![age](https://developer.mend.io/api/mc/badges/age/maven/com.google.protobuf:protobuf-java-util/3.25.8?slim=true)](https://docs.renovatebot.com/merge-confidence/) | [![confidence](https://developer.mend.io/api/mc/badges/confidence/maven/com.google.protobuf:protobuf-java-util/3.25.5/3.25.8?slim=true)](https://docs.renovatebot.com/merge-confidence/) | --- > [!WARNING] > Some dependencies could not be looked up. Check the Dependency Dashboard for more information. --- ### Release Notes <details> <summary>protocolbuffers/protobuf (com.google.protobuf:protobuf-java-util)</summary> ### [`v3.25.8`](https://redirect.github.com/protocolbuffers/protobuf/compare/v3.25.7...v3.25.8) [Compare Source](https://redirect.github.com/protocolbuffers/protobuf/compare/v3.25.7...v3.25.8) ### [`v3.25.7`](https://redirect.github.com/protocolbuffers/protobuf/compare/v3.25.6...v3.25.7) [Compare Source](https://redirect.github.com/protocolbuffers/protobuf/compare/v3.25.6...v3.25.7) ### [`v3.25.6`](https://redirect.github.com/protocolbuffers/protobuf/compare/v3.25.5...v3.25.6) [Compare Source](https://redirect.github.com/protocolbuffers/protobuf/compare/v3.25.5...v3.25.6) </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Disabled by config. Please merge this manually once you are satisfied. ♻ **Rebasing**: Whenever PR is behind base branch, or you tick the rebase/retry checkbox. 🔕 **Ignore**: Close this PR and you won't be reminded about this update again. --- - [ ] If you want to rebase/retry this PR, check this box --- This PR was generated by [Mend Renovate](https://mend.io/renovate/). View the [repository job log](https://developer.mend.io/github/langchain4j/langchain4j).  Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>

dliubarskyi

@kpavlov thanks a lot, good idea to abstract away error tests!

dliubarskyi · 2025-07-07T13:51:42Z

...hain4j-core/src/test/kotlin/dev/langchain4j/model/chat/common/AbstractChatModelErrorsTest.kt

+ */
+@TestInstance(TestInstance.Lifecycle.PER_CLASS)
+@Execution(ExecutionMode.CONCURRENT)
+public abstract class AbstractChatModelErrorsTest<


please keep the code written in Java (for all tests)

dliubarskyi · 2025-07-07T13:54:58Z

...hain4j-core/src/test/kotlin/dev/langchain4j/model/chat/common/AbstractChatModelErrorsTest.kt

+    }
+
+    @Test
+    public fun should_handle_timeout() {


Original tests run for a few timeouts (1, 10, 100 ms) to make sure both connect and read timeouts are tested, could you please keep that logic?

...hain4j-core/src/test/kotlin/dev/langchain4j/model/chat/common/AbstractChatModelErrorsTest.kt

dliubarskyi · 2025-07-07T14:03:16Z

...official/src/main/java/dev/langchain4j/model/openaiofficial/OpenAiOfficialBaseChatModel.java

@@ -38,6 +42,7 @@ abstract class OpenAiOfficialBaseChatModel {
    protected TokenCountEstimator tokenCountEstimator;
    protected List<ChatModelListener> listeners;
    protected Set<Capability> supportedCapabilities;
+    protected int maxRetries = 2;


Why is this required? maxRetries is set on the OpenAIClientAsync

dliubarskyi · 2025-07-07T14:04:49Z

...ficial/src/main/java/dev/langchain4j/model/openaiofficial/OpenAiOfficialExceptionMapper.java

+    public RuntimeException mapException(Throwable t) {
+        if (t instanceof com.openai.errors.OpenAIServiceException httpResponseException) {
+            final var statusCode = httpResponseException.statusCode();
+            return mapHttpStatusCode(new HttpException(statusCode, httpResponseException), statusCode);


Suggested change

return mapHttpStatusCode(new HttpException(statusCode, httpResponseException), statusCode);

return mapHttpStatusCode(httpResponseException, statusCode);

why adding an extra layer of exceptions?

Once simplified, please remove new ctors in HttpException which are not required any more

dliubarskyi · 2025-07-07T14:06:25Z

...ficial/src/main/java/dev/langchain4j/model/openaiofficial/OpenAiOfficialExceptionMapper.java

+            final var statusCode = httpResponseException.statusCode();
+            return mapHttpStatusCode(new HttpException(statusCode, httpResponseException), statusCode);
+        } else if (t instanceof OpenAIIoException ioException) {
+            if (t.getCause() instanceof InterruptedIOException) {


Looks strange, doesn't OpenAI SDK throw a proper timeout exception?

kpavlov added 2 commits July 6, 2025 01:22

Add test for streaming request in GoogleAiGeminiStreamingChatModelTest

b96e43c

Add error handling tests for AbstractChatModel and its implementations

22ad747

kpavlov added the Testing label Jul 6, 2025

kpavlov added 3 commits July 6, 2025 11:56

AbstractChatModelErrorsTest: refactor and enhance error handling tests

c0e035e

Add OpenAiOfficialChatModelErrorsTest and improve error handling cons…

469706b

…istency in AbstractChatModelErrorsTest

Improve error handling in OpenAiOfficialChatModel by introducing `O…

a73d26d

…penAiOfficialExceptionMapper` and adjusting maxRetries logic

kpavlov marked this pull request as ready for review July 6, 2025 10:14

kpavlov requested review from dliubarskyi, mariofusco and glaforge July 6, 2025 10:14

kpavlov added the Gemini label Jul 6, 2025

kpavlov changed the title ~~Refactor tests with ai-mock~~ Refactor tests with ai-mock. Add exception mapping in OpenAiOfficialChatModel Jul 6, 2025

kpavlov added the bug Something isn't working label Jul 6, 2025

kpavlov changed the title ~~Refactor tests with ai-mock. Add exception mapping in OpenAiOfficialChatModel~~ Refactor negative tests with ai-mock. Add exception mapping in OpenAiOfficialChatModel Jul 6, 2025

kpavlov changed the title ~~Refactor negative tests with ai-mock. Add exception mapping in OpenAiOfficialChatModel~~ Refactor negative scenario tests with ai-mock. Add exception mapping in OpenAiOfficialChatModel Jul 6, 2025

kpavlov changed the title ~~Refactor negative scenario tests with ai-mock. Add exception mapping in OpenAiOfficialChatModel~~ Refactor negative scenario tests. Add exception mapping in OpenAiOfficialChatModel Jul 6, 2025

kpavlov changed the title ~~Refactor negative scenario tests. Add exception mapping in OpenAiOfficialChatModel~~ Refactor negative scenario tests. Add exception mapping to OpenAiOfficialChatModel Jul 6, 2025

kpavlov mentioned this pull request Jul 6, 2025

Fix: OpenAI, Azure OpenAI: throw ContentFilteredException in case of content filtered or refusal #3294

Merged

8 tasks

LizeRaes and others added 7 commits July 6, 2025 22:01

Feat/OpenAI compatible docs (#3309)

12b54b0

Update dependency maven to v3.9.10 (#3314)

9b79a4d

Update dependency com.microsoft.onnxruntime:onnxruntime to v1.22.0 (#…

43cd1b3

…3316)

Update ai.djl.version to v0.33.0 (#3049)

7a9873f

Update jlama.version to v0.8.4 (#3048)

dbed6f0

Reorder parameters in VespaEmbeddingStore (#3301)

0b9ac95

kpavlov force-pushed the tests-with-aimock branch from b5fdc1c to 0b9ac95 Compare July 6, 2025 19:01

kpavlov added 3 commits July 6, 2025 22:03

Merge branch 'main' into tests-with-aimock

e3324b4

Merge branch 'main' into tests-with-aimock

b6e768e

Merge branch 'main' into tests-with-aimock

21bb22d

dliubarskyi reviewed Jul 7, 2025

View reviewed changes

Merge branch 'main' into tests-with-aimock

cabb75e

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Uh oh!

Refactor negative scenario tests. Add exception mapping to OpenAiOfficialChatModel #3310

Refactor negative scenario tests. Add exception mapping to OpenAiOfficialChatModel #3310

Uh oh!

kpavlov commented Jul 6, 2025 •

edited

Loading

Uh oh!

dliubarskyi left a comment

Uh oh!

dliubarskyi Jul 7, 2025

Uh oh!

dliubarskyi Jul 7, 2025

Uh oh!

Uh oh!

dliubarskyi Jul 7, 2025

Uh oh!

dliubarskyi Jul 7, 2025

Uh oh!

dliubarskyi Jul 7, 2025

Uh oh!

dliubarskyi Jul 7, 2025

Uh oh!

Uh oh!

	return mapHttpStatusCode(new HttpException(statusCode, httpResponseException), statusCode);
	return mapHttpStatusCode(httpResponseException, statusCode);

Uh oh!

Refactor negative scenario tests. Add exception mapping to OpenAiOfficialChatModel #3310

Are you sure you want to change the base?

Refactor negative scenario tests. Add exception mapping to OpenAiOfficialChatModel #3310

Uh oh!

Conversation

kpavlov commented Jul 6, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Issue

Change

General checklist

Checklist for adding new maven module

Checklist for adding new embedding store integration

Checklist for changing existing embedding store integration

Uh oh!

dliubarskyi left a comment

Choose a reason for hiding this comment

Uh oh!

dliubarskyi Jul 7, 2025

Choose a reason for hiding this comment

Uh oh!

dliubarskyi Jul 7, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

dliubarskyi Jul 7, 2025

Choose a reason for hiding this comment

Uh oh!

dliubarskyi Jul 7, 2025

Choose a reason for hiding this comment

Uh oh!

dliubarskyi Jul 7, 2025

Choose a reason for hiding this comment

Uh oh!

dliubarskyi Jul 7, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

kpavlov commented Jul 6, 2025 •

edited

Loading