Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
76 changes: 72 additions & 4 deletions core/src/main/java/com/sap/ai/sdk/core/common/ClientException.java
Original file line number Diff line number Diff line change
@@ -1,11 +1,13 @@
package com.sap.ai.sdk.core.common;

import com.google.common.annotations.Beta;
import javax.annotation.Nonnull;
import javax.annotation.Nullable;
import lombok.AccessLevel;
import lombok.Getter;
import lombok.Setter;
import lombok.experimental.StandardException;
import org.apache.hc.core5.http.ClassicHttpRequest;
import org.apache.hc.core5.http.ClassicHttpResponse;

/**
* Generic exception for errors occurring when using AI SDK clients.
Expand All @@ -19,9 +21,75 @@ public class ClientException extends RuntimeException {
/**
* Wraps a structured error payload received from the remote service, if available. This can be
* used to extract more detailed error information.
*
* @since 1.10.0
*/
@Nullable
@Getter(onMethod_ = @Beta, value = AccessLevel.PROTECTED)
@Setter(onMethod_ = @Beta, value = AccessLevel.PROTECTED)
ClientError clientError;
@Getter(onMethod_ = @Beta, value = AccessLevel.PUBLIC)
Copy link
Member

@rpanackal rpanackal Aug 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(Question/Suggestion)

Should getClientError() remain protected? As far as I see, ClientError is an internal wrapper for error response.

For users, there exist a getErrorResponse() in module specific exceptions like OpenAiClientException to shortcut access to the error response.

I feel like getErrorResponse() is convenient than getClientError() and would reduce confusion. Having both method public may be redundant?

Copy link
Contributor Author

@newtork newtork Aug 12, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The following API is available

catch(OrchestrationClientException e) {
  e.getMessage(); // default
  e.getCause(); // default

  e.getHttpResponse(); // ClassicHttpResponse
  e.getHttpRequest(); // ClassicHttpRequest
  e.getClientError().getMessage(); // convenient message
  e.getErrorResponse(); // ErrorResponse (model class)
  e.getErrorResponseStreaming(); // ErrorResponseStreaming  (model class)
}

The e.getClientError().getMessage() is the reason.
However I would also prefer if it wasn't split.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The string from e.getMessage() contains e.getClientError().getMessage().

We don't need to expose e.getClientError().getMessage() and leave it as simply a method for error response type-agnostic retrieval of message.

No strong opinion.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We don't need to expose e.getClientError().getMessage()

I was not planning to challenge visibility of this method in this PR. 🤔

private ClientError clientError;

/**
* The original HTTP response that caused this exception, if available.
*
* @since 1.10.0
*/
@Nullable
@Getter(onMethod_ = @Beta, value = AccessLevel.PUBLIC)
private ClassicHttpResponse httpResponse;

/**
* The original HTTP request that caused this exception, if available.
*
* @since 1.10.0
*/
@Nullable
@Getter(onMethod_ = @Beta, value = AccessLevel.PUBLIC)
private ClassicHttpRequest httpRequest;

/**
* Sets the original HTTP request that caused this exception.
*
* @param clientError the original structured error payload received from the remote service, can
* be null if not available.
* @return the current instance of {@link ClientException} with the changed ClientError data
* @param <T> the type of the exception, typically a subclass of {@link ClientException}
*/
@SuppressWarnings("unchecked")
@Nonnull
public <T extends ClientException> T setClientError(@Nullable final ClientError clientError) {
this.clientError = clientError;
return (T) this;
}

/**
* Sets the original HTTP request that caused this exception.
*
* @param httpResponse the original HTTP response that caused this exception, can be null if not
* available.
* @return the current instance of {@link ClientException} with the changed HTTP response
* @param <T> the type of the exception, typically a subclass of {@link ClientException}
*/
@SuppressWarnings("unchecked")
@Nonnull
public <T extends ClientException> T setHttpResponse(
@Nullable final ClassicHttpResponse httpResponse) {
this.httpResponse = httpResponse;
return (T) this;
}

/**
* Sets the original HTTP request that caused this exception.
*
* @param httpRequest the original HTTP request that caused this exception, can be null if not
* available.
* @return the current instance of {@link ClientException} with the changed HTTP request
* @param <T> the type of the exception, typically a subclass of {@link ClientException}
*/
@SuppressWarnings("unchecked")
@Nonnull
public <T extends ClientException> T setHttpRequest(
@Nullable final ClassicHttpRequest httpRequest) {
this.httpRequest = httpRequest;
return (T) this;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
* @param <R> The subtype of {@link ClientError} payload that can be processed by this factory.
*/
@Beta
@FunctionalInterface
public interface ClientExceptionFactory<E extends ClientException, R extends ClientError> {

/**
Expand All @@ -22,16 +23,34 @@ public interface ClientExceptionFactory<E extends ClientException, R extends Cli
* @return An instance of the specified {@link ClientException} type
*/
@Nonnull
E build(@Nonnull final String message, @Nullable final Throwable cause);
default E build(@Nonnull final String message, @Nullable final Throwable cause) {
return build(message, null, cause);
}

/**
* Creates an exception with a message and optional cause.
*
* @param message A descriptive message for the exception.
* @return An instance of the specified {@link ClientException} type
*/
@Nonnull
default E build(@Nonnull final String message) {
return build(message, null);
}

/**
* Creates an exception from a given message and an HTTP error response that has been successfully
* deserialized into a {@link ClientError} object.
*
* @param message A descriptive message for the exception.
* @param clientError The structured {@link ClientError} object deserialized from the response.
* @param clientError The structured {@link ClientError} object deserialized from the response,
* null if not exist.
* @param cause An optional cause of the exception, can be null if not applicable.
* @return An instance of the specified {@link ClientException} type
*/
@Nonnull
E buildFromClientError(@Nonnull final String message, @Nonnull final R clientError);
E build(
@Nonnull final String message,
@Nullable final R clientError,
@Nullable final Throwable cause);
}
Original file line number Diff line number Diff line change
Expand Up @@ -79,17 +79,18 @@ public T handleResponse(@Nonnull final ClassicHttpResponse response) throws E {
private T parseSuccess(@Nonnull final ClassicHttpResponse response) throws E {
final HttpEntity responseEntity = response.getEntity();
if (responseEntity == null) {
throw exceptionFactory.build("The HTTP Response is empty", null);
throw exceptionFactory.build("The HTTP Response is empty").setHttpResponse(response);
}

val message = "Failed to parse response entity.";
val content =
tryGetContent(responseEntity)
.getOrElseThrow(e -> exceptionFactory.build("Failed to parse response entity.", e));
.getOrElseThrow(e -> exceptionFactory.build(message, e).setHttpResponse(response));
try {
return objectMapper.readValue(content, successType);
} catch (final JsonProcessingException e) {
log.error("Failed to parse response to type {}", successType);
throw exceptionFactory.build("Failed to parse response", e);
throw exceptionFactory.build("Failed to parse response", e).setHttpResponse(response);
}
}

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

if (entity == null) {
val message = getErrorMessage(httpResponse, "The HTTP Response is empty");
throw exceptionFactory.build(message, null);
throw exceptionFactory.build(message).setHttpResponse(httpResponse);
}
val maybeContent = tryGetContent(entity);
if (maybeContent.isFailure()) {
val message = getErrorMessage(httpResponse, "Failed to read the response content");
val baseException = exceptionFactory.build(message, null);
val baseException = exceptionFactory.build(message).setHttpResponse(httpResponse);
baseException.addSuppressed(maybeContent.getCause());
throw baseException;
}
val content = maybeContent.get();
if (content == null || content.isBlank()) {
val message = getErrorMessage(httpResponse, "Empty or blank response content");
throw exceptionFactory.build(message, null);
throw exceptionFactory.build(message).setHttpResponse(httpResponse);
}

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

parseErrorResponseAndThrow(content, httpResponse);
Expand All @@ -151,20 +152,19 @@ protected void parseErrorResponseAndThrow(
val maybeClientError = Try.of(() -> objectMapper.readValue(content, errorType));
if (maybeClientError.isFailure()) {
val message = getErrorMessage(httpResponse, "Failed to parse the JSON error response");
val baseException = exceptionFactory.build(message, null);
val baseException = exceptionFactory.build(message).setHttpResponse(httpResponse);
baseException.addSuppressed(maybeClientError.getCause());
throw baseException;
}
final R clientError = maybeClientError.get();
val message = getErrorMessage(httpResponse, clientError.getMessage());
throw exceptionFactory.buildFromClientError(message, clientError);
throw exceptionFactory.build(message, clientError, null).setHttpResponse(httpResponse);
}

private static String getErrorMessage(
@Nonnull final ClassicHttpResponse httpResponse, @Nullable final String additionalMessage) {
@Nonnull final ClassicHttpResponse rsp, @Nullable final String additionalMessage) {
val baseErrorMessage =
"Request failed with status %d (%s)"
.formatted(httpResponse.getCode(), httpResponse.getReasonPhrase());
"Request failed with status %d (%s)".formatted(rsp.getCode(), rsp.getReasonPhrase());

val message = Optional.ofNullable(additionalMessage).orElse("");
return message.isEmpty() ? baseErrorMessage : "%s: %s".formatted(baseErrorMessage, message);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,14 +63,14 @@ public Stream<D> handleStreamingResponse(@Nonnull final ClassicHttpResponse resp
super.buildAndThrowException(response);
}

return IterableStreamConverter.lines(response.getEntity(), exceptionFactory)
return IterableStreamConverter.lines(response, exceptionFactory)
// half of the lines are empty newlines, the last line is "data: [DONE]"
.filter(line -> !line.isEmpty() && !"data: [DONE]".equals(line.trim()))
.peek(
line -> {
if (!line.startsWith("data: ")) {
final String msg = "Failed to parse response";
throw exceptionFactory.build(msg, null);
throw exceptionFactory.build(msg).setHttpResponse(response);
}
})
.map(
Expand All @@ -80,7 +80,8 @@ public Stream<D> handleStreamingResponse(@Nonnull final ClassicHttpResponse resp
return objectMapper.readValue(data, successType);
} catch (final IOException e) { // exception message e gets lost
log.error("Failed to parse delta chunk to type {}", successType);
throw exceptionFactory.build("Failed to parse delta chunk", e);
final String message = "Failed to parse delta chunk";
throw exceptionFactory.build(message, e).setHttpResponse(response);
}
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@

import io.vavr.control.Try;
import java.io.BufferedReader;
import java.io.IOException;
import java.io.InputStream;
import java.io.InputStreamReader;
import java.util.Iterator;
import java.util.NoSuchElementException;
Expand All @@ -17,11 +15,10 @@
import java.util.stream.Stream;
import java.util.stream.StreamSupport;
import javax.annotation.Nonnull;
import javax.annotation.Nullable;
import lombok.AccessLevel;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
import org.apache.hc.core5.http.HttpEntity;
import org.apache.hc.core5.http.ClassicHttpResponse;

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

private static final String ERR_CONTENT = "Failed to read response content.";
private static final String ERR_INTERRUPTED = "Parsing response content was interrupted";
private static final String ERR_CLOSE = "Could not close input stream with error: {} (ignored)";

/** Read next entry for Stream or {@code null} when no further entry can be read. */
private final Callable<T> readHandler;

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

/** Error handler to be called when Stream is interrupted. */
private final Function<Exception, RuntimeException> errorHandler;
private final Function<Throwable, RuntimeException> errorHandler;

private boolean isDone = false;
private boolean isNextFetched = false;
Expand Down Expand Up @@ -86,44 +87,44 @@ public T next() {

/**
* Create a sequential Stream of lines from an HTTP response string (UTF-8). The underlying {@link
* InputStream} is closed, when the resulting Stream is closed (e.g. via try-with-resources) or
* when an exception occurred.
* java.io.InputStream} is closed, when the resulting Stream is closed (e.g. via
* try-with-resources) or when an exception occurred.
*
* @param entity The HTTP entity object.
* @param response The HTTP response object.
* @param exceptionFactory The exception factory to use for creating exceptions.
* @return A sequential Stream object.
* @throws ClientException if the provided HTTP entity object is {@code null} or empty.
*/
@SuppressWarnings("PMD.CloseResource") // Stream is closed automatically when consumed
@Nonnull
static <R extends ClientError> Stream<String> lines(
@Nullable final HttpEntity entity,
@Nonnull final ClassicHttpResponse response,
@Nonnull final ClientExceptionFactory<? extends ClientException, R> exceptionFactory)
throws ClientException {
if (entity == null) {
throw exceptionFactory.build("The HTTP Response is empty", null);
if (response.getEntity() == null) {
throw exceptionFactory.build("The HTTP Response is empty").setHttpResponse(response);
}

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

// initialize buffered reader
final var reader = new BufferedReader(new InputStreamReader(inputStream, UTF_8), BUFFER_SIZE);

// define close handler
final Runnable closeHandler =
() ->
Try.run(reader::close)
.onFailure(
e ->
log.debug(
"Could not close input stream with error: {} (ignored)",
e.getClass().getSimpleName()));
final Function<Exception, RuntimeException> errHandler =
e -> exceptionFactory.build("Parsing response content was interrupted", e);
() -> Try.run(reader::close).onFailure(e -> log.debug(ERR_CLOSE, e.getClass()));

// define error handler
final Function<Throwable, RuntimeException> errHandler =
e -> exceptionFactory.build(ERR_INTERRUPTED, e).setHttpResponse(response);

// initialize lazy stream iterator
final var iterator = new IterableStreamConverter<>(reader::readLine, closeHandler, errHandler);

// create lazy stream as output
final var spliterator = Spliterators.spliteratorUnknownSize(iterator, ORDERED | NONNULL);
return StreamSupport.stream(spliterator, /* NOT PARALLEL */ false).onClose(closeHandler);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import com.fasterxml.jackson.core.JsonParseException;
import java.io.IOException;
import javax.annotation.Nonnull;
import javax.annotation.Nullable;
import lombok.Data;
import lombok.SneakyThrows;
import lombok.experimental.StandardException;
Expand All @@ -35,16 +36,11 @@ static class MyException extends ClientException {}
static class MyExceptionFactory implements ClientExceptionFactory<MyException, MyError> {
@Nonnull
@Override
public MyException build(@Nonnull String message, Throwable cause) {
return new MyException(message, cause);
}

@Nonnull
@Override
public MyException buildFromClientError(@Nonnull String message, @Nonnull MyError clientError) {
var ex = new MyException(message);
ex.clientError = clientError;
return ex;
public MyException build(
@Nonnull final String message,
@Nullable final MyError clientError,
@Nullable final Throwable cause) {
return new MyException(message, cause).setClientError(clientError);
}
}

Expand Down
Loading