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.
This commit is contained in:
amithkoujalgi 2025-09-18 01:22:12 +05:30
parent fe87c4ccc8
commit 7788f954d6
No known key found for this signature in database
GPG Key ID: E29A37746AF94B70
15 changed files with 81 additions and 59 deletions

View File

@ -169,8 +169,6 @@
<artifactId>spotless-maven-plugin</artifactId>
<version>2.46.1</version>
<configuration>
<!-- &lt;!&ndash; optional: limit format enforcement to just the files changed by this feature branch &ndash;&gt;-->
<!-- <ratchetFrom>origin/main</ratchetFrom>-->
<formats>
<!-- you can define as many formats as you want, each is independent -->
<format>

View File

@ -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);
}
}

View File

@ -100,7 +100,8 @@ public class OllamaChatRequestBuilder {
OllamaChatMessageRole role,
String content,
List<OllamaChatToolCalls> toolCalls,
String... imageUrls) {
String... imageUrls)
throws IOException, InterruptedException {
List<OllamaChatMessage> messages = this.request.getMessages();
List<byte[]> 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;
}
}
}

View File

@ -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
* <pre>{@code json }</pre>
* or
* <pre>{@code {"key1": "val1", "key2": "val2"} }</pre>
*/
@JsonProperty(value = "format", required = false, defaultValue = "json")
protected Object format;

View File

@ -112,10 +112,8 @@ public class OllamaResult {
throw new IllegalArgumentException("Response is not a valid JSON object");
}
Map<String, Object> response =
getObjectMapper()
.readValue(responseStr, new TypeReference<Map<String, Object>>() {});
return response;
return getObjectMapper()
.readValue(responseStr, new TypeReference<Map<String, Object>>() {});
} catch (JsonProcessingException e) {
throw new IllegalArgumentException(
"Failed to parse response as JSON: " + e.getMessage(), e);

View File

@ -65,12 +65,8 @@ public class OllamaStructuredResult {
*/
public Map<String, Object> getStructuredResponse() {
try {
Map<String, Object> response =
getObjectMapper()
.readValue(
this.getResponse(),
new TypeReference<Map<String, Object>>() {});
return response;
return getObjectMapper()
.readValue(this.getResponse(), new TypeReference<Map<String, Object>>() {});
} catch (JsonProcessingException e) {
throw new RuntimeException(e);
}

View File

@ -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<String, Object> arguments) {
String city = (String) arguments.get(paramCityName);

View File

@ -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<OllamaChatToolCalls> 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<OllamaChatToolCalls> 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<OllamaChatToolCalls> 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<OllamaChatToolCalls> finalToolCalls =
* // chatResult.getResponseModel().getMessage().getToolCalls();
* // assertNull(finalToolCalls);
*/
}
@Test

View File

@ -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();

View File

@ -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() {}
});

View File

@ -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

View File

@ -104,11 +104,9 @@ public class TestChatRequestSerialization extends AbstractSerializationTest<Olla
assertThrowsExactly(
IllegalArgumentException.class,
() -> {
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<Olla
.build();
String jsonRequest = serialize(req);
// no jackson deserialization as format property is not boolean ==> 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");

View File

@ -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<OllamaEmbedRequestModel> {
class TestEmbedRequestSerialization extends AbstractSerializationTest<OllamaEmbedRequestModel> {
private OllamaEmbedRequestBuilder builder;

View File

@ -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<OllamaGenerateRequest> {
class TestGenerateRequestSerialization extends AbstractSerializationTest<OllamaGenerateRequest> {
private OllamaGenerateRequestBuilder builder;

View File

@ -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<ModelPullResponse> {
class TestModelPullResponseSerialization extends AbstractSerializationTest<ModelPullResponse> {
/**
* Test the specific error case reported in GitHub issue #138.