Skip to content

Commit 31fabfa

Browse files
CharlesDuboisSAPCharlesDuboisSAP
committed
Reduce duplication of ResponseHandlers
1 parent 8618dcd commit 31fabfa

File tree

16 files changed

+186
-92
lines changed

16 files changed

+186
-92
lines changed

‎core/pom.xml‎

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,12 @@
3030
</developers>
3131
<properties>
3232
<project.rootdir>${project.basedir}/../</project.rootdir>
33+
<coverage.complexity>60%</coverage.complexity>
34+
<coverage.line>70%</coverage.line>
35+
<coverage.instruction>68%</coverage.instruction>
36+
<coverage.branch>52%</coverage.branch>
37+
<coverage.method>73%</coverage.method>
38+
<coverage.class>87%</coverage.class>
3339
</properties>
3440

3541
<dependencies>
@@ -66,6 +72,10 @@
6672
<groupId>org.apache.httpcomponents.client5</groupId>
6773
<artifactId>httpclient5</artifactId>
6874
</dependency>
75+
<dependency>
76+
<groupId>org.apache.httpcomponents.core5</groupId>
77+
<artifactId>httpcore5</artifactId>
78+
</dependency>
6979
<dependency>
7080
<groupId>com.google.code.findbugs</groupId>
7181
<artifactId>jsr305</artifactId>
@@ -129,11 +139,6 @@
129139
<artifactId>wiremock</artifactId>
130140
<scope>test</scope>
131141
</dependency>
132-
<dependency>
133-
<groupId>org.apache.httpcomponents.core5</groupId>
134-
<artifactId>httpcore5</artifactId>
135-
<scope>test</scope>
136-
</dependency>
137142
<dependency>
138143
<groupId>org.assertj</groupId>
139144
<artifactId>assertj-core</artifactId>

‎core/src/main/java/com/sap/ai/sdk/core/commons/ClientError.java‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,14 @@
1+
package com.sap.ai.sdk.core.commons;
2+
3+
import javax.annotation.Nonnull;
4+
5+
/** Generic class that contains a JSON error response. */
6+
public interface ClientError {
7+
/**
8+
* Get the error message.
9+
*
10+
* @return The error message
11+
*/
12+
@Nonnull
13+
String getMessage();
14+
}

‎core/src/main/java/com/sap/ai/sdk/core/commons/ClientException.java‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
package com.sap.ai.sdk.core.commons;
2+
3+
import lombok.experimental.StandardException;
4+
5+
/** Generic exception for errors occurring when using AI SDK clients. */
6+
@StandardException
7+
public class ClientException extends RuntimeException {}

orchestration/src/main/java/com/sap/ai/sdk/orchestration/OrchestrationResponseHandler.java renamed to core/src/main/java/com/sap/ai/sdk/core/commons/ClientResponseHandler.java

Lines changed: 71 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,13 @@
1-
package com.sap.ai.sdk.orchestration;
1+
package com.sap.ai.sdk.core.commons;
22

3-
import static com.sap.ai.sdk.orchestration.OrchestrationClient.JACKSON;
3+
import static com.sap.ai.sdk.core.JacksonConfiguration.getDefaultObjectMapper;
44

55
import com.fasterxml.jackson.core.JsonProcessingException;
6-
import com.sap.ai.sdk.orchestration.model.ErrorResponse;
6+
import com.fasterxml.jackson.databind.ObjectMapper;
77
import io.vavr.control.Try;
88
import java.io.IOException;
99
import java.nio.charset.StandardCharsets;
10+
import java.util.function.BiFunction;
1011
import javax.annotation.Nonnull;
1112
import lombok.RequiredArgsConstructor;
1213
import lombok.extern.slf4j.Slf4j;
@@ -18,28 +19,78 @@
1819
import org.apache.hc.core5.http.io.HttpClientResponseHandler;
1920
import org.apache.hc.core5.http.io.entity.EntityUtils;
2021

22+
/**
23+
* Parse incoming JSON responses and handles any errors
24+
*
25+
* @param <T> The type of the response
26+
* @param <E> The type of the exception to throw
27+
*/
2128
@Slf4j
2229
@RequiredArgsConstructor
23-
class OrchestrationResponseHandler<T> implements HttpClientResponseHandler<T> {
30+
public class ClientResponseHandler<T, E extends ClientException>
31+
implements HttpClientResponseHandler<T> {
2432
@Nonnull private final Class<T> responseType;
33+
@Nonnull private final Class<? extends ClientError> errorType;
34+
@Nonnull private final BiFunction<String, Throwable, E> exceptionType;
2535

36+
/** The parses for JSON responses, will be private once we can remove mixins */
37+
@Nonnull public ObjectMapper JACKSON = getDefaultObjectMapper();
38+
39+
/**
40+
* Processes a {@link ClassicHttpResponse} and returns some value corresponding to that response.
41+
*
42+
* @param response The response to process
43+
* @return A model class instantiated from the response
44+
* @throws E in case of a problem or the connection was aborted
45+
*/
46+
@Nonnull
47+
@Override
48+
public T handleResponse(@Nonnull final ClassicHttpResponse response) throws E {
49+
if (response.getCode() >= 300) {
50+
buildExceptionAndThrow(response);
51+
}
52+
return parseResponse(response);
53+
}
54+
55+
// The InputStream of the HTTP entity is closed by EntityUtils.toString
56+
@SuppressWarnings("PMD.CloseResource")
2657
@Nonnull
27-
private static String getContent(@Nonnull final HttpEntity entity) {
58+
private T parseResponse(@Nonnull final ClassicHttpResponse response) throws E {
59+
final HttpEntity responseEntity = response.getEntity();
60+
if (responseEntity == null) {
61+
throw exceptionType.apply("Response was empty.", null);
62+
}
63+
val content = getContent(responseEntity);
64+
log.debug("Parsing response from JSON response: {}", content);
65+
try {
66+
return JACKSON.readValue(content, responseType);
67+
} catch (final JsonProcessingException e) {
68+
log.error("Failed to parse the following response: {}", content);
69+
throw exceptionType.apply("Failed to parse response", e);
70+
}
71+
}
72+
73+
@Nonnull
74+
private String getContent(@Nonnull final HttpEntity entity) {
2875
try {
2976
return EntityUtils.toString(entity, StandardCharsets.UTF_8);
3077
} catch (IOException | ParseException e) {
31-
throw new OrchestrationClientException("Failed to read response content.", e);
78+
throw exceptionType.apply("Failed to read response content.", e);
3279
}
3380
}
3481

35-
// The InputStream of the HTTP entity is closed by EntityUtils.toString
82+
/**
83+
* Parse the error response and throw an exception.
84+
*
85+
* @param response The response to process
86+
*/
3687
@SuppressWarnings("PMD.CloseResource")
37-
static void buildExceptionAndThrow(@Nonnull final ClassicHttpResponse response)
38-
throws OrchestrationClientException {
88+
public void buildExceptionAndThrow(@Nonnull final ClassicHttpResponse response) throws E {
3989
val exception =
40-
new OrchestrationClientException(
41-
"Request to orchestration service failed with status %s %s"
42-
.formatted(response.getCode(), response.getReasonPhrase()));
90+
exceptionType.apply(
91+
"Request failed with status %s %s"
92+
.formatted(response.getCode(), response.getReasonPhrase()),
93+
null);
4394
val entity = response.getEntity();
4495
if (entity == null) {
4596
throw exception;
@@ -54,9 +105,7 @@ static void buildExceptionAndThrow(@Nonnull final ClassicHttpResponse response)
54105
throw exception;
55106
}
56107

57-
log.error(
58-
"The orchestration service responded with an HTTP error and the following content: {}",
59-
content);
108+
log.error("The service responded with an HTTP error and the following content: {}", content);
60109
val contentType = ContentType.parse(entity.getContentType());
61110
if (!ContentType.APPLICATION_JSON.isSameMimeType(contentType)) {
62111
throw exception;
@@ -68,59 +117,20 @@ static void buildExceptionAndThrow(@Nonnull final ClassicHttpResponse response)
68117
/**
69118
* Parse the error response and throw an exception.
70119
*
71-
* @param errorResponse the error response, most likely a JSON of {@link ErrorResponse}.
120+
* @param errorResponse the error response, most likely a unique JSON class.
72121
* @param baseException a base exception to add the error message to.
73122
*/
74-
static void parseErrorAndThrow(
75-
@Nonnull final String errorResponse,
76-
@Nonnull final OrchestrationClientException baseException)
77-
throws OrchestrationClientException {
78-
val maybeError = Try.of(() -> JACKSON.readValue(errorResponse, ErrorResponse.class));
123+
public void parseErrorAndThrow(
124+
@Nonnull final String errorResponse, @Nonnull final E baseException) throws E {
125+
val maybeError = Try.of(() -> JACKSON.readValue(errorResponse, errorType));
79126
if (maybeError.isFailure()) {
80127
baseException.addSuppressed(maybeError.getCause());
81128
throw baseException;
82129
}
83130

84-
throw new OrchestrationClientException(
131+
throw exceptionType.apply(
85132
"%s and error message: '%s'"
86-
.formatted(baseException.getMessage(), maybeError.get().getMessage()));
87-
}
88-
89-
/**
90-
* Processes a {@link ClassicHttpResponse} and returns some value corresponding to that response.
91-
*
92-
* @param response The response to process
93-
* @return A model class instantiated from the response
94-
* @throws OrchestrationClientException in case of a problem or the connection was aborted
95-
*/
96-
@Override
97-
public T handleResponse(@Nonnull final ClassicHttpResponse response)
98-
throws OrchestrationClientException {
99-
if (response.getCode() >= 300) {
100-
buildExceptionAndThrow(response);
101-
}
102-
val result = parseResponse(response);
103-
log.debug("Received the following response from orchestration service: {}", result);
104-
return result;
105-
}
106-
107-
// The InputStream of the HTTP entity is closed by EntityUtils.toString
108-
@SuppressWarnings("PMD.CloseResource")
109-
@Nonnull
110-
private T parseResponse(@Nonnull final ClassicHttpResponse response)
111-
throws OrchestrationClientException {
112-
final HttpEntity responseEntity = response.getEntity();
113-
if (responseEntity == null) {
114-
throw new OrchestrationClientException("Response from Orchestration service was empty.");
115-
}
116-
val content = getContent(responseEntity);
117-
log.debug("Parsing response from JSON response: {}", content);
118-
try {
119-
return JACKSON.readValue(content, responseType);
120-
} catch (final JsonProcessingException e) {
121-
log.error("Failed to parse the following response from orchestration service: {}", content);
122-
throw new OrchestrationClientException(
123-
"Failed to parse response from orchestration service", e);
124-
}
133+
.formatted(baseException.getMessage(), maybeError.get().getMessage()),
134+
null);
125135
}
126136
}

‎foundation-models/openai/pom.xml‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,12 @@
3333
</developers>
3434
<properties>
3535
<project.rootdir>${project.basedir}/../../</project.rootdir>
36+
<coverage.complexity>70%</coverage.complexity>
37+
<coverage.line>76%</coverage.line>
38+
<coverage.instruction>75%</coverage.instruction>
39+
<coverage.branch>66%</coverage.branch>
40+
<coverage.method>83%</coverage.method>
41+
<coverage.class>85%</coverage.class>
3642
</properties>
3743
<dependencies>
3844
<dependency>

‎foundation-models/openai/src/main/java/com/sap/ai/sdk/foundationmodels/openai/OpenAiClient.java‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,13 +7,15 @@
77
import com.google.common.annotations.Beta;
88
import com.sap.ai.sdk.core.AiCoreService;
99
import com.sap.ai.sdk.core.DeploymentResolutionException;
10+
import com.sap.ai.sdk.core.commons.ClientResponseHandler;
1011
import com.sap.ai.sdk.foundationmodels.openai.model.OpenAiChatCompletionDelta;
1112
import com.sap.ai.sdk.foundationmodels.openai.model.OpenAiChatCompletionOutput;
1213
import com.sap.ai.sdk.foundationmodels.openai.model.OpenAiChatCompletionParameters;
1314
import com.sap.ai.sdk.foundationmodels.openai.model.OpenAiChatMessage.OpenAiChatSystemMessage;
1415
import com.sap.ai.sdk.foundationmodels.openai.model.OpenAiChatMessage.OpenAiChatUserMessage;
1516
import com.sap.ai.sdk.foundationmodels.openai.model.OpenAiEmbeddingOutput;
1617
import com.sap.ai.sdk.foundationmodels.openai.model.OpenAiEmbeddingParameters;
18+
import com.sap.ai.sdk.foundationmodels.openai.model.OpenAiError;
1719
import com.sap.ai.sdk.foundationmodels.openai.model.StreamedDelta;
1820
import com.sap.cloud.sdk.cloudplatform.connectivity.ApacheHttpClient5Accessor;
1921
import com.sap.cloud.sdk.cloudplatform.connectivity.DefaultHttpDestination;
@@ -283,7 +285,9 @@ private <T> T executeRequest(
283285
final BasicClassicHttpRequest request, @Nonnull final Class<T> responseType) {
284286
try {
285287
final var client = ApacheHttpClient5Accessor.getHttpClient(destination);
286-
return client.execute(request, new OpenAiResponseHandler<>(responseType));
288+
return client.execute(
289+
request,
290+
new ClientResponseHandler<>(responseType, OpenAiError.class, OpenAiClientException::new));
287291
} catch (final IOException e) {
288292
throw new OpenAiClientException("Request to OpenAI model failed", e);
289293
}

‎foundation-models/openai/src/main/java/com/sap/ai/sdk/foundationmodels/openai/OpenAiClientException.java‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,8 @@
11
package com.sap.ai.sdk.foundationmodels.openai;
22

3+
import com.sap.ai.sdk.core.commons.ClientException;
34
import lombok.experimental.StandardException;
45

56
/** Generic exception for errors occurring when using OpenAI foundation models. */
67
@StandardException
7-
public class OpenAiClientException extends RuntimeException {}
8+
public class OpenAiClientException extends ClientException {}

‎foundation-models/openai/src/main/java/com/sap/ai/sdk/foundationmodels/openai/OpenAiResponseHandler.java‎

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
import javax.annotation.Nonnull;
1111
import lombok.RequiredArgsConstructor;
1212
import lombok.extern.slf4j.Slf4j;
13+
import lombok.val;
1314
import org.apache.hc.core5.http.ClassicHttpResponse;
1415
import org.apache.hc.core5.http.ContentType;
1516
import org.apache.hc.core5.http.HttpEntity;
@@ -48,7 +49,7 @@ private T parseResponse(@Nonnull final ClassicHttpResponse response)
4849
if (responseEntity == null) {
4950
throw new OpenAiClientException("Response from OpenAI model was empty.");
5051
}
51-
final var content = getContent(responseEntity);
52+
val content = getContent(responseEntity);
5253
try {
5354
return JACKSON.readValue(content, responseType);
5455
} catch (final JsonProcessingException e) {
@@ -70,26 +71,26 @@ private static String getContent(@Nonnull final HttpEntity entity) {
7071
@SuppressWarnings("PMD.CloseResource")
7172
static void buildExceptionAndThrow(@Nonnull final ClassicHttpResponse response)
7273
throws OpenAiClientException {
73-
final var exception =
74+
val exception =
7475
new OpenAiClientException(
7576
"Request to OpenAI model failed with status %s %s"
7677
.formatted(response.getCode(), response.getReasonPhrase()));
77-
final var entity = response.getEntity();
78+
val entity = response.getEntity();
7879
if (entity == null) {
7980
throw exception;
8081
}
81-
final var maybeContent = Try.of(() -> getContent(entity));
82+
val maybeContent = Try.of(() -> getContent(entity));
8283
if (maybeContent.isFailure()) {
8384
exception.addSuppressed(maybeContent.getCause());
8485
throw exception;
8586
}
86-
final var content = maybeContent.get();
87+
val content = maybeContent.get();
8788
if (content.isBlank()) {
8889
throw exception;
8990
}
9091

9192
log.error("OpenAI model responded with an HTTP error and the following content: {}", content);
92-
final var contentType = ContentType.parse(entity.getContentType());
93+
val contentType = ContentType.parse(entity.getContentType());
9394
if (!ContentType.APPLICATION_JSON.isSameMimeType(contentType)) {
9495
throw exception;
9596
}
@@ -106,13 +107,13 @@ static void buildExceptionAndThrow(@Nonnull final ClassicHttpResponse response)
106107
static void parseErrorAndThrow(
107108
@Nonnull final String errorResponse, @Nonnull final OpenAiClientException baseException)
108109
throws OpenAiClientException {
109-
final var maybeError = Try.of(() -> JACKSON.readValue(errorResponse, OpenAiError.class));
110+
val maybeError = Try.of(() -> JACKSON.readValue(errorResponse, OpenAiError.class));
110111
if (maybeError.isFailure()) {
111112
baseException.addSuppressed(maybeError.getCause());
112113
throw baseException;
113114
}
114115

115-
final var error = maybeError.get().getError();
116+
val error = maybeError.get().getError();
116117
if (error == null) {
117118
throw baseException;
118119
}

‎foundation-models/openai/src/main/java/com/sap/ai/sdk/foundationmodels/openai/model/OpenAiError.java‎

Lines changed: 10 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,8 @@
22

33
import com.fasterxml.jackson.annotation.JsonProperty;
44
import com.google.common.annotations.Beta;
5+
import com.sap.ai.sdk.core.commons.ClientError;
6+
import javax.annotation.Nonnull;
57
import javax.annotation.Nullable;
68
import lombok.EqualsAndHashCode;
79
import lombok.Getter;
@@ -13,9 +15,16 @@
1315
@EqualsAndHashCode
1416
@ToString
1517
@Beta
16-
public class OpenAiError {
18+
public class OpenAiError implements ClientError {
1719
/** The error object. */
1820
@JsonProperty("error")
1921
@Getter(onMethod_ = @Nullable)
2022
private OpenAiErrorBase error;
23+
24+
@Nonnull
25+
@Override
26+
public String getMessage() {
27+
final String message = error.getMessage();
28+
return message == null ? "" : message;
29+
}
2130
}

‎orchestration/pom.xml‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,12 @@
3131
</developers>
3232
<properties>
3333
<project.rootdir>${project.basedir}/../</project.rootdir>
34+
<coverage.complexity>70%</coverage.complexity>
35+
<coverage.line>86%</coverage.line>
36+
<coverage.instruction>86%</coverage.instruction>
37+
<coverage.branch>67%</coverage.branch>
38+
<coverage.method>85%</coverage.method>
39+
<coverage.class>100%</coverage.class>
3440
</properties>
3541

3642
<dependencies>

0 commit comments

Comments
 (0)