From 36f7d14c68c806544c65935372f0d44b2f0f48b8 Mon Sep 17 00:00:00 2001 From: amithkoujalgi Date: Sun, 28 Sep 2025 22:28:48 +0530 Subject: [PATCH] Refactor OllamaAPI exception handling to properly manage InterruptedException and improve logging. Remove unused Logback Classic dependency from pom.xml and clean up commented-out code in integration tests. --- pom.xml | 8 -- .../java/io/github/ollama4j/OllamaAPI.java | 45 ++++++++++- .../response/OllamaAsyncResultStreamer.java | 8 +- .../OllamaAPIIntegrationTest.java | 60 --------------- .../ollama4j/unittests/TestMockedAPIs.java | 75 ------------------- .../TestOllamaChatRequestBuilder.java | 25 ------- .../unittests/TestOptionsAndUtils.java | 4 +- .../ollama4j/unittests/TestToolRegistry.java | 52 ------------- .../unittests/TestToolsPromptBuilder.java | 67 ----------------- 9 files changed, 50 insertions(+), 294 deletions(-) delete mode 100644 src/test/java/io/github/ollama4j/unittests/TestToolRegistry.java delete mode 100644 src/test/java/io/github/ollama4j/unittests/TestToolsPromptBuilder.java diff --git a/pom.xml b/pom.xml index 9e1fd27..3132f56 100644 --- a/pom.xml +++ b/pom.xml @@ -276,14 +276,6 @@ 2.0.17 - - - - - - - - org.junit.jupiter junit-jupiter-api diff --git a/src/main/java/io/github/ollama4j/OllamaAPI.java b/src/main/java/io/github/ollama4j/OllamaAPI.java index 7347844..401228c 100644 --- a/src/main/java/io/github/ollama4j/OllamaAPI.java +++ b/src/main/java/io/github/ollama4j/OllamaAPI.java @@ -109,7 +109,6 @@ public class OllamaAPI { */ public OllamaAPI() { this.host = "http://localhost:11434"; - // initializeMetrics(); } /** @@ -124,7 +123,6 @@ public class OllamaAPI { this.host = host; } LOG.info("Ollama4j client initialized. Connected to Ollama server at: {}", this.host); - // initializeMetrics(); } /** @@ -174,6 +172,9 @@ public class OllamaAPI { response = httpClient.send(httpRequest, HttpResponse.BodyHandlers.ofString()); statusCode = response.statusCode(); return statusCode == 200; + } catch (InterruptedException ie) { + Thread.currentThread().interrupt(); + throw new OllamaException("Ping interrupted", ie); } catch (Exception e) { throw new OllamaException("Ping failed", e); } finally { @@ -220,6 +221,9 @@ public class OllamaAPI { } else { throw new OllamaException(statusCode + " - " + responseString); } + } catch (InterruptedException ie) { + Thread.currentThread().interrupt(); + throw new OllamaException("ps interrupted", ie); } catch (Exception e) { throw new OllamaException("ps failed", e); } finally { @@ -262,6 +266,9 @@ public class OllamaAPI { } else { throw new OllamaException(statusCode + " - " + responseString); } + } catch (InterruptedException ie) { + Thread.currentThread().interrupt(); + throw new OllamaException("listModels interrupted", ie); } catch (Exception e) { throw new OllamaException(e.getMessage(), e); } finally { @@ -353,6 +360,9 @@ public class OllamaAPI { if (statusCode != 200) { throw new OllamaException(statusCode + " - " + responseString); } + } catch (InterruptedException ie) { + Thread.currentThread().interrupt(); + throw new OllamaException("Thread was interrupted during model pull.", ie); } catch (Exception e) { throw new OllamaException(e.getMessage(), e); } finally { @@ -425,6 +435,9 @@ public class OllamaAPI { } else { throw new OllamaException(statusCode + " - " + responseString); } + } catch (InterruptedException ie) { + Thread.currentThread().interrupt(); + throw new OllamaException("Thread was interrupted", ie); } catch (Exception e) { throw new OllamaException(e.getMessage(), e); } finally { @@ -468,6 +481,9 @@ public class OllamaAPI { + " after " + numberOfRetriesForModelPull + " retries"); + } catch (InterruptedException ie) { + Thread.currentThread().interrupt(); + throw new OllamaException("Thread was interrupted", ie); } catch (Exception e) { throw new OllamaException(e.getMessage(), e); } @@ -507,6 +523,9 @@ public class OllamaAPI { } else { throw new OllamaException(statusCode + " - " + responseBody); } + } catch (InterruptedException ie) { + Thread.currentThread().interrupt(); + throw new OllamaException("Thread was interrupted", ie); } catch (Exception e) { throw new OllamaException(e.getMessage(), e); } finally { @@ -555,7 +574,7 @@ public class OllamaAPI { new BufferedReader( new InputStreamReader(response.body(), StandardCharsets.UTF_8))) { String line; - StringBuffer lines = new StringBuffer(); + StringBuilder lines = new StringBuilder(); while ((line = reader.readLine()) != null) { ModelPullResponse res = Utils.getObjectMapper().readValue(line, ModelPullResponse.class); @@ -568,6 +587,9 @@ public class OllamaAPI { } out = lines; } + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + throw new OllamaException("Thread was interrupted", e); } catch (Exception e) { throw new OllamaException(e.getMessage(), e); } finally { @@ -617,6 +639,9 @@ public class OllamaAPI { if (statusCode != 200) { throw new OllamaException(statusCode + " - " + responseBody); } + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + throw new OllamaException("Thread was interrupted", e); } catch (Exception e) { throw new OllamaException(statusCode + " - " + out, e); } finally { @@ -674,6 +699,10 @@ public class OllamaAPI { LOG.debug("Unload response: {} - {}", statusCode, responseBody); throw new OllamaException(statusCode + " - " + responseBody); } + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + LOG.debug("Unload interrupted: {} - {}", statusCode, out); + throw new OllamaException(statusCode + " - " + out, e); } catch (Exception e) { LOG.debug("Unload failed: {} - {}", statusCode, out); throw new OllamaException(statusCode + " - " + out, e); @@ -716,6 +745,9 @@ public class OllamaAPI { } else { throw new OllamaException(statusCode + " - " + responseBody); } + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + throw new OllamaException("Thread was interrupted", e); } catch (Exception e) { throw new OllamaException(e.getMessage(), e); } finally { @@ -848,7 +880,6 @@ public class OllamaAPI { if (request.isUseTools()) { // add all registered tools to request request.setTools(toolRegistry.getRegisteredTools()); - System.out.println("Use tools is set."); } if (tokenHandler != null) { @@ -907,6 +938,9 @@ public class OllamaAPI { toolCallTries++; } return result; + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + throw new OllamaException("Thread was interrupted", e); } catch (Exception e) { throw new OllamaException(e.getMessage(), e); } @@ -1124,6 +1158,9 @@ public class OllamaAPI { statusCode = result.getHttpStatusCode(); out = result; return result; + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + throw new OllamaException("Thread was interrupted", e); } catch (Exception e) { throw new OllamaException(e.getMessage(), e); } finally { diff --git a/src/main/java/io/github/ollama4j/models/response/OllamaAsyncResultStreamer.java b/src/main/java/io/github/ollama4j/models/response/OllamaAsyncResultStreamer.java index 4929c81..cb566b6 100644 --- a/src/main/java/io/github/ollama4j/models/response/OllamaAsyncResultStreamer.java +++ b/src/main/java/io/github/ollama4j/models/response/OllamaAsyncResultStreamer.java @@ -136,19 +136,25 @@ public class OllamaAsyncResultStreamer extends Thread { try { reader.close(); } catch (IOException e) { + /* do nothing */ } } if (responseBodyStream != null) { try { responseBodyStream.close(); } catch (IOException e) { + /* do nothing */ } } } if (statusCode != 200) { throw new OllamaException(this.completeResponse); } - } catch (IOException | InterruptedException | OllamaException e) { + } catch (InterruptedException e) { + Thread.currentThread().interrupt(); + this.succeeded = false; + this.completeResponse = "[FAILED] " + e.getMessage(); + } catch (IOException | OllamaException e) { this.succeeded = false; this.completeResponse = "[FAILED] " + e.getMessage(); } diff --git a/src/test/java/io/github/ollama4j/integrationtests/OllamaAPIIntegrationTest.java b/src/test/java/io/github/ollama4j/integrationtests/OllamaAPIIntegrationTest.java index 16cc6e1..5a5bf39 100644 --- a/src/test/java/io/github/ollama4j/integrationtests/OllamaAPIIntegrationTest.java +++ b/src/test/java/io/github/ollama4j/integrationtests/OllamaAPIIntegrationTest.java @@ -336,7 +336,6 @@ class OllamaAPIIntegrationTest { .withThink(false) .withOptions(new OptionsBuilder().build()) .build(); - OllamaGenerateStreamObserver handler = null; OllamaResult result = api.generate( request, @@ -821,33 +820,6 @@ class OllamaAPIIntegrationTest { assertNotNull(chatResult.getResponseModel()); } - // /** - // * Tests generateWithImages using an image URL as input. - // * - // *

Scenario: Calls generateWithImages with a vision model and an image URL,expecting a - // * non-empty response. Usage: generateWithImages, image from URL, no streaming. - // */ - // @Test - // @Order(17) - // void shouldGenerateWithImageURLs() - // throws OllamaBaseException { - // api.pullModel(VISION_MODEL); - // - // OllamaResult result = - // api.generateWithImages( - // VISION_MODEL, - // "What is in this image?", - // List.of( - // - // "https://i.pinimg.com/736x/f9/4e/cb/f94ecba040696a3a20b484d2e15159ec.jpg"), - // new OptionsBuilder().build(), - // null, - // null); - // assertNotNull(result); - // assertNotNull(result.getResponse()); - // assertFalse(result.getResponse().isEmpty()); - // } - /** * Tests generateWithImages using an image file as input. * @@ -1041,38 +1013,6 @@ class OllamaAPIIntegrationTest { assertFalse(result.getResponse().isEmpty()); } - // /** - // * Tests generate with raw=true and thinking enabled. - // * - // *

Scenario: Calls generate with raw=true and think=true combination. Usage: generate, - // * raw=true, thinking enabled, no streaming. - // */ - // @Test - // @Order(23) - // void shouldGenerateWithRawModeAndThinking() - // throws OllamaBaseException - // { - // api.pullModel(THINKING_TOOL_MODEL_2); - // api.unloadModel(THINKING_TOOL_MODEL_2); - // boolean raw = - // true; // if true no formatting will be applied to the prompt. You may choose - // to use - // // the raw parameter if you are specifying a full templated prompt in your - // // request to the API - // boolean thinking = true; - // OllamaResult result = - // api.generate( - // THINKING_TOOL_MODEL_2, - // "Validate: 1+1=2", - // raw, - // thinking, - // new OptionsBuilder().build(), - // new OllamaGenerateStreamObserver(null, null)); - // assertNotNull(result); - // assertNotNull(result.getResponse()); - // assertNotNull(result.getThinking()); - // } - /** * Tests generate with all parameters enabled: raw=true, thinking=true, and streaming. * diff --git a/src/test/java/io/github/ollama4j/unittests/TestMockedAPIs.java b/src/test/java/io/github/ollama4j/unittests/TestMockedAPIs.java index abe6a60..1b944d5 100644 --- a/src/test/java/io/github/ollama4j/unittests/TestMockedAPIs.java +++ b/src/test/java/io/github/ollama4j/unittests/TestMockedAPIs.java @@ -90,20 +90,6 @@ class TestMockedAPIs { } } - // @Test - // void testRegisteredTools() { - // OllamaAPI ollamaAPI = Mockito.mock(OllamaAPI.class); - // doNothing().when(ollamaAPI).registerTools(Collections.emptyList()); - // ollamaAPI.registerTools(Collections.emptyList()); - // verify(ollamaAPI, times(1)).registerTools(Collections.emptyList()); - // - // List toolSpecifications = new ArrayList<>(); - // toolSpecifications.add(getSampleToolSpecification()); - // doNothing().when(ollamaAPI).registerTools(toolSpecifications); - // ollamaAPI.registerTools(toolSpecifications); - // verify(ollamaAPI, times(1)).registerTools(toolSpecifications); - // } - @Test void testGetModelDetails() { OllamaAPI ollamaAPI = Mockito.mock(OllamaAPI.class); @@ -169,7 +155,6 @@ class TestMockedAPIs { OllamaAPI ollamaAPI = Mockito.mock(OllamaAPI.class); String model = "llama2"; String prompt = "some prompt text"; - OptionsBuilder optionsBuilder = new OptionsBuilder(); OllamaGenerateStreamObserver observer = new OllamaGenerateStreamObserver(null, null); try { OllamaGenerateRequest request = @@ -318,64 +303,4 @@ class TestMockedAPIs { throw new RuntimeException("Failed to run test: testGetRoleFound"); } } - - // private static Tools.ToolSpecification getSampleToolSpecification() { - // return Tools.ToolSpecification.builder() - // .functionName("current-weather") - // .functionDescription("Get current weather") - // .toolFunction( - // new ToolFunction() { - // @Override - // public Object apply(Map arguments) { - // String location = arguments.get("city").toString(); - // return "Currently " + location + "'s weather is beautiful."; - // } - // }) - // .toolPrompt( - // Tools.PromptFuncDefinition.builder() - // .type("prompt") - // .function( - // Tools.PromptFuncDefinition.PromptFuncSpec.builder() - // .name("get-location-weather-info") - // .description("Get location details") - // .parameters( - // Tools.PromptFuncDefinition.Parameters - // .builder() - // .type("object") - // .properties( - // Map.of( - // "city", - // Tools - // - // .PromptFuncDefinition - // - // .Property - // - // .builder() - // .type( - // - // "string") - // - // .description( - // - // "The city," - // - // + " e.g." - // - // + " New Delhi," - // - // + " India") - // - // .required( - // - // true) - // - // .build())) - // - // .required(java.util.List.of("city")) - // .build()) - // .build()) - // .build()) - // .build(); - // } } diff --git a/src/test/java/io/github/ollama4j/unittests/TestOllamaChatRequestBuilder.java b/src/test/java/io/github/ollama4j/unittests/TestOllamaChatRequestBuilder.java index 50c4b4a..7b069a6 100644 --- a/src/test/java/io/github/ollama4j/unittests/TestOllamaChatRequestBuilder.java +++ b/src/test/java/io/github/ollama4j/unittests/TestOllamaChatRequestBuilder.java @@ -37,29 +37,4 @@ class TestOllamaChatRequestBuilder { assertNotNull(afterReset.getMessages()); assertEquals(0, afterReset.getMessages().size()); } - - // @Test - // void testImageUrlFailuresThrowExceptionAndBuilderRemainsUsable() { - // OllamaChatRequestBuilder builder = OllamaChatRequestBuilder.builder().withModel("m"); - // String invalidUrl = "ht!tp:/bad_url"; // clearly invalid URL format - // - // // Exception should be thrown for invalid URL - // assertThrows( - // Exception.class, - // () -> { - // builder.withMessage( - // OllamaChatMessageRole.USER, "hi", Collections.emptyList(), - // invalidUrl); - // }); - // - // OllamaChatRequest req = - // builder.withMessage(OllamaChatMessageRole.USER, "hello", - // Collections.emptyList()) - // .build(); - // - // assertNotNull(req.getMessages()); - // assert (!req.getMessages().isEmpty()); - // OllamaChatMessage msg = req.getMessages().get(0); - // assertNotNull(msg.getResponse()); - // } } 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/TestToolRegistry.java b/src/test/java/io/github/ollama4j/unittests/TestToolRegistry.java deleted file mode 100644 index c672a74..0000000 --- a/src/test/java/io/github/ollama4j/unittests/TestToolRegistry.java +++ /dev/null @@ -1,52 +0,0 @@ -/* - * Ollama4j - Java library for interacting with Ollama server. - * Copyright (c) 2025 Amith Koujalgi and contributors. - * - * Licensed under the MIT License (the "License"); - * you may not use this file except in compliance with the License. - * -*/ -package io.github.ollama4j.unittests; - -import static org.junit.jupiter.api.Assertions.*; - -class TestToolRegistry { - // - // @Test - // void testAddAndGetToolFunction() { - // ToolRegistry registry = new ToolRegistry(); - // ToolFunction fn = args -> "ok:" + args.get("x"); - // - // Tools.ToolSpecification spec = - // Tools.ToolSpecification.builder() - // .functionName("test") - // .functionDescription("desc") - // .toolFunction(fn) - // .build(); - // - // registry.addTool("test", spec); - // ToolFunction retrieved = registry.getToolFunction("test"); - // assertNotNull(retrieved); - // assertEquals("ok:42", retrieved.apply(Map.of("x", 42))); - // } - // - // @Test - // void testGetUnknownReturnsNull() { - // ToolRegistry registry = new ToolRegistry(); - // assertNull(registry.getToolFunction("nope")); - // } - // - // @Test - // void testClearRemovesAll() { - // ToolRegistry registry = new ToolRegistry(); - // registry.addTool("a", Tools.ToolSpecification.builder().toolFunction(args -> - // 1).build()); - // registry.addTool("b", Tools.ToolSpecification.builder().toolFunction(args -> - // 2).build()); - // assertFalse(registry.getRegisteredSpecs().isEmpty()); - // registry.clear(); - // assertTrue(registry.getRegisteredSpecs().isEmpty()); - // assertNull(registry.getToolFunction("a")); - // assertNull(registry.getToolFunction("b")); - // } -} diff --git a/src/test/java/io/github/ollama4j/unittests/TestToolsPromptBuilder.java b/src/test/java/io/github/ollama4j/unittests/TestToolsPromptBuilder.java deleted file mode 100644 index 3cb0d30..0000000 --- a/src/test/java/io/github/ollama4j/unittests/TestToolsPromptBuilder.java +++ /dev/null @@ -1,67 +0,0 @@ -/* - * Ollama4j - Java library for interacting with Ollama server. - * Copyright (c) 2025 Amith Koujalgi and contributors. - * - * Licensed under the MIT License (the "License"); - * you may not use this file except in compliance with the License. - * -*/ -package io.github.ollama4j.unittests; - -class TestToolsPromptBuilder { - // - // @Test - // void testPromptBuilderIncludesToolsAndPrompt() throws JsonProcessingException { - // Tools.PromptFuncDefinition.Property cityProp = - // Tools.PromptFuncDefinition.Property.builder() - // .type("string") - // .description("city name") - // .required(true) - // .build(); - // - // Tools.PromptFuncDefinition.Property unitsProp = - // Tools.PromptFuncDefinition.Property.builder() - // .type("string") - // .description("units") - // .enumValues(List.of("metric", "imperial")) - // .required(false) - // .build(); - // - // Tools.PromptFuncDefinition.Parameters params = - // Tools.PromptFuncDefinition.Parameters.builder() - // .type("object") - // .properties(Map.of("city", cityProp, "units", unitsProp)) - // .build(); - // - // Tools.PromptFuncDefinition.PromptFuncSpec spec = - // Tools.PromptFuncDefinition.PromptFuncSpec.builder() - // .name("getWeather") - // .description("Get weather for a city") - // .parameters(params) - // .build(); - // - // Tools.PromptFuncDefinition def = - // Tools.PromptFuncDefinition.builder().type("function").function(spec).build(); - // - // Tools.ToolSpecification toolSpec = - // Tools.ToolSpecification.builder() - // .functionName("getWeather") - // .functionDescription("Get weather for a city") - // .toolPrompt(def) - // .build(); - // - // Tools.PromptBuilder pb = - // new Tools.PromptBuilder() - // .withToolSpecification(toolSpec) - // .withPrompt("Tell me the weather."); - // - // String built = pb.build(); - // assertTrue(built.contains("[AVAILABLE_TOOLS]")); - // assertTrue(built.contains("[/AVAILABLE_TOOLS]")); - // assertTrue(built.contains("[INST]")); - // assertTrue(built.contains("Tell me the weather.")); - // assertTrue(built.contains("\"name\":\"getWeather\"")); - // assertTrue(built.contains("\"required\":[\"city\"]")); - // assertTrue(built.contains("\"enum\":[\"metric\",\"imperial\"]")); - // } -}