diff --git a/core/src/main/java/com/sap/ai/sdk/core/common/ClientException.java b/core/src/main/java/com/sap/ai/sdk/core/common/ClientException.java index cd311993f..251bb9c00 100644 --- a/core/src/main/java/com/sap/ai/sdk/core/common/ClientException.java +++ b/core/src/main/java/com/sap/ai/sdk/core/common/ClientException.java @@ -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. @@ -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) + 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 the type of the exception, typically a subclass of {@link ClientException} + */ + @SuppressWarnings("unchecked") + @Nonnull + public 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 the type of the exception, typically a subclass of {@link ClientException} + */ + @SuppressWarnings("unchecked") + @Nonnull + public 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 the type of the exception, typically a subclass of {@link ClientException} + */ + @SuppressWarnings("unchecked") + @Nonnull + public T setHttpRequest( + @Nullable final ClassicHttpRequest httpRequest) { + this.httpRequest = httpRequest; + return (T) this; + } } diff --git a/core/src/main/java/com/sap/ai/sdk/core/common/ClientExceptionFactory.java b/core/src/main/java/com/sap/ai/sdk/core/common/ClientExceptionFactory.java index 991c11c52..bfe03379b 100644 --- a/core/src/main/java/com/sap/ai/sdk/core/common/ClientExceptionFactory.java +++ b/core/src/main/java/com/sap/ai/sdk/core/common/ClientExceptionFactory.java @@ -12,6 +12,7 @@ * @param The subtype of {@link ClientError} payload that can be processed by this factory. */ @Beta +@FunctionalInterface public interface ClientExceptionFactory { /** @@ -22,16 +23,34 @@ public interface ClientExceptionFactory 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); } } @@ -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( @@ -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); @@ -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); diff --git a/core/src/main/java/com/sap/ai/sdk/core/common/ClientStreamingHandler.java b/core/src/main/java/com/sap/ai/sdk/core/common/ClientStreamingHandler.java index 280c51953..f86b072e1 100644 --- a/core/src/main/java/com/sap/ai/sdk/core/common/ClientStreamingHandler.java +++ b/core/src/main/java/com/sap/ai/sdk/core/common/ClientStreamingHandler.java @@ -63,14 +63,14 @@ public Stream 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( @@ -80,7 +80,8 @@ public Stream 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); } }); } diff --git a/core/src/main/java/com/sap/ai/sdk/core/common/IterableStreamConverter.java b/core/src/main/java/com/sap/ai/sdk/core/common/IterableStreamConverter.java index fb06af6fc..12ec15fac 100644 --- a/core/src/main/java/com/sap/ai/sdk/core/common/IterableStreamConverter.java +++ b/core/src/main/java/com/sap/ai/sdk/core/common/IterableStreamConverter.java @@ -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; @@ -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}. @@ -37,6 +34,10 @@ class IterableStreamConverter implements Iterator { /** 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 readHandler; @@ -44,7 +45,7 @@ class IterableStreamConverter implements Iterator { private final Runnable stopHandler; /** Error handler to be called when Stream is interrupted. */ - private final Function errorHandler; + private final Function errorHandler; private boolean isDone = false; private boolean isNextFetched = false; @@ -86,10 +87,10 @@ 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. @@ -97,33 +98,33 @@ public T next() { @SuppressWarnings("PMD.CloseResource") // Stream is closed automatically when consumed @Nonnull static Stream lines( - @Nullable final HttpEntity entity, + @Nonnull final ClassicHttpResponse response, @Nonnull final ClientExceptionFactory 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 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 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); } diff --git a/core/src/test/java/com/sap/ai/sdk/core/common/ClientResponseHandlerTest.java b/core/src/test/java/com/sap/ai/sdk/core/common/ClientResponseHandlerTest.java index 32bb4495b..f81652a98 100644 --- a/core/src/test/java/com/sap/ai/sdk/core/common/ClientResponseHandlerTest.java +++ b/core/src/test/java/com/sap/ai/sdk/core/common/ClientResponseHandlerTest.java @@ -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; @@ -35,16 +36,11 @@ static class MyException extends ClientException {} static class MyExceptionFactory implements ClientExceptionFactory { @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); } } diff --git a/core/src/test/java/com/sap/ai/sdk/core/common/ClientStreamingHandlerTest.java b/core/src/test/java/com/sap/ai/sdk/core/common/ClientStreamingHandlerTest.java index 1b36bf6e2..370cc8616 100644 --- a/core/src/test/java/com/sap/ai/sdk/core/common/ClientStreamingHandlerTest.java +++ b/core/src/test/java/com/sap/ai/sdk/core/common/ClientStreamingHandlerTest.java @@ -79,11 +79,7 @@ void testHandleStreamingResponse() { """; var response = spy(new BasicClassicHttpResponse(200, "OK")); - when(response.getEntity()) - .thenReturn(new StringEntity(validStreamContent)) - .thenReturn(new StringEntity(emptyStreamContent)) - .thenReturn(new StringEntity(malformedLineContent)) - .thenReturn(new StringEntity(invalidJsonContent)); + when(response.getEntity()).thenReturn(new StringEntity(validStreamContent)); var stream1 = sut.handleStreamingResponse(response); var deltas1 = stream1.toList(); @@ -93,14 +89,17 @@ void testHandleStreamingResponse() { assertThat(deltas1.get(1).getDeltaContent()).isEqualTo("delta2"); assertThat(deltas1.get(1).getFinishReason()).isEqualTo("length"); + when(response.getEntity()).thenReturn(new StringEntity(emptyStreamContent)); var stream2 = sut.handleStreamingResponse(response); assertThat(stream2).isEmpty(); + when(response.getEntity()).thenReturn(new StringEntity(malformedLineContent)); var stream3 = sut.handleStreamingResponse(response); assertThatThrownBy(stream3::toList) .isInstanceOf(MyException.class) .hasMessageContaining("Failed to parse response"); + when(response.getEntity()).thenReturn(new StringEntity(invalidJsonContent)); var stream4 = sut.handleStreamingResponse(response); assertThatThrownBy(stream4::toList) .isInstanceOf(MyException.class) diff --git a/core/src/test/java/com/sap/ai/sdk/core/common/IterableStreamConverterTest.java b/core/src/test/java/com/sap/ai/sdk/core/common/IterableStreamConverterTest.java index 355b523a7..3823849d2 100644 --- a/core/src/test/java/com/sap/ai/sdk/core/common/IterableStreamConverterTest.java +++ b/core/src/test/java/com/sap/ai/sdk/core/common/IterableStreamConverterTest.java @@ -18,10 +18,12 @@ import java.nio.charset.StandardCharsets; import java.util.concurrent.atomic.AtomicInteger; import javax.annotation.Nonnull; +import javax.annotation.Nullable; import lombok.SneakyThrows; import lombok.experimental.StandardException; import org.apache.hc.core5.http.ContentType; import org.apache.hc.core5.http.io.entity.InputStreamEntity; +import org.apache.hc.core5.http.message.BasicClassicHttpResponse; import org.junit.jupiter.api.DisplayName; import org.junit.jupiter.api.Test; @@ -34,8 +36,10 @@ void testLines() { final var input = TEMPLATE.repeat(IterableStreamConverter.BUFFER_SIZE); final var inputStream = spy(new ByteArrayInputStream(input.getBytes(StandardCharsets.UTF_8))); final var entity = new InputStreamEntity(inputStream, ContentType.TEXT_PLAIN); + final var response = new BasicClassicHttpResponse(200, "OK"); + response.setEntity(entity); - final var sut = IterableStreamConverter.lines(entity, new TestClientExceptionFactory()); + final var sut = IterableStreamConverter.lines(response, new TestClientExceptionFactory()); verify(inputStream, never()).read(); verify(inputStream, never()).read(any()); verify(inputStream, never()).read(any(), anyInt(), anyInt()); @@ -70,8 +74,10 @@ void testLinesFindFirst() { }); final var entity = new InputStreamEntity(inputStream, ContentType.TEXT_PLAIN); + final var response = new BasicClassicHttpResponse(200, "OK"); + response.setEntity(entity); - final var sut = IterableStreamConverter.lines(entity, new TestClientExceptionFactory()); + final var sut = IterableStreamConverter.lines(response, new TestClientExceptionFactory()); assertThat(sut.findFirst()).contains("Foo Bar"); verify(inputStream, times(1)).read(any(), anyInt(), anyInt()); verify(inputStream, never()).close(); @@ -94,8 +100,10 @@ void testLinesThrows() { .thenThrow(new IOException("Ups!")); final var entity = new InputStreamEntity(inputStream, ContentType.TEXT_PLAIN); + final var response = new BasicClassicHttpResponse(200, "OK"); + response.setEntity(entity); - final var sut = IterableStreamConverter.lines(entity, new TestClientExceptionFactory()); + final var sut = IterableStreamConverter.lines(response, new TestClientExceptionFactory()); assertThatThrownBy(sut::count) .isInstanceOf(TestClientException.class) .hasMessage("Parsing response content was interrupted") @@ -114,17 +122,11 @@ static class TestClientExceptionFactory @Nonnull @Override - public TestClientException build(@Nonnull String message, Throwable cause) { - return new TestClientException(message, cause); - } - - @Nonnull - @Override - public TestClientException buildFromClientError( - @Nonnull String message, @Nonnull ClientError clientError) { - TestClientException exception = new TestClientException(message); - exception.clientError = clientError; - return exception; + public TestClientException build( + @Nonnull final String message, + @Nullable final ClientError clientError, + @Nullable final Throwable cause) { + return new TestClientException(message, cause).setClientError(clientError); } } } diff --git a/docs/release_notes.md b/docs/release_notes.md index 5745d6681..a4702e9ca 100644 --- a/docs/release_notes.md +++ b/docs/release_notes.md @@ -12,7 +12,10 @@ ### ✨ New Functionality -- +- Extend `OpenAiClientException` and `OrchestrationClientException` to retrieve error diagnostics information received from remote service. + New available accessors for troubleshooting: `getErrorResponse()`, `getHttpResponse()` and, `getHttpRequest()`. + Please note: depending on the error response, these methods may return `null` if the information is not available. + ### 📈 Improvements diff --git a/foundation-models/openai/src/main/java/com/sap/ai/sdk/foundationmodels/openai/OpenAiClient.java b/foundation-models/openai/src/main/java/com/sap/ai/sdk/foundationmodels/openai/OpenAiClient.java index 71416711a..7ce79dd16 100644 --- a/foundation-models/openai/src/main/java/com/sap/ai/sdk/foundationmodels/openai/OpenAiClient.java +++ b/foundation-models/openai/src/main/java/com/sap/ai/sdk/foundationmodels/openai/OpenAiClient.java @@ -1,5 +1,6 @@ package com.sap.ai.sdk.foundationmodels.openai; +import static com.sap.ai.sdk.foundationmodels.openai.OpenAiClientException.FACTORY; import static com.sap.ai.sdk.foundationmodels.openai.OpenAiUtils.getOpenAiObjectMapper; import com.fasterxml.jackson.core.JsonProcessingException; @@ -42,7 +43,8 @@ @RequiredArgsConstructor(access = AccessLevel.PRIVATE) public final class OpenAiClient { private static final String DEFAULT_API_VERSION = "2024-02-01"; - static final ObjectMapper JACKSON = getOpenAiObjectMapper(); + + private static final ObjectMapper JACKSON = getOpenAiObjectMapper(); @Nullable private String systemPrompt = null; @@ -420,7 +422,8 @@ private static void serializeAndSetHttpEntity( final var json = JACKSON.writeValueAsString(payload); request.setEntity(new StringEntity(json, ContentType.APPLICATION_JSON)); } catch (final JsonProcessingException e) { - throw new OpenAiClientException("Failed to serialize request parameters", e); + throw new OpenAiClientException("Failed to serialize request parameters", e) + .setHttpRequest(request); } } @@ -430,11 +433,9 @@ private T executeRequest( try { final var client = ApacheHttpClient5Accessor.getHttpClient(destination); return client.execute( - request, - new ClientResponseHandler<>( - responseType, OpenAiError.class, new OpenAiExceptionFactory())); + request, new ClientResponseHandler<>(responseType, OpenAiError.class, FACTORY)); } catch (final IOException e) { - throw new OpenAiClientException("Request to OpenAI model failed", e); + throw new OpenAiClientException("Request to OpenAI model failed", e).setHttpRequest(request); } } @@ -443,12 +444,11 @@ private Stream streamRequest( final BasicClassicHttpRequest request, @Nonnull final Class deltaType) { try { final var client = ApacheHttpClient5Accessor.getHttpClient(destination); - return new ClientStreamingHandler<>( - deltaType, OpenAiError.class, new OpenAiExceptionFactory()) + return new ClientStreamingHandler<>(deltaType, OpenAiError.class, FACTORY) .objectMapper(JACKSON) .handleStreamingResponse(client.executeOpen(null, request, null)); } catch (final IOException e) { - throw new OpenAiClientException("Request to OpenAI model failed", e); + throw new OpenAiClientException("Request to OpenAI model failed", e).setHttpRequest(request); } } } diff --git a/foundation-models/openai/src/main/java/com/sap/ai/sdk/foundationmodels/openai/OpenAiClientException.java b/foundation-models/openai/src/main/java/com/sap/ai/sdk/foundationmodels/openai/OpenAiClientException.java index ca3e9d50a..296e1a6a1 100644 --- a/foundation-models/openai/src/main/java/com/sap/ai/sdk/foundationmodels/openai/OpenAiClientException.java +++ b/foundation-models/openai/src/main/java/com/sap/ai/sdk/foundationmodels/openai/OpenAiClientException.java @@ -2,18 +2,17 @@ import com.google.common.annotations.Beta; import com.sap.ai.sdk.core.common.ClientException; +import com.sap.ai.sdk.core.common.ClientExceptionFactory; import com.sap.ai.sdk.foundationmodels.openai.generated.model.ErrorResponse; -import javax.annotation.Nonnull; import javax.annotation.Nullable; import lombok.experimental.StandardException; /** Generic exception for errors occurring when using OpenAI foundation models. */ @StandardException public class OpenAiClientException extends ClientException { - OpenAiClientException(@Nonnull final String message, @Nonnull final OpenAiError clientError) { - super(message); - setClientError(clientError); - } + + static final ClientExceptionFactory FACTORY = + (message, error, cause) -> new OpenAiClientException(message, cause).setClientError(error); /** * Retrieves the {@link ErrorResponse} from the OpenAI service, if available. diff --git a/foundation-models/openai/src/main/java/com/sap/ai/sdk/foundationmodels/openai/OpenAiExceptionFactory.java b/foundation-models/openai/src/main/java/com/sap/ai/sdk/foundationmodels/openai/OpenAiExceptionFactory.java deleted file mode 100644 index 4cdf65af7..000000000 --- a/foundation-models/openai/src/main/java/com/sap/ai/sdk/foundationmodels/openai/OpenAiExceptionFactory.java +++ /dev/null @@ -1,23 +0,0 @@ -package com.sap.ai.sdk.foundationmodels.openai; - -import com.google.common.annotations.Beta; -import com.sap.ai.sdk.core.common.ClientExceptionFactory; -import javax.annotation.Nonnull; -import javax.annotation.Nullable; - -@Beta -class OpenAiExceptionFactory implements ClientExceptionFactory { - - @Nonnull - public OpenAiClientException build( - @Nonnull final String message, @Nullable final Throwable cause) { - return new OpenAiClientException(message, cause); - } - - @Nonnull - @Override - public OpenAiClientException buildFromClientError( - @Nonnull final String message, @Nonnull final OpenAiError openAiError) { - return new OpenAiClientException(message, openAiError); - } -} diff --git a/foundation-models/openai/src/test/java/com/sap/ai/sdk/foundationmodels/openai/BaseOpenAiClientTest.java b/foundation-models/openai/src/test/java/com/sap/ai/sdk/foundationmodels/openai/BaseOpenAiClientTest.java index d1c97f8c7..342238380 100644 --- a/foundation-models/openai/src/test/java/com/sap/ai/sdk/foundationmodels/openai/BaseOpenAiClientTest.java +++ b/foundation-models/openai/src/test/java/com/sap/ai/sdk/foundationmodels/openai/BaseOpenAiClientTest.java @@ -10,6 +10,7 @@ import static com.github.tomakehurst.wiremock.client.WireMock.serverError; import static com.github.tomakehurst.wiremock.client.WireMock.stubFor; import static com.github.tomakehurst.wiremock.client.WireMock.urlPathEqualTo; +import static org.assertj.core.api.Assertions.assertThat; import static org.mockito.ArgumentMatchers.any; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; @@ -117,7 +118,8 @@ static void assertForErrorHandling(@Nonnull final Runnable request) { .assertThatThrownBy(request::run) .describedAs("Server errors should be handled") .isInstanceOf(OpenAiClientException.class) - .hasMessageContaining("500"); + .hasMessageContaining("500") + .satisfies(e -> assertThat(((OpenAiClientException) e).getHttpResponse()).isNotNull()); softly .assertThatThrownBy(request::run) diff --git a/orchestration/src/main/java/com/sap/ai/sdk/orchestration/OrchestrationChatResponse.java b/orchestration/src/main/java/com/sap/ai/sdk/orchestration/OrchestrationChatResponse.java index ac8764856..100291936 100644 --- a/orchestration/src/main/java/com/sap/ai/sdk/orchestration/OrchestrationChatResponse.java +++ b/orchestration/src/main/java/com/sap/ai/sdk/orchestration/OrchestrationChatResponse.java @@ -44,7 +44,7 @@ public String getContent() throws OrchestrationFilterException.Output { if ("content_filter".equals(choice.getFinishReason())) { final var filterDetails = Try.of(this::getOutputFilteringChoices).getOrElseGet(e -> Map.of()); final var message = "Content filter filtered the output."; - throw new OrchestrationFilterException.Output(message, filterDetails); + throw new OrchestrationFilterException.Output(message).setFilterDetails(filterDetails); } return choice.getMessage().getContent(); } diff --git a/orchestration/src/main/java/com/sap/ai/sdk/orchestration/OrchestrationClient.java b/orchestration/src/main/java/com/sap/ai/sdk/orchestration/OrchestrationClient.java index 0606499a3..d7e165cdb 100644 --- a/orchestration/src/main/java/com/sap/ai/sdk/orchestration/OrchestrationClient.java +++ b/orchestration/src/main/java/com/sap/ai/sdk/orchestration/OrchestrationClient.java @@ -122,7 +122,7 @@ private static void throwOnContentFilter(@Nonnull final OrchestrationChatComplet final var filterDetails = Try.of(() -> getOutputFilteringChoices(delta)).getOrElseGet(e -> Map.of()); final var message = "Content filter filtered the output."; - throw new OrchestrationFilterException.Output(message, filterDetails); + throw new OrchestrationFilterException.Output(message).setFilterDetails(filterDetails); } } diff --git a/orchestration/src/main/java/com/sap/ai/sdk/orchestration/OrchestrationClientException.java b/orchestration/src/main/java/com/sap/ai/sdk/orchestration/OrchestrationClientException.java index cc8097e45..350679e4d 100644 --- a/orchestration/src/main/java/com/sap/ai/sdk/orchestration/OrchestrationClientException.java +++ b/orchestration/src/main/java/com/sap/ai/sdk/orchestration/OrchestrationClientException.java @@ -2,9 +2,17 @@ import com.google.common.annotations.Beta; import com.sap.ai.sdk.core.common.ClientException; +import com.sap.ai.sdk.core.common.ClientExceptionFactory; +import com.sap.ai.sdk.orchestration.OrchestrationFilterException.Input; import com.sap.ai.sdk.orchestration.model.Error; import com.sap.ai.sdk.orchestration.model.ErrorResponse; import com.sap.ai.sdk.orchestration.model.ErrorResponseStreaming; +import com.sap.ai.sdk.orchestration.model.ErrorStreaming; +import com.sap.ai.sdk.orchestration.model.GenericModuleResult; +import com.sap.ai.sdk.orchestration.model.ModuleResults; +import com.sap.ai.sdk.orchestration.model.ModuleResultsStreaming; +import java.util.Collections; +import java.util.Map; import java.util.Optional; import javax.annotation.Nonnull; import javax.annotation.Nullable; @@ -14,10 +22,43 @@ @StandardException public class OrchestrationClientException extends ClientException { - OrchestrationClientException( - @Nonnull final String message, @Nonnull final OrchestrationError clientError) { - super(message); - setClientError(clientError); + static final ClientExceptionFactory FACTORY = + (message, clientError, cause) -> { + final var details = extractInputFilterDetails(clientError); + if (details.isEmpty()) { + return new OrchestrationClientException(message, cause).setClientError(clientError); + } + return new Input(message, cause).setFilterDetails(details).setClientError(clientError); + }; + + @SuppressWarnings("unchecked") + @Nonnull + static Map extractInputFilterDetails(@Nullable final OrchestrationError error) { + if (error instanceof OrchestrationError.Synchronous synchronousError) { + return Optional.of(synchronousError.getErrorResponse()) + .map(ErrorResponse::getError) + .map(Error::getIntermediateResults) + .map(ModuleResults::getInputFiltering) + .map(GenericModuleResult::getData) + .map(map -> (Map) map) + .orElseGet(Collections::emptyMap); + } else if (error instanceof OrchestrationError.Streaming streamingError) { + return Optional.of(streamingError.getErrorResponse()) + .map(ErrorResponseStreaming::getError) + .map(ErrorStreaming::getIntermediateResults) + .map(ModuleResultsStreaming::getInputFiltering) + .map(GenericModuleResult::getData) + .filter(Map.class::isInstance) + .map(map -> (Map) map) + .orElseGet(Collections::emptyMap); + } + return Collections.emptyMap(); + } + + @Override + @Nullable + public OrchestrationError getClientError() { + return (OrchestrationError) super.getClientError(); } /** @@ -29,8 +70,7 @@ public class OrchestrationClientException extends ClientException { @Beta @Nullable public ErrorResponse getErrorResponse() { - final var clientError = super.getClientError(); - if (clientError instanceof OrchestrationError.Synchronous orchestrationError) { + if (getClientError() instanceof OrchestrationError.Synchronous orchestrationError) { return orchestrationError.getErrorResponse(); } return null; @@ -45,8 +85,7 @@ public ErrorResponse getErrorResponse() { @Beta @Nullable public ErrorResponseStreaming getErrorResponseStreaming() { - final var clientError = super.getClientError(); - if (clientError instanceof OrchestrationError.Streaming orchestrationError) { + if (getClientError() instanceof OrchestrationError.Streaming orchestrationError) { return orchestrationError.getErrorResponse(); } return null; diff --git a/orchestration/src/main/java/com/sap/ai/sdk/orchestration/OrchestrationError.java b/orchestration/src/main/java/com/sap/ai/sdk/orchestration/OrchestrationError.java index ad6b9256e..c33eccb4f 100644 --- a/orchestration/src/main/java/com/sap/ai/sdk/orchestration/OrchestrationError.java +++ b/orchestration/src/main/java/com/sap/ai/sdk/orchestration/OrchestrationError.java @@ -19,7 +19,6 @@ */ @Beta public interface OrchestrationError extends ClientError { - /** * Orchestration error response for synchronous requests. * @@ -30,11 +29,7 @@ public interface OrchestrationError extends ClientError { class Synchronous implements OrchestrationError { ErrorResponse errorResponse; - /** - * Gets the error message from the contained original response. - * - * @return the error message - */ + @Override @Nonnull public String getMessage() { final Error e = errorResponse.getError(); @@ -53,11 +48,7 @@ public String getMessage() { class Streaming implements OrchestrationError { ErrorResponseStreaming errorResponse; - /** - * Gets the error message from the contained original response. - * - * @return the error message - */ + @Override @Nonnull public String getMessage() { final ErrorStreaming e = errorResponse.getError(); diff --git a/orchestration/src/main/java/com/sap/ai/sdk/orchestration/OrchestrationExceptionFactory.java b/orchestration/src/main/java/com/sap/ai/sdk/orchestration/OrchestrationExceptionFactory.java deleted file mode 100644 index 756bd1451..000000000 --- a/orchestration/src/main/java/com/sap/ai/sdk/orchestration/OrchestrationExceptionFactory.java +++ /dev/null @@ -1,66 +0,0 @@ -package com.sap.ai.sdk.orchestration; - -import com.google.common.annotations.Beta; -import com.sap.ai.sdk.core.common.ClientExceptionFactory; -import com.sap.ai.sdk.orchestration.model.Error; -import com.sap.ai.sdk.orchestration.model.ErrorResponse; -import com.sap.ai.sdk.orchestration.model.ErrorResponseStreaming; -import com.sap.ai.sdk.orchestration.model.ErrorStreaming; -import com.sap.ai.sdk.orchestration.model.GenericModuleResult; -import com.sap.ai.sdk.orchestration.model.ModuleResults; -import com.sap.ai.sdk.orchestration.model.ModuleResultsStreaming; -import java.util.Collections; -import java.util.Map; -import java.util.Optional; -import javax.annotation.Nonnull; -import javax.annotation.Nullable; - -@Beta -class OrchestrationExceptionFactory - implements ClientExceptionFactory { - - @Nonnull - public OrchestrationClientException build( - @Nonnull final String message, @Nullable final Throwable cause) { - return new OrchestrationClientException(message, cause); - } - - @Nonnull - @Override - public OrchestrationClientException buildFromClientError( - @Nonnull final String message, @Nonnull final OrchestrationError clientError) { - - final var inputFilterDetails = extractInputFilterDetails(clientError); - if (!inputFilterDetails.isEmpty()) { - return new OrchestrationFilterException.Input(message, clientError, inputFilterDetails); - } - - return new OrchestrationClientException(message, clientError); - } - - @SuppressWarnings("unchecked") - @Nonnull - private Map extractInputFilterDetails(@Nonnull final OrchestrationError error) { - - if (error instanceof OrchestrationError.Synchronous synchronousError) { - return Optional.of(synchronousError.getErrorResponse()) - .map(ErrorResponse::getError) - .map(Error::getIntermediateResults) - .map(ModuleResults::getInputFiltering) - .map(GenericModuleResult::getData) - .filter(Map.class::isInstance) - .map(map -> (Map) map) - .orElseGet(Collections::emptyMap); - } else if (error instanceof OrchestrationError.Streaming streamingError) { - return Optional.of(streamingError.getErrorResponse()) - .map(ErrorResponseStreaming::getError) - .map(ErrorStreaming::getIntermediateResults) - .map(ModuleResultsStreaming::getInputFiltering) - .map(GenericModuleResult::getData) - .filter(Map.class::isInstance) - .map(map -> (Map) map) - .orElseGet(Collections::emptyMap); - } - return Collections.emptyMap(); - } -} diff --git a/orchestration/src/main/java/com/sap/ai/sdk/orchestration/OrchestrationFilterException.java b/orchestration/src/main/java/com/sap/ai/sdk/orchestration/OrchestrationFilterException.java index 13a83a117..c62d74fa2 100644 --- a/orchestration/src/main/java/com/sap/ai/sdk/orchestration/OrchestrationFilterException.java +++ b/orchestration/src/main/java/com/sap/ai/sdk/orchestration/OrchestrationFilterException.java @@ -12,6 +12,8 @@ import javax.annotation.Nullable; import lombok.AccessLevel; import lombok.Getter; +import lombok.Setter; +import lombok.experimental.Accessors; import lombok.experimental.StandardException; /** Base exception for errors occurring during orchestration filtering. */ @@ -20,7 +22,11 @@ public class OrchestrationFilterException extends OrchestrationClientException { /** Details about the filters that caused the exception. */ - @Getter @Nonnull protected Map filterDetails = Map.of(); + @Accessors(chain = true) + @Setter(AccessLevel.PACKAGE) + @Getter + @Nonnull + protected Map filterDetails = Map.of(); /** * Retrieves LlamaGuard 3.8b details from {@code filterDetails}, if present. @@ -37,22 +43,8 @@ public LlamaGuard38b getLlamaGuard38b() { } /** Exception thrown when an error occurs during input filtering. */ + @StandardException public static class Input extends OrchestrationFilterException { - /** - * Constructs a new OrchestrationInputFilterException. - * - * @param message The detail message. - * @param clientError The specific client error. - * @param filterDetails Details about the filter that caused the exception. - */ - Input( - @Nonnull final String message, - @Nonnull final OrchestrationError clientError, - @Nonnull final Map filterDetails) { - super(message); - setClientError(clientError); - this.filterDetails = filterDetails; - } /** * Retrieves Azure Content Safety input details from {@code filterDetails}, if present. @@ -75,17 +67,8 @@ public AzureContentSafetyInput getAzureContentSafetyInput() { * Exception thrown when an error occurs during output filtering, specifically when the finish * reason is a content filter. */ + @StandardException public static class Output extends OrchestrationFilterException { - /** - * Constructs a new OrchestrationOutputFilterException. - * - * @param message The detail message. - * @param filterDetails Details about the filter that caused the exception. - */ - Output(@Nonnull final String message, @Nonnull final Map filterDetails) { - super(message); - this.filterDetails = filterDetails; - } /** * Retrieves Azure Content Safety output details from {@code filterDetails}, if present. diff --git a/orchestration/src/main/java/com/sap/ai/sdk/orchestration/OrchestrationHttpExecutor.java b/orchestration/src/main/java/com/sap/ai/sdk/orchestration/OrchestrationHttpExecutor.java index 80a9a43d2..59954c929 100644 --- a/orchestration/src/main/java/com/sap/ai/sdk/orchestration/OrchestrationHttpExecutor.java +++ b/orchestration/src/main/java/com/sap/ai/sdk/orchestration/OrchestrationHttpExecutor.java @@ -1,5 +1,6 @@ package com.sap.ai.sdk.orchestration; +import static com.sap.ai.sdk.orchestration.OrchestrationClientException.FACTORY; import static com.sap.ai.sdk.orchestration.OrchestrationJacksonConfiguration.getOrchestrationObjectMapper; import com.fasterxml.jackson.core.JsonProcessingException; @@ -26,6 +27,7 @@ @Slf4j class OrchestrationHttpExecutor { private final Supplier destinationSupplier; + private static final ObjectMapper JACKSON = getOrchestrationObjectMapper(); OrchestrationHttpExecutor(@Nonnull final Supplier destinationSupplier) @@ -47,10 +49,7 @@ T execute( val client = getHttpClient(); val handler = - new ClientResponseHandler<>( - responseType, - OrchestrationError.Synchronous.class, - new OrchestrationExceptionFactory()) + new ClientResponseHandler<>(responseType, OrchestrationError.Synchronous.class, FACTORY) .objectMapper(JACKSON); return client.execute(request, handler); @@ -77,9 +76,7 @@ Stream stream( val client = getHttpClient(); return new ClientStreamingHandler<>( - OrchestrationChatCompletionDelta.class, - OrchestrationError.Streaming.class, - new OrchestrationExceptionFactory()) + OrchestrationChatCompletionDelta.class, OrchestrationError.Streaming.class, FACTORY) .objectMapper(JACKSON) .handleStreamingResponse(client.executeOpen(null, request, null)); diff --git a/sample-code/spring-app/src/main/java/com/sap/ai/sdk/app/controllers/OpenAiController.java b/sample-code/spring-app/src/main/java/com/sap/ai/sdk/app/controllers/OpenAiController.java index 2abe83fb5..76a3415a1 100644 --- a/sample-code/spring-app/src/main/java/com/sap/ai/sdk/app/controllers/OpenAiController.java +++ b/sample-code/spring-app/src/main/java/com/sap/ai/sdk/app/controllers/OpenAiController.java @@ -7,7 +7,6 @@ import com.sap.ai.sdk.foundationmodels.openai.model.OpenAiUsage; import com.sap.cloud.sdk.cloudplatform.thread.ThreadContextExecutors; import java.io.IOException; -import java.util.Arrays; import java.util.concurrent.atomic.AtomicReference; import javax.annotation.Nonnull; import javax.annotation.Nullable; @@ -102,7 +101,7 @@ public static void send(@Nonnull final ResponseBodyEmitter emitter, @Nonnull fin try { emitter.send(chunk); } catch (final IOException e) { - log.error(Arrays.toString(e.getStackTrace())); + log.error("Failed to send chunk: {}", e.getMessage(), e); emitter.completeWithError(e); } }