From 3366c2ea48de0725901de9e1bda178c969e2d3dc Mon Sep 17 00:00:00 2001
From: I538344 <charles.dubois@sap.com>
Date: Fri, 30 Aug 2024 08:34:09 +0200
Subject: [PATCH] Added error handling test

---
 e2e-test-app/pom.xml                          |  1 +
 foundation-models/openai/pom.xml              |  5 ++++
 .../openai/OpenAiStreamingHandler.java        |  9 +++++--
 .../openai/OpenAiClientTest.java              | 24 +++++++++++++++----
 pom.xml                                       |  5 ++++
 5 files changed, 38 insertions(+), 6 deletions(-)

diff --git a/e2e-test-app/pom.xml b/e2e-test-app/pom.xml
index aed6b77f..1a0a56b8 100644
--- a/e2e-test-app/pom.xml
+++ b/e2e-test-app/pom.xml
@@ -90,6 +90,7 @@
     <dependency>
       <groupId>org.springframework</groupId>
       <artifactId>spring-webmvc</artifactId>
+      <version>${springframework.version}</version>
     </dependency>
     <dependency>
       <groupId>com.google.code.findbugs</groupId>
diff --git a/foundation-models/openai/pom.xml b/foundation-models/openai/pom.xml
index b1319ac0..a9289772 100644
--- a/foundation-models/openai/pom.xml
+++ b/foundation-models/openai/pom.xml
@@ -97,6 +97,11 @@
       <artifactId>junit-jupiter-api</artifactId>
       <scope>test</scope>
     </dependency>
+    <dependency>
+      <groupId>org.junit.jupiter</groupId>
+      <artifactId>junit-jupiter-params</artifactId>
+      <scope>test</scope>
+    </dependency>
     <dependency>
       <groupId>org.wiremock</groupId>
       <artifactId>wiremock</artifactId>
diff --git a/foundation-models/openai/src/main/java/com/sap/ai/sdk/foundationmodels/openai/OpenAiStreamingHandler.java b/foundation-models/openai/src/main/java/com/sap/ai/sdk/foundationmodels/openai/OpenAiStreamingHandler.java
index 89cf969a..ef3b5c19 100644
--- a/foundation-models/openai/src/main/java/com/sap/ai/sdk/foundationmodels/openai/OpenAiStreamingHandler.java
+++ b/foundation-models/openai/src/main/java/com/sap/ai/sdk/foundationmodels/openai/OpenAiStreamingHandler.java
@@ -69,7 +69,9 @@ private Stream<D> parseResponse(@Nonnull final ClassicHttpResponse response)
         .peek(
             responseLine -> {
               if (!responseLine.startsWith("data: ")) {
-                parseErrorAndThrow(responseLine, new OpenAiClientException());
+                parseErrorAndThrow(
+                    responseLine,
+                    new OpenAiClientException("Failed to parse response from OpenAI model"));
               }
             })
         .map(
@@ -78,7 +80,10 @@ private Stream<D> parseResponse(@Nonnull final ClassicHttpResponse response)
               try {
                 return JACKSON.readValue(data, deltaType);
               } catch (final IOException e) {
-                throw new OpenAiClientException("Failed to parse delta message: " + data, e);
+                log.error(
+                    "Failed to parse the following response from OpenAI model: {}", responseLine);
+                throw new OpenAiClientException(
+                    "Failed to parse delta message: " + responseLine, e);
               }
             })
         .peek(
diff --git a/foundation-models/openai/src/test/java/com/sap/ai/sdk/foundationmodels/openai/OpenAiClientTest.java b/foundation-models/openai/src/test/java/com/sap/ai/sdk/foundationmodels/openai/OpenAiClientTest.java
index 6e968d8f..1c8b0d2f 100644
--- a/foundation-models/openai/src/test/java/com/sap/ai/sdk/foundationmodels/openai/OpenAiClientTest.java
+++ b/foundation-models/openai/src/test/java/com/sap/ai/sdk/foundationmodels/openai/OpenAiClientTest.java
@@ -21,10 +21,16 @@
 import java.io.IOException;
 import java.util.List;
 import java.util.stream.Stream;
+import javax.annotation.Nonnull;
 import org.assertj.core.api.SoftAssertions;
 import org.junit.jupiter.api.BeforeEach;
 import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.TestInstance;
+import org.junit.jupiter.api.TestInstance.Lifecycle;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.MethodSource;
 
+@TestInstance(Lifecycle.PER_CLASS)
 @WireMockTest
 class OpenAiClientTest {
   private OpenAiClient client;
@@ -55,8 +61,19 @@ void apiVersion() {
     verify(exactly(2), postRequestedFor(anyUrl()).withoutQueryParam("api-version"));
   }
 
-  @Test
-  void errorHandling() {
+  private Stream<Runnable> chatCompletionCalls() {
+    return Stream.of(
+        () -> client.chatCompletion(new OpenAiChatCompletionParameters()),
+        () ->
+            client
+                .streamChatCompletion(new OpenAiChatCompletionParameters())
+                // the stream needs to be consumed to parse the response
+                .forEach(System.out::println));
+  }
+
+  @ParameterizedTest
+  @MethodSource("chatCompletionCalls")
+  void chatCompletionErrorHandling(@Nonnull final Runnable request) {
     final var errorJson =
         """
             { "error": { "code": null, "message": "foo", "type": "invalid stuff" } }
@@ -91,7 +108,6 @@ void errorHandling() {
             .willSetStateTo("4"));
     stubFor(post(anyUrl()).inScenario("Errors").whenScenarioStateIs("4").willReturn(noContent()));
 
-    final Runnable request = () -> client.chatCompletion(new OpenAiChatCompletionParameters());
     final var softly = new SoftAssertions();
 
     softly
@@ -255,7 +271,7 @@ void embedding() throws IOException {
           postRequestedFor(urlPathEqualTo("/embeddings"))
               .withRequestBody(
                   equalToJson("""
-                          {"input":["Hello World"]}""")));
+                      {"input":["Hello World"]}""")));
     }
   }
 
diff --git a/pom.xml b/pom.xml
index c9dd00da..8840341b 100644
--- a/pom.xml
+++ b/pom.xml
@@ -91,6 +91,11 @@
         <version>${junit-jupiter.version}</version>
         <scope>test</scope>
       </dependency>
+      <dependency>
+        <groupId>org.junit.jupiter</groupId>
+        <artifactId>junit-jupiter-params</artifactId>
+        <version>${junit-jupiter.version}</version>
+      </dependency>
       <dependency>
         <groupId>org.wiremock</groupId>
         <artifactId>wiremock</artifactId>