Skip to content

Commit b35d43e

Browse files
newtorkbot-sdk-js
andauthored
chore: Improve Exception Handling and Accessors to Response/Request (#529)
* Initial * Drive by code improvement * Fix test * Fix annotation * Add missing http response * Reduce complexity of exception factory * Fix compilation * Improve code style * Formatting * Minor Code cleanup * Add/Update release note * Fix merge conflict * Establish public getMessage() in interface * Formatting * Remove duplicate --------- Co-authored-by: SAP Cloud SDK Bot <[email protected]>
1 parent e3ac050 commit b35d43e

File tree

21 files changed

+248
-238
lines changed

21 files changed

+248
-238
lines changed

core/src/main/java/com/sap/ai/sdk/core/common/ClientException.java

Lines changed: 72 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,13 @@
11
package com.sap.ai.sdk.core.common;
22

33
import com.google.common.annotations.Beta;
4+
import javax.annotation.Nonnull;
45
import javax.annotation.Nullable;
56
import lombok.AccessLevel;
67
import lombok.Getter;
7-
import lombok.Setter;
88
import lombok.experimental.StandardException;
9+
import org.apache.hc.core5.http.ClassicHttpRequest;
10+
import org.apache.hc.core5.http.ClassicHttpResponse;
911

1012
/**
1113
* Generic exception for errors occurring when using AI SDK clients.
@@ -19,9 +21,75 @@ public class ClientException extends RuntimeException {
1921
/**
2022
* Wraps a structured error payload received from the remote service, if available. This can be
2123
* used to extract more detailed error information.
24+
*
25+
* @since 1.10.0
2226
*/
2327
@Nullable
24-
@Getter(onMethod_ = @Beta, value = AccessLevel.PROTECTED)
25-
@Setter(onMethod_ = @Beta, value = AccessLevel.PROTECTED)
26-
ClientError clientError;
28+
@Getter(onMethod_ = @Beta, value = AccessLevel.PUBLIC)
29+
private ClientError clientError;
30+
31+
/**
32+
* The original HTTP response that caused this exception, if available.
33+
*
34+
* @since 1.10.0
35+
*/
36+
@Nullable
37+
@Getter(onMethod_ = @Beta, value = AccessLevel.PUBLIC)
38+
private ClassicHttpResponse httpResponse;
39+
40+
/**
41+
* The original HTTP request that caused this exception, if available.
42+
*
43+
* @since 1.10.0
44+
*/
45+
@Nullable
46+
@Getter(onMethod_ = @Beta, value = AccessLevel.PUBLIC)
47+
private ClassicHttpRequest httpRequest;
48+
49+
/**
50+
* Sets the original HTTP request that caused this exception.
51+
*
52+
* @param clientError the original structured error payload received from the remote service, can
53+
* be null if not available.
54+
* @return the current instance of {@link ClientException} with the changed ClientError data
55+
* @param <T> the type of the exception, typically a subclass of {@link ClientException}
56+
*/
57+
@SuppressWarnings("unchecked")
58+
@Nonnull
59+
public <T extends ClientException> T setClientError(@Nullable final ClientError clientError) {
60+
this.clientError = clientError;
61+
return (T) this;
62+
}
63+
64+
/**
65+
* Sets the original HTTP request that caused this exception.
66+
*
67+
* @param httpResponse the original HTTP response that caused this exception, can be null if not
68+
* available.
69+
* @return the current instance of {@link ClientException} with the changed HTTP response
70+
* @param <T> the type of the exception, typically a subclass of {@link ClientException}
71+
*/
72+
@SuppressWarnings("unchecked")
73+
@Nonnull
74+
public <T extends ClientException> T setHttpResponse(
75+
@Nullable final ClassicHttpResponse httpResponse) {
76+
this.httpResponse = httpResponse;
77+
return (T) this;
78+
}
79+
80+
/**
81+
* Sets the original HTTP request that caused this exception.
82+
*
83+
* @param httpRequest the original HTTP request that caused this exception, can be null if not
84+
* available.
85+
* @return the current instance of {@link ClientException} with the changed HTTP request
86+
* @param <T> the type of the exception, typically a subclass of {@link ClientException}
87+
*/
88+
@SuppressWarnings("unchecked")
89+
@Nonnull
90+
public <T extends ClientException> T setHttpRequest(
91+
@Nullable final ClassicHttpRequest httpRequest) {
92+
this.httpRequest = httpRequest;
93+
return (T) this;
94+
}
2795
}

core/src/main/java/com/sap/ai/sdk/core/common/ClientExceptionFactory.java

Lines changed: 22 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,7 @@
1212
* @param <R> The subtype of {@link ClientError} payload that can be processed by this factory.
1313
*/
1414
@Beta
15+
@FunctionalInterface
1516
public interface ClientExceptionFactory<E extends ClientException, R extends ClientError> {
1617

1718
/**
@@ -22,16 +23,34 @@ public interface ClientExceptionFactory<E extends ClientException, R extends Cli
2223
* @return An instance of the specified {@link ClientException} type
2324
*/
2425
@Nonnull
25-
E build(@Nonnull final String message, @Nullable final Throwable cause);
26+
default E build(@Nonnull final String message, @Nullable final Throwable cause) {
27+
return build(message, null, cause);
28+
}
29+
30+
/**
31+
* Creates an exception with a message and optional cause.
32+
*
33+
* @param message A descriptive message for the exception.
34+
* @return An instance of the specified {@link ClientException} type
35+
*/
36+
@Nonnull
37+
default E build(@Nonnull final String message) {
38+
return build(message, null);
39+
}
2640

2741
/**
2842
* Creates an exception from a given message and an HTTP error response that has been successfully
2943
* deserialized into a {@link ClientError} object.
3044
*
3145
* @param message A descriptive message for the exception.
32-
* @param clientError The structured {@link ClientError} object deserialized from the response.
46+
* @param clientError The structured {@link ClientError} object deserialized from the response,
47+
* null if not exist.
48+
* @param cause An optional cause of the exception, can be null if not applicable.
3349
* @return An instance of the specified {@link ClientException} type
3450
*/
3551
@Nonnull
36-
E buildFromClientError(@Nonnull final String message, @Nonnull final R clientError);
52+
E build(
53+
@Nonnull final String message,
54+
@Nullable final R clientError,
55+
@Nullable final Throwable cause);
3756
}

core/src/main/java/com/sap/ai/sdk/core/common/ClientResponseHandler.java

Lines changed: 12 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -79,17 +79,18 @@ public T handleResponse(@Nonnull final ClassicHttpResponse response) throws E {
7979
private T parseSuccess(@Nonnull final ClassicHttpResponse response) throws E {
8080
final HttpEntity responseEntity = response.getEntity();
8181
if (responseEntity == null) {
82-
throw exceptionFactory.build("The HTTP Response is empty", null);
82+
throw exceptionFactory.build("The HTTP Response is empty").setHttpResponse(response);
8383
}
8484

85+
val message = "Failed to parse response entity.";
8586
val content =
8687
tryGetContent(responseEntity)
87-
.getOrElseThrow(e -> exceptionFactory.build("Failed to parse response entity.", e));
88+
.getOrElseThrow(e -> exceptionFactory.build(message, e).setHttpResponse(response));
8889
try {
8990
return objectMapper.readValue(content, successType);
9091
} catch (final JsonProcessingException e) {
9192
log.error("Failed to parse response to type {}", successType);
92-
throw exceptionFactory.build("Failed to parse response", e);
93+
throw exceptionFactory.build("Failed to parse response", e).setHttpResponse(response);
9394
}
9495
}
9596

@@ -111,19 +112,19 @@ protected void buildAndThrowException(@Nonnull final ClassicHttpResponse httpRes
111112

112113
if (entity == null) {
113114
val message = getErrorMessage(httpResponse, "The HTTP Response is empty");
114-
throw exceptionFactory.build(message, null);
115+
throw exceptionFactory.build(message).setHttpResponse(httpResponse);
115116
}
116117
val maybeContent = tryGetContent(entity);
117118
if (maybeContent.isFailure()) {
118119
val message = getErrorMessage(httpResponse, "Failed to read the response content");
119-
val baseException = exceptionFactory.build(message, null);
120+
val baseException = exceptionFactory.build(message).setHttpResponse(httpResponse);
120121
baseException.addSuppressed(maybeContent.getCause());
121122
throw baseException;
122123
}
123124
val content = maybeContent.get();
124125
if (content == null || content.isBlank()) {
125126
val message = getErrorMessage(httpResponse, "Empty or blank response content");
126-
throw exceptionFactory.build(message, null);
127+
throw exceptionFactory.build(message).setHttpResponse(httpResponse);
127128
}
128129

129130
log.error(
@@ -133,7 +134,7 @@ protected void buildAndThrowException(@Nonnull final ClassicHttpResponse httpRes
133134
val contentType = ContentType.parse(entity.getContentType());
134135
if (!ContentType.APPLICATION_JSON.isSameMimeType(contentType)) {
135136
val message = getErrorMessage(httpResponse, "The response Content-Type is not JSON");
136-
throw exceptionFactory.build(message, null);
137+
throw exceptionFactory.build(message).setHttpResponse(httpResponse);
137138
}
138139

139140
parseErrorResponseAndThrow(content, httpResponse);
@@ -151,20 +152,19 @@ protected void parseErrorResponseAndThrow(
151152
val maybeClientError = Try.of(() -> objectMapper.readValue(content, errorType));
152153
if (maybeClientError.isFailure()) {
153154
val message = getErrorMessage(httpResponse, "Failed to parse the JSON error response");
154-
val baseException = exceptionFactory.build(message, null);
155+
val baseException = exceptionFactory.build(message).setHttpResponse(httpResponse);
155156
baseException.addSuppressed(maybeClientError.getCause());
156157
throw baseException;
157158
}
158159
final R clientError = maybeClientError.get();
159160
val message = getErrorMessage(httpResponse, clientError.getMessage());
160-
throw exceptionFactory.buildFromClientError(message, clientError);
161+
throw exceptionFactory.build(message, clientError, null).setHttpResponse(httpResponse);
161162
}
162163

163164
private static String getErrorMessage(
164-
@Nonnull final ClassicHttpResponse httpResponse, @Nullable final String additionalMessage) {
165+
@Nonnull final ClassicHttpResponse rsp, @Nullable final String additionalMessage) {
165166
val baseErrorMessage =
166-
"Request failed with status %d (%s)"
167-
.formatted(httpResponse.getCode(), httpResponse.getReasonPhrase());
167+
"Request failed with status %d (%s)".formatted(rsp.getCode(), rsp.getReasonPhrase());
168168

169169
val message = Optional.ofNullable(additionalMessage).orElse("");
170170
return message.isEmpty() ? baseErrorMessage : "%s: %s".formatted(baseErrorMessage, message);

core/src/main/java/com/sap/ai/sdk/core/common/ClientStreamingHandler.java

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -63,14 +63,14 @@ public Stream<D> handleStreamingResponse(@Nonnull final ClassicHttpResponse resp
6363
super.buildAndThrowException(response);
6464
}
6565

66-
return IterableStreamConverter.lines(response.getEntity(), exceptionFactory)
66+
return IterableStreamConverter.lines(response, exceptionFactory)
6767
// half of the lines are empty newlines, the last line is "data: [DONE]"
6868
.filter(line -> !line.isEmpty() && !"data: [DONE]".equals(line.trim()))
6969
.peek(
7070
line -> {
7171
if (!line.startsWith("data: ")) {
7272
final String msg = "Failed to parse response";
73-
throw exceptionFactory.build(msg, null);
73+
throw exceptionFactory.build(msg).setHttpResponse(response);
7474
}
7575
})
7676
.map(
@@ -80,7 +80,8 @@ public Stream<D> handleStreamingResponse(@Nonnull final ClassicHttpResponse resp
8080
return objectMapper.readValue(data, successType);
8181
} catch (final IOException e) { // exception message e gets lost
8282
log.error("Failed to parse delta chunk to type {}", successType);
83-
throw exceptionFactory.build("Failed to parse delta chunk", e);
83+
final String message = "Failed to parse delta chunk";
84+
throw exceptionFactory.build(message, e).setHttpResponse(response);
8485
}
8586
});
8687
}

core/src/main/java/com/sap/ai/sdk/core/common/IterableStreamConverter.java

Lines changed: 27 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,6 @@
66

77
import io.vavr.control.Try;
88
import java.io.BufferedReader;
9-
import java.io.IOException;
10-
import java.io.InputStream;
119
import java.io.InputStreamReader;
1210
import java.util.Iterator;
1311
import java.util.NoSuchElementException;
@@ -17,11 +15,10 @@
1715
import java.util.stream.Stream;
1816
import java.util.stream.StreamSupport;
1917
import javax.annotation.Nonnull;
20-
import javax.annotation.Nullable;
2118
import lombok.AccessLevel;
2219
import lombok.RequiredArgsConstructor;
2320
import lombok.extern.slf4j.Slf4j;
24-
import org.apache.hc.core5.http.HttpEntity;
21+
import org.apache.hc.core5.http.ClassicHttpResponse;
2522

2623
/**
2724
* Internal utility class to convert from a reading handler to {@link Iterable} and {@link Stream}.
@@ -37,14 +34,18 @@ class IterableStreamConverter<T> implements Iterator<T> {
3734
/** see DEFAULT_CHAR_BUFFER_SIZE in {@link BufferedReader} * */
3835
static final int BUFFER_SIZE = 8192;
3936

37+
private static final String ERR_CONTENT = "Failed to read response content.";
38+
private static final String ERR_INTERRUPTED = "Parsing response content was interrupted";
39+
private static final String ERR_CLOSE = "Could not close input stream with error: {} (ignored)";
40+
4041
/** Read next entry for Stream or {@code null} when no further entry can be read. */
4142
private final Callable<T> readHandler;
4243

4344
/** Close handler to be called when Stream terminated. */
4445
private final Runnable stopHandler;
4546

4647
/** Error handler to be called when Stream is interrupted. */
47-
private final Function<Exception, RuntimeException> errorHandler;
48+
private final Function<Throwable, RuntimeException> errorHandler;
4849

4950
private boolean isDone = false;
5051
private boolean isNextFetched = false;
@@ -86,44 +87,44 @@ public T next() {
8687

8788
/**
8889
* Create a sequential Stream of lines from an HTTP response string (UTF-8). The underlying {@link
89-
* InputStream} is closed, when the resulting Stream is closed (e.g. via try-with-resources) or
90-
* when an exception occurred.
90+
* java.io.InputStream} is closed, when the resulting Stream is closed (e.g. via
91+
* try-with-resources) or when an exception occurred.
9192
*
92-
* @param entity The HTTP entity object.
93+
* @param response The HTTP response object.
9394
* @param exceptionFactory The exception factory to use for creating exceptions.
9495
* @return A sequential Stream object.
9596
* @throws ClientException if the provided HTTP entity object is {@code null} or empty.
9697
*/
9798
@SuppressWarnings("PMD.CloseResource") // Stream is closed automatically when consumed
9899
@Nonnull
99100
static <R extends ClientError> Stream<String> lines(
100-
@Nullable final HttpEntity entity,
101+
@Nonnull final ClassicHttpResponse response,
101102
@Nonnull final ClientExceptionFactory<? extends ClientException, R> exceptionFactory)
102103
throws ClientException {
103-
if (entity == null) {
104-
throw exceptionFactory.build("The HTTP Response is empty", null);
104+
if (response.getEntity() == null) {
105+
throw exceptionFactory.build("The HTTP Response is empty").setHttpResponse(response);
105106
}
106107

107-
final InputStream inputStream;
108-
try {
109-
inputStream = entity.getContent();
110-
} catch (final IOException e) {
111-
throw exceptionFactory.build("Failed to read response content.", e);
112-
}
108+
// access input stream
109+
final var inputStream =
110+
Try.of(() -> response.getEntity().getContent())
111+
.getOrElseThrow(e -> exceptionFactory.build(ERR_CONTENT, e).setHttpResponse(response));
113112

113+
// initialize buffered reader
114114
final var reader = new BufferedReader(new InputStreamReader(inputStream, UTF_8), BUFFER_SIZE);
115+
116+
// define close handler
115117
final Runnable closeHandler =
116-
() ->
117-
Try.run(reader::close)
118-
.onFailure(
119-
e ->
120-
log.debug(
121-
"Could not close input stream with error: {} (ignored)",
122-
e.getClass().getSimpleName()));
123-
final Function<Exception, RuntimeException> errHandler =
124-
e -> exceptionFactory.build("Parsing response content was interrupted", e);
118+
() -> Try.run(reader::close).onFailure(e -> log.debug(ERR_CLOSE, e.getClass()));
119+
120+
// define error handler
121+
final Function<Throwable, RuntimeException> errHandler =
122+
e -> exceptionFactory.build(ERR_INTERRUPTED, e).setHttpResponse(response);
125123

124+
// initialize lazy stream iterator
126125
final var iterator = new IterableStreamConverter<>(reader::readLine, closeHandler, errHandler);
126+
127+
// create lazy stream as output
127128
final var spliterator = Spliterators.spliteratorUnknownSize(iterator, ORDERED | NONNULL);
128129
return StreamSupport.stream(spliterator, /* NOT PARALLEL */ false).onClose(closeHandler);
129130
}

core/src/test/java/com/sap/ai/sdk/core/common/ClientResponseHandlerTest.java

Lines changed: 6 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
import com.fasterxml.jackson.core.JsonParseException;
1111
import java.io.IOException;
1212
import javax.annotation.Nonnull;
13+
import javax.annotation.Nullable;
1314
import lombok.Data;
1415
import lombok.SneakyThrows;
1516
import lombok.experimental.StandardException;
@@ -35,16 +36,11 @@ static class MyException extends ClientException {}
3536
static class MyExceptionFactory implements ClientExceptionFactory<MyException, MyError> {
3637
@Nonnull
3738
@Override
38-
public MyException build(@Nonnull String message, Throwable cause) {
39-
return new MyException(message, cause);
40-
}
41-
42-
@Nonnull
43-
@Override
44-
public MyException buildFromClientError(@Nonnull String message, @Nonnull MyError clientError) {
45-
var ex = new MyException(message);
46-
ex.clientError = clientError;
47-
return ex;
39+
public MyException build(
40+
@Nonnull final String message,
41+
@Nullable final MyError clientError,
42+
@Nullable final Throwable cause) {
43+
return new MyException(message, cause).setClientError(clientError);
4844
}
4945
}
5046

0 commit comments

Comments
 (0)