From 7788f954d62c973b8376f7212a640572b21718ba Mon Sep 17 00:00:00 2001 From: amithkoujalgi Date: Thu, 18 Sep 2025 01:22:12 +0530 Subject: [PATCH] Improve error handling and code clarity across modules Enhanced error handling for image URL loading in OllamaChatRequestBuilder, ensuring exceptions are thrown and logged appropriately. Updated test cases to reflect new exception behavior. Improved documentation and code clarity in WeatherTool and test classes. Refactored JSON parsing in response models for conciseness. Minor cleanup in pom.xml and integration test comments for better maintainability. --- pom.xml | 2 - .../java/io/github/ollama4j/OllamaAPI.java | 3 +- .../models/chat/OllamaChatRequestBuilder.java | 18 ++++++--- .../models/request/OllamaCommonRequest.java | 8 +++- .../models/response/OllamaResult.java | 6 +-- .../response/OllamaStructuredResult.java | 8 +--- .../tools/sampletools/WeatherTool.java | 9 ++++- .../OllamaAPIIntegrationTest.java | 40 ++++++++++--------- .../TestOllamaChatRequestBuilder.java | 12 ++++-- .../unittests/TestOllamaRequestBody.java | 10 ++++- .../unittests/TestOptionsAndUtils.java | 4 +- .../jackson/TestChatRequestSerialization.java | 11 +++-- .../TestEmbedRequestSerialization.java | 3 +- .../TestGenerateRequestSerialization.java | 3 +- .../TestModelPullResponseSerialization.java | 3 +- 15 files changed, 81 insertions(+), 59 deletions(-) diff --git a/pom.xml b/pom.xml index 8cd542d..4b451c0 100644 --- a/pom.xml +++ b/pom.xml @@ -169,8 +169,6 @@ spotless-maven-plugin 2.46.1 - - diff --git a/src/main/java/io/github/ollama4j/OllamaAPI.java b/src/main/java/io/github/ollama4j/OllamaAPI.java index 9c4e28d..f90043e 100644 --- a/src/main/java/io/github/ollama4j/OllamaAPI.java +++ b/src/main/java/io/github/ollama4j/OllamaAPI.java @@ -727,7 +727,8 @@ public class OllamaAPI { LOG.debug("Model response:\n{}", ollamaResult); return ollamaResult; } else { - LOG.debug("Model response:\n{}", Utils.toJSON(responseBody)); + String errorResponse = Utils.toJSON(responseBody); + LOG.debug("Model response:\n{}", errorResponse); throw new OllamaBaseException(statusCode + " - " + responseBody); } } diff --git a/src/main/java/io/github/ollama4j/models/chat/OllamaChatRequestBuilder.java b/src/main/java/io/github/ollama4j/models/chat/OllamaChatRequestBuilder.java index 4ce62dc..5311101 100644 --- a/src/main/java/io/github/ollama4j/models/chat/OllamaChatRequestBuilder.java +++ b/src/main/java/io/github/ollama4j/models/chat/OllamaChatRequestBuilder.java @@ -100,7 +100,8 @@ public class OllamaChatRequestBuilder { OllamaChatMessageRole role, String content, List toolCalls, - String... imageUrls) { + String... imageUrls) + throws IOException, InterruptedException { List messages = this.request.getMessages(); List binaryImages = null; if (imageUrls.length > 0) { @@ -112,11 +113,18 @@ public class OllamaChatRequestBuilder { imageUrl, imageURLConnectTimeoutSeconds, imageURLReadTimeoutSeconds)); - } catch (Exception e) { - LOG.warn( - "Loading image from URL '{}' failed, will not add to message!", + } catch (InterruptedException e) { + LOG.error( + "Failed to load image from URL: {}. Cause: {}", imageUrl, - e); + e.getMessage()); + throw e; + } catch (IOException e) { + LOG.warn( + "Failed to load image from URL: {}. Cause: {}", + imageUrl, + e.getMessage()); + throw e; } } } diff --git a/src/main/java/io/github/ollama4j/models/request/OllamaCommonRequest.java b/src/main/java/io/github/ollama4j/models/request/OllamaCommonRequest.java index aa3768d..d8c996c 100644 --- a/src/main/java/io/github/ollama4j/models/request/OllamaCommonRequest.java +++ b/src/main/java/io/github/ollama4j/models/request/OllamaCommonRequest.java @@ -21,8 +21,12 @@ public abstract class OllamaCommonRequest { protected String model; - // @JsonSerialize(using = BooleanToJsonFormatFlagSerializer.class) - // this can either be set to format=json or format={"key1": "val1", "key2": "val2"} + /** + * The value can either be + * 
{@code json }
+ * or + * 
{@code {"key1": "val1", "key2": "val2"} }
+ */ @JsonProperty(value = "format", required = false, defaultValue = "json") protected Object format; diff --git a/src/main/java/io/github/ollama4j/models/response/OllamaResult.java b/src/main/java/io/github/ollama4j/models/response/OllamaResult.java index 1c1abb5..76b0982 100644 --- a/src/main/java/io/github/ollama4j/models/response/OllamaResult.java +++ b/src/main/java/io/github/ollama4j/models/response/OllamaResult.java @@ -112,10 +112,8 @@ public class OllamaResult { throw new IllegalArgumentException("Response is not a valid JSON object"); } - Map response = - getObjectMapper() - .readValue(responseStr, new TypeReference>() {}); - return response; + return getObjectMapper() + .readValue(responseStr, new TypeReference>() {}); } catch (JsonProcessingException e) { throw new IllegalArgumentException( "Failed to parse response as JSON: " + e.getMessage(), e); diff --git a/src/main/java/io/github/ollama4j/models/response/OllamaStructuredResult.java b/src/main/java/io/github/ollama4j/models/response/OllamaStructuredResult.java index 7cdc4bc..17c6ba4 100644 --- a/src/main/java/io/github/ollama4j/models/response/OllamaStructuredResult.java +++ b/src/main/java/io/github/ollama4j/models/response/OllamaStructuredResult.java @@ -65,12 +65,8 @@ public class OllamaStructuredResult { */ public Map getStructuredResponse() { try { - Map response = - getObjectMapper() - .readValue( - this.getResponse(), - new TypeReference>() {}); - return response; + return getObjectMapper() + .readValue(this.getResponse(), new TypeReference>() {}); } catch (JsonProcessingException e) { throw new RuntimeException(e); } diff --git a/src/main/java/io/github/ollama4j/tools/sampletools/WeatherTool.java b/src/main/java/io/github/ollama4j/tools/sampletools/WeatherTool.java index 2a13ece..0fd06b9 100644 --- a/src/main/java/io/github/ollama4j/tools/sampletools/WeatherTool.java +++ b/src/main/java/io/github/ollama4j/tools/sampletools/WeatherTool.java @@ -15,7 +15,14 @@ import java.util.Map; public class WeatherTool { private String paramCityName = "cityName"; - public WeatherTool() {} + /** + * Default constructor for WeatherTool. + * This constructor is intentionally left empty because no initialization is required + * for this sample tool. If future state or dependencies are needed, they can be added here. + */ + public WeatherTool() { + // No initialization required + } public String getCurrentWeather(Map arguments) { String city = (String) arguments.get(paramCityName); diff --git a/src/test/java/io/github/ollama4j/integrationtests/OllamaAPIIntegrationTest.java b/src/test/java/io/github/ollama4j/integrationtests/OllamaAPIIntegrationTest.java index 9d178fc..5c29a3e 100644 --- a/src/test/java/io/github/ollama4j/integrationtests/OllamaAPIIntegrationTest.java +++ b/src/test/java/io/github/ollama4j/integrationtests/OllamaAPIIntegrationTest.java @@ -619,25 +619,27 @@ class OllamaAPIIntegrationTest { OllamaChatMessageRole.ASSISTANT.getRoleName(), chatResult.getResponseModel().getMessage().getRole().getRoleName()); - // Reproducing this scenario consistently is challenging, as the model's behavior can vary. - // Therefore, these checks are currently skipped until a more reliable approach is found. - - // List toolCalls = - // chatResult.getChatHistory().get(1).getToolCalls(); - // assertEquals(1, toolCalls.size()); - // OllamaToolCallsFunction function = toolCalls.get(0).getFunction(); - // assertEquals("sayHello", function.getName()); - // assertEquals(2, function.getArguments().size()); - // Object name = function.getArguments().get("name"); - // assertNotNull(name); - // assertEquals("Rahul", name); - // Object numberOfHearts = function.getArguments().get("numberOfHearts"); - // assertNotNull(numberOfHearts); - // assertTrue(Integer.parseInt(numberOfHearts.toString()) > 1); - // assertTrue(chatResult.getChatHistory().size() > 2); - // List finalToolCalls = - // chatResult.getResponseModel().getMessage().getToolCalls(); - // assertNull(finalToolCalls); + /* + * Reproducing this scenario consistently is challenging, as the model's behavior can vary. + * Therefore, these checks are currently skipped until a more reliable approach is found. + * + * // List toolCalls = + * // chatResult.getChatHistory().get(1).getToolCalls(); + * // assertEquals(1, toolCalls.size()); + * // OllamaToolCallsFunction function = toolCalls.get(0).getFunction(); + * // assertEquals("sayHello", function.getName()); + * // assertEquals(2, function.getArguments().size()); + * // Object name = function.getArguments().get("name"); + * // assertNotNull(name); + * // assertEquals("Rahul", name); + * // Object numberOfHearts = function.getArguments().get("numberOfHearts"); + * // assertNotNull(numberOfHearts); + * // assertTrue(Integer.parseInt(numberOfHearts.toString()) > 1); + * // assertTrue(chatResult.getChatHistory().size() > 2); + * // List finalToolCalls = + * // chatResult.getResponseModel().getMessage().getToolCalls(); + * // assertNull(finalToolCalls); + */ } @Test diff --git a/src/test/java/io/github/ollama4j/unittests/TestOllamaChatRequestBuilder.java b/src/test/java/io/github/ollama4j/unittests/TestOllamaChatRequestBuilder.java index d461d58..636c266 100644 --- a/src/test/java/io/github/ollama4j/unittests/TestOllamaChatRequestBuilder.java +++ b/src/test/java/io/github/ollama4j/unittests/TestOllamaChatRequestBuilder.java @@ -40,14 +40,18 @@ class TestOllamaChatRequestBuilder { } @Test - void testImageUrlFailuresAreHandledAndBuilderRemainsUsable() { + void testImageUrlFailuresThrowExceptionAndBuilderRemainsUsable() { OllamaChatRequestBuilder builder = OllamaChatRequestBuilder.getInstance("m"); String invalidUrl = "ht!tp:/bad_url"; // clearly invalid URL format - // No exception should be thrown; builder should handle invalid URL gracefully - builder.withMessage(OllamaChatMessageRole.USER, "hi", Collections.emptyList(), invalidUrl); + // Exception should be thrown for invalid URL + assertThrows( + Exception.class, + () -> { + builder.withMessage( + OllamaChatMessageRole.USER, "hi", Collections.emptyList(), invalidUrl); + }); - // The builder should still be usable after the exception OllamaChatRequest req = builder.withMessage(OllamaChatMessageRole.USER, "hello", Collections.emptyList()) .build(); diff --git a/src/test/java/io/github/ollama4j/unittests/TestOllamaRequestBody.java b/src/test/java/io/github/ollama4j/unittests/TestOllamaRequestBody.java index 2e14063..d3af32e 100644 --- a/src/test/java/io/github/ollama4j/unittests/TestOllamaRequestBody.java +++ b/src/test/java/io/github/ollama4j/unittests/TestOllamaRequestBody.java @@ -50,9 +50,17 @@ class TestOllamaRequestBody { } @Override - public void onError(Throwable throwable) {} + // This method is intentionally left empty because, for this test, + // we do not expect any errors to occur during synchronous publishing. + // If an error does occur, the test will fail elsewhere. + public void onError(Throwable throwable) { + // No action needed for this test + } @Override + // This method is intentionally left empty because for this test, + // all the data is synchronously delivered by the publisher, so no action is + // needed on completion. public void onComplete() {} }); diff --git a/src/test/java/io/github/ollama4j/unittests/TestOptionsAndUtils.java b/src/test/java/io/github/ollama4j/unittests/TestOptionsAndUtils.java index 409237c..45fefff 100644 --- a/src/test/java/io/github/ollama4j/unittests/TestOptionsAndUtils.java +++ b/src/test/java/io/github/ollama4j/unittests/TestOptionsAndUtils.java @@ -67,9 +67,9 @@ class TestOptionsAndUtils { @Test void testOptionsBuilderRejectsUnsupportedCustomType() { - OptionsBuilder builder = new OptionsBuilder(); assertThrows( - IllegalArgumentException.class, () -> builder.setCustomOption("bad", new Object())); + IllegalArgumentException.class, + () -> new OptionsBuilder().setCustomOption("bad", new Object())); } @Test diff --git a/src/test/java/io/github/ollama4j/unittests/jackson/TestChatRequestSerialization.java b/src/test/java/io/github/ollama4j/unittests/jackson/TestChatRequestSerialization.java index 9c577a5..e533090 100644 --- a/src/test/java/io/github/ollama4j/unittests/jackson/TestChatRequestSerialization.java +++ b/src/test/java/io/github/ollama4j/unittests/jackson/TestChatRequestSerialization.java @@ -104,11 +104,9 @@ public class TestChatRequestSerialization extends AbstractSerializationTest { - OllamaChatRequest req = - builder.withMessage(OllamaChatMessageRole.USER, "Some prompt") - .withOptions( - b.setCustomOption("cust_obj", new Object()).build()) - .build(); + builder.withMessage(OllamaChatMessageRole.USER, "Some prompt") + .withOptions(b.setCustomOption("cust_obj", new Object()).build()) + .build(); }); } @@ -120,7 +118,8 @@ public class TestChatRequestSerialization extends AbstractSerializationTest omit as deserialization + // no jackson deserialization as format property is not boolean ==> omit as + // deserialization // of request is never used in real code anyways JSONObject jsonObject = new JSONObject(jsonRequest); String requestFormatProperty = jsonObject.getString("format"); diff --git a/src/test/java/io/github/ollama4j/unittests/jackson/TestEmbedRequestSerialization.java b/src/test/java/io/github/ollama4j/unittests/jackson/TestEmbedRequestSerialization.java index 2038bd3..0fa2175 100644 --- a/src/test/java/io/github/ollama4j/unittests/jackson/TestEmbedRequestSerialization.java +++ b/src/test/java/io/github/ollama4j/unittests/jackson/TestEmbedRequestSerialization.java @@ -16,8 +16,7 @@ import io.github.ollama4j.utils.OptionsBuilder; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -public class TestEmbedRequestSerialization - extends AbstractSerializationTest { +class TestEmbedRequestSerialization extends AbstractSerializationTest { private OllamaEmbedRequestBuilder builder; diff --git a/src/test/java/io/github/ollama4j/unittests/jackson/TestGenerateRequestSerialization.java b/src/test/java/io/github/ollama4j/unittests/jackson/TestGenerateRequestSerialization.java index 2d52997..8cbbf08 100644 --- a/src/test/java/io/github/ollama4j/unittests/jackson/TestGenerateRequestSerialization.java +++ b/src/test/java/io/github/ollama4j/unittests/jackson/TestGenerateRequestSerialization.java @@ -17,8 +17,7 @@ import org.json.JSONObject; import org.junit.jupiter.api.BeforeEach; import org.junit.jupiter.api.Test; -public class TestGenerateRequestSerialization - extends AbstractSerializationTest { +class TestGenerateRequestSerialization extends AbstractSerializationTest { private OllamaGenerateRequestBuilder builder; diff --git a/src/test/java/io/github/ollama4j/unittests/jackson/TestModelPullResponseSerialization.java b/src/test/java/io/github/ollama4j/unittests/jackson/TestModelPullResponseSerialization.java index a767030..c981bf1 100644 --- a/src/test/java/io/github/ollama4j/unittests/jackson/TestModelPullResponseSerialization.java +++ b/src/test/java/io/github/ollama4j/unittests/jackson/TestModelPullResponseSerialization.java @@ -19,8 +19,7 @@ import org.junit.jupiter.api.Test; * error responses from Ollama server that return HTTP 200 with error messages * in the JSON body. */ -public class TestModelPullResponseSerialization - extends AbstractSerializationTest { +class TestModelPullResponseSerialization extends AbstractSerializationTest { /** * Test the specific error case reported in GitHub issue #138.