From 7fbb4bc7ea49993c8cf2c4fab265cb34771d89bd Mon Sep 17 00:00:00 2001 From: Ido Berkovich Date: Sun, 26 Jan 2025 13:27:03 +0200 Subject: [PATCH 1/3] OPIK-853 handle gemini errors correctly --- .../opik/domain/llm/LlmProviderService.java | 2 +- .../llm/antropic/LlmProviderAnthropic.java | 4 +-- .../llm/gemini/LlmProviderGemini.java | 33 +++++++++++++++++-- .../llm/openai/LlmProviderOpenAi.java | 4 +-- 4 files changed, 36 insertions(+), 7 deletions(-) diff --git a/apps/opik-backend/src/main/java/com/comet/opik/domain/llm/LlmProviderService.java b/apps/opik-backend/src/main/java/com/comet/opik/domain/llm/LlmProviderService.java index 3ef78b8c5a..f4bf399756 100644 --- a/apps/opik-backend/src/main/java/com/comet/opik/domain/llm/LlmProviderService.java +++ b/apps/opik-backend/src/main/java/com/comet/opik/domain/llm/LlmProviderService.java @@ -22,5 +22,5 @@ void generateStream( void validateRequest(@NonNull ChatCompletionRequest request); - Optional getLlmProviderError(@NonNull Throwable runtimeException); + Optional getLlmProviderError(@NonNull Throwable throwable); } diff --git a/apps/opik-backend/src/main/java/com/comet/opik/infrastructure/llm/antropic/LlmProviderAnthropic.java b/apps/opik-backend/src/main/java/com/comet/opik/infrastructure/llm/antropic/LlmProviderAnthropic.java index 80ea278948..7458f377c5 100644 --- a/apps/opik-backend/src/main/java/com/comet/opik/infrastructure/llm/antropic/LlmProviderAnthropic.java +++ b/apps/opik-backend/src/main/java/com/comet/opik/infrastructure/llm/antropic/LlmProviderAnthropic.java @@ -57,8 +57,8 @@ public void validateRequest(@NonNull ChatCompletionRequest request) { } @Override - public Optional getLlmProviderError(@NonNull Throwable runtimeException) { - if (runtimeException instanceof AnthropicHttpException anthropicHttpException) { + public Optional getLlmProviderError(@NonNull Throwable throwable) { + if (throwable instanceof AnthropicHttpException anthropicHttpException) { return Optional.of(new ErrorMessage(anthropicHttpException.statusCode(), anthropicHttpException.getMessage())); } diff --git a/apps/opik-backend/src/main/java/com/comet/opik/infrastructure/llm/gemini/LlmProviderGemini.java b/apps/opik-backend/src/main/java/com/comet/opik/infrastructure/llm/gemini/LlmProviderGemini.java index 291f705b87..0df22e9a89 100644 --- a/apps/opik-backend/src/main/java/com/comet/opik/infrastructure/llm/gemini/LlmProviderGemini.java +++ b/apps/opik-backend/src/main/java/com/comet/opik/infrastructure/llm/gemini/LlmProviderGemini.java @@ -2,6 +2,8 @@ import com.comet.opik.api.ChunkedResponseHandler; import com.comet.opik.domain.llm.LlmProviderService; +import com.comet.opik.utils.JsonUtils; +import com.fasterxml.jackson.databind.JsonNode; import dev.ai4j.openai4j.chat.ChatCompletionRequest; import dev.ai4j.openai4j.chat.ChatCompletionResponse; import io.dropwizard.jersey.errors.ErrorMessage; @@ -40,8 +42,35 @@ public void validateRequest(@NonNull ChatCompletionRequest request) { } @Override - public Optional getLlmProviderError(@NonNull Throwable runtimeException) { + public Optional getLlmProviderError(@NonNull Throwable throwable) { + /// gemini throws RuntimeExceptions with message structure as follows: + /// ``` + /// java.lang.RuntimeException: HTTP error (429): { + /// "error": { + /// "code": 429, + /// "message": "Resource has been exhausted (e.g. check quota).", + /// "status": "RESOURCE_EXHAUSTED" + /// } + /// } + /// ``` + String message = throwable.getMessage(); + if (message.contains("{")) { + String jsonPart = message.substring(message.indexOf("{")); // Extract JSON part + try { + // Parse JSON + JsonNode errorNode = JsonUtils.MAPPER.readTree(jsonPart).get("error"); + if (errorNode != null) { + // Customize the message based on the error + return Optional.of(new ErrorMessage( + errorNode.get("code").asInt(), + errorNode.get("message").asText(), + errorNode.get("status").asText())); + } + } catch (Exception e) { + return Optional.empty(); + } + } + return Optional.empty(); } - } diff --git a/apps/opik-backend/src/main/java/com/comet/opik/infrastructure/llm/openai/LlmProviderOpenAi.java b/apps/opik-backend/src/main/java/com/comet/opik/infrastructure/llm/openai/LlmProviderOpenAi.java index d1eb422c3e..76edef6ff2 100644 --- a/apps/opik-backend/src/main/java/com/comet/opik/infrastructure/llm/openai/LlmProviderOpenAi.java +++ b/apps/opik-backend/src/main/java/com/comet/opik/infrastructure/llm/openai/LlmProviderOpenAi.java @@ -43,8 +43,8 @@ public void validateRequest(@NonNull ChatCompletionRequest request) { } @Override - public Optional getLlmProviderError(@NonNull Throwable runtimeException) { - if (runtimeException instanceof OpenAiHttpException openAiHttpException) { + public Optional getLlmProviderError(@NonNull Throwable throwable) { + if (throwable instanceof OpenAiHttpException openAiHttpException) { return Optional.of(new ErrorMessage(openAiHttpException.code(), openAiHttpException.getMessage())); } From 60c8f1531a9907e5fc5e49487acdfbd0933aa31a Mon Sep 17 00:00:00 2001 From: Ido Berkovich Date: Sun, 26 Jan 2025 13:29:28 +0200 Subject: [PATCH 2/3] OPIK-853 handle gemini client errors correctly --- .../llm/gemini/LlmProviderGemini.java | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/apps/opik-backend/src/main/java/com/comet/opik/infrastructure/llm/gemini/LlmProviderGemini.java b/apps/opik-backend/src/main/java/com/comet/opik/infrastructure/llm/gemini/LlmProviderGemini.java index 0df22e9a89..deafd18858 100644 --- a/apps/opik-backend/src/main/java/com/comet/opik/infrastructure/llm/gemini/LlmProviderGemini.java +++ b/apps/opik-backend/src/main/java/com/comet/opik/infrastructure/llm/gemini/LlmProviderGemini.java @@ -7,6 +7,7 @@ import dev.ai4j.openai4j.chat.ChatCompletionRequest; import dev.ai4j.openai4j.chat.ChatCompletionResponse; import io.dropwizard.jersey.errors.ErrorMessage; +import jakarta.ws.rs.core.Response; import lombok.NonNull; import lombok.RequiredArgsConstructor; @@ -14,6 +15,8 @@ import java.util.concurrent.CompletableFuture; import java.util.function.Consumer; +import static jakarta.ws.rs.core.Response.Status.Family.familyOf; + @RequiredArgsConstructor public class LlmProviderGemini implements LlmProviderService { private final @NonNull GeminiClientGenerator llmProviderClientGenerator; @@ -60,11 +63,14 @@ public Optional getLlmProviderError(@NonNull Throwable throwable) // Parse JSON JsonNode errorNode = JsonUtils.MAPPER.readTree(jsonPart).get("error"); if (errorNode != null) { - // Customize the message based on the error - return Optional.of(new ErrorMessage( - errorNode.get("code").asInt(), - errorNode.get("message").asText(), - errorNode.get("status").asText())); + var code = errorNode.get("code").asInt(); + if (familyOf(code) == Response.Status.Family.CLIENT_ERROR) { + // Customize the message based on the error + return Optional.of(new ErrorMessage( + code, + errorNode.get("message").asText(), + errorNode.get("status").asText())); + } } } catch (Exception e) { return Optional.empty(); From d33f54536f4f796ecad9eed4db4996f1e3d6a7f1 Mon Sep 17 00:00:00 2001 From: Ido Berkovich Date: Mon, 17 Feb 2025 13:31:17 +0200 Subject: [PATCH 3/3] OPIK-853 pr comments --- .../llm/gemini/GeminiErrorObject.java | 18 +++++++ .../llm/gemini/LlmProviderGemini.java | 49 ++++++++----------- 2 files changed, 38 insertions(+), 29 deletions(-) create mode 100644 apps/opik-backend/src/main/java/com/comet/opik/infrastructure/llm/gemini/GeminiErrorObject.java diff --git a/apps/opik-backend/src/main/java/com/comet/opik/infrastructure/llm/gemini/GeminiErrorObject.java b/apps/opik-backend/src/main/java/com/comet/opik/infrastructure/llm/gemini/GeminiErrorObject.java new file mode 100644 index 0000000000..1372186c12 --- /dev/null +++ b/apps/opik-backend/src/main/java/com/comet/opik/infrastructure/llm/gemini/GeminiErrorObject.java @@ -0,0 +1,18 @@ +package com.comet.opik.infrastructure.llm.gemini; + +import io.dropwizard.jersey.errors.ErrorMessage; + +import java.util.Optional; + +public record GeminiErrorObject(GeminiError error) { + public Optional toErrorMessage() { + if (error != null) { + return Optional.of(new ErrorMessage(error.code(), error.message(), error().status())); + } + + return Optional.empty(); + } +} + +record GeminiError(int code, String message, String status) { +} diff --git a/apps/opik-backend/src/main/java/com/comet/opik/infrastructure/llm/gemini/LlmProviderGemini.java b/apps/opik-backend/src/main/java/com/comet/opik/infrastructure/llm/gemini/LlmProviderGemini.java index deafd18858..4d46bd10a6 100644 --- a/apps/opik-backend/src/main/java/com/comet/opik/infrastructure/llm/gemini/LlmProviderGemini.java +++ b/apps/opik-backend/src/main/java/com/comet/opik/infrastructure/llm/gemini/LlmProviderGemini.java @@ -3,20 +3,19 @@ import com.comet.opik.api.ChunkedResponseHandler; import com.comet.opik.domain.llm.LlmProviderService; import com.comet.opik.utils.JsonUtils; -import com.fasterxml.jackson.databind.JsonNode; import dev.ai4j.openai4j.chat.ChatCompletionRequest; import dev.ai4j.openai4j.chat.ChatCompletionResponse; import io.dropwizard.jersey.errors.ErrorMessage; -import jakarta.ws.rs.core.Response; import lombok.NonNull; import lombok.RequiredArgsConstructor; +import lombok.extern.slf4j.Slf4j; +import java.io.UncheckedIOException; import java.util.Optional; import java.util.concurrent.CompletableFuture; import java.util.function.Consumer; -import static jakarta.ws.rs.core.Response.Status.Family.familyOf; - +@Slf4j @RequiredArgsConstructor public class LlmProviderGemini implements LlmProviderService { private final @NonNull GeminiClientGenerator llmProviderClientGenerator; @@ -44,35 +43,27 @@ public void generateStream(@NonNull ChatCompletionRequest request, @NonNull Stri public void validateRequest(@NonNull ChatCompletionRequest request) { } + /// gemini throws RuntimeExceptions with message structure as follows: + /// ``` + /// java.lang.RuntimeException: HTTP error (429): { + /// "error": { + /// "code": 429, + /// "message": "Resource has been exhausted (e.g. check quota).", + /// "status": "RESOURCE_EXHAUSTED" + /// } + /// } + /// ``` @Override public Optional getLlmProviderError(@NonNull Throwable throwable) { - /// gemini throws RuntimeExceptions with message structure as follows: - /// ``` - /// java.lang.RuntimeException: HTTP error (429): { - /// "error": { - /// "code": 429, - /// "message": "Resource has been exhausted (e.g. check quota).", - /// "status": "RESOURCE_EXHAUSTED" - /// } - /// } - /// ``` String message = throwable.getMessage(); - if (message.contains("{")) { - String jsonPart = message.substring(message.indexOf("{")); // Extract JSON part + var openBraceIndex = message.indexOf('{'); + if (openBraceIndex >= 0) { + String jsonPart = message.substring(openBraceIndex); // Extract JSON part try { - // Parse JSON - JsonNode errorNode = JsonUtils.MAPPER.readTree(jsonPart).get("error"); - if (errorNode != null) { - var code = errorNode.get("code").asInt(); - if (familyOf(code) == Response.Status.Family.CLIENT_ERROR) { - // Customize the message based on the error - return Optional.of(new ErrorMessage( - code, - errorNode.get("message").asText(), - errorNode.get("status").asText())); - } - } - } catch (Exception e) { + var geminiError = JsonUtils.readValue(jsonPart, GeminiErrorObject.class); + return geminiError.toErrorMessage(); + } catch (UncheckedIOException e) { + log.warn("failed to parse Gemini error message", e); return Optional.empty(); } }