From 68a421aa72fb5c0e99cbfa643261f6b8db330506 Mon Sep 17 00:00:00 2001 From: otopba Date: Fri, 3 Apr 2026 20:30:45 +0300 Subject: [PATCH] fix: include error response body and status code in FalException Previously, responseToException() parsed the JSON error body but discarded it, creating FalException with only "Request failed with code: NNN". This lost valuable error details from the API (validation messages, error descriptions, etc.). Changes: - Extract human-readable messages from common error response formats: {"detail": [{"msg": "..."}]}, {"message": "..."}, {"error": "..."} - Add statusCode and body fields to FalException for programmatic access - Maintain full backward compatibility with existing constructors - Add 23 unit tests covering all parsing paths and edge cases Co-Authored-By: Claude Opus 4.6 (1M context) --- client/build.gradle.kts | 3 + .../ai/fal/client/exception/FalException.java | 37 +++ .../java/ai/fal/client/http/HttpClient.java | 66 +++- .../ai/fal/client/http/HttpClientTest.java | 283 ++++++++++++++++++ 4 files changed, 387 insertions(+), 2 deletions(-) create mode 100644 client/src/test/java/ai/fal/client/http/HttpClientTest.java diff --git a/client/build.gradle.kts b/client/build.gradle.kts index 9fd5aab..d1379ee 100644 --- a/client/build.gradle.kts +++ b/client/build.gradle.kts @@ -31,6 +31,9 @@ dependencies { compileOnly("org.projectlombok:lombok:1.18.34") annotationProcessor("org.projectlombok:lombok:1.18.34") + + testImplementation("org.junit.jupiter:junit-jupiter:5.10.3") + testImplementation("org.assertj:assertj-core:3.26.0") } tasks.withType { diff --git a/client/src/main/java/ai/fal/client/exception/FalException.java b/client/src/main/java/ai/fal/client/exception/FalException.java index a387f7a..774d744 100644 --- a/client/src/main/java/ai/fal/client/exception/FalException.java +++ b/client/src/main/java/ai/fal/client/exception/FalException.java @@ -10,23 +10,60 @@ public class FalException extends RuntimeException { @Nullable private final String requestId; + private final int statusCode; + + @Nullable + private final String body; + public FalException(@Nonnull String message, @Nullable String requestId) { super(requireNonNull(message)); this.requestId = requestId; + this.statusCode = -1; + this.body = null; + } + + public FalException( + @Nonnull String message, + @Nullable String requestId, + int statusCode, + @Nullable String body) { + super(requireNonNull(message)); + this.requestId = requestId; + this.statusCode = statusCode; + this.body = body; } public FalException(@Nonnull String message, @Nonnull Throwable cause, @Nullable String requestId) { super(requireNonNull(message), cause); this.requestId = requestId; + this.statusCode = -1; + this.body = null; } public FalException(Throwable cause) { super(cause); this.requestId = null; + this.statusCode = -1; + this.body = null; } @Nullable public String getRequestId() { return this.requestId; } + + /** + * Returns the HTTP status code of the failed response, or -1 if not available. + */ + public int getStatusCode() { + return this.statusCode; + } + + /** + * Returns the raw response body of the failed response, or null if not available. + */ + @Nullable + public String getBody() { + return this.body; + } } diff --git a/client/src/main/java/ai/fal/client/http/HttpClient.java b/client/src/main/java/ai/fal/client/http/HttpClient.java index 84183c6..cd1e150 100644 --- a/client/src/main/java/ai/fal/client/http/HttpClient.java +++ b/client/src/main/java/ai/fal/client/http/HttpClient.java @@ -8,6 +8,7 @@ import com.google.gson.JsonElement; import jakarta.annotation.Nonnull; import java.io.IOException; +import java.util.ArrayList; import java.util.Collections; import java.util.Map; import java.util.Optional; @@ -101,15 +102,76 @@ public T handleResponse(Response response, Class resultType) { public FalException responseToException(Response response) { final var requestId = response.header(HEADER_REQUEST_ID); + final var statusCode = response.code(); final var contentType = response.header("content-type"); + String bodyString = null; + if (contentType != null && contentType.contains("application/json")) { final var body = response.body(); if (body != null) { - final var json = gson.fromJson(body.charStream(), JsonElement.class); + try { + bodyString = body.string(); + } catch (IOException ignored) { + } + } + } + + var message = "Request failed with code: " + statusCode; + if (bodyString != null) { + final var detail = extractDetailMessage(bodyString); + if (detail != null) { + message += ": " + detail; } } - return new FalException("Request failed with code: " + response.code(), requestId); + return new FalException(message, requestId, statusCode, bodyString); + } + + private String extractDetailMessage(String bodyString) { + try { + final var json = gson.fromJson(bodyString, JsonElement.class); + if (json == null || !json.isJsonObject()) { + return null; + } + final var jsonObject = json.getAsJsonObject(); + + // Handle {"detail": [{"msg": "...", ...}, ...]} format (validation errors) + if (jsonObject.has("detail")) { + final var detail = jsonObject.get("detail"); + if (detail.isJsonArray()) { + final var messages = new ArrayList(); + for (final var item : detail.getAsJsonArray()) { + if (item.isJsonObject()) { + final var itemObj = item.getAsJsonObject(); + if (itemObj.has("msg")) { + messages.add(itemObj.get("msg").getAsString()); + } + } + } + if (!messages.isEmpty()) { + return String.join("; ", messages); + } + } + if (detail.isJsonPrimitive() && detail.getAsJsonPrimitive().isString()) { + return detail.getAsString(); + } + } + + // Handle {"message": "..."} format + if (jsonObject.has("message")) { + return jsonObject.get("message").getAsString(); + } + + // Handle {"error": "..."} format + if (jsonObject.has("error")) { + final var error = jsonObject.get("error"); + if (error.isJsonPrimitive()) { + return error.getAsString(); + } + } + } catch (Exception ignored) { + } + return null; } public Output wrapInResult(Response response, Class resultType) { diff --git a/client/src/test/java/ai/fal/client/http/HttpClientTest.java b/client/src/test/java/ai/fal/client/http/HttpClientTest.java new file mode 100644 index 0000000..0aaaaeb --- /dev/null +++ b/client/src/test/java/ai/fal/client/http/HttpClientTest.java @@ -0,0 +1,283 @@ +package ai.fal.client.http; + +import static org.assertj.core.api.Assertions.assertThat; + +import ai.fal.client.ClientConfig; +import ai.fal.client.CredentialsResolver; +import ai.fal.client.exception.FalException; +import java.io.IOException; +import okhttp3.MediaType; +import okhttp3.OkHttpClient; +import okhttp3.Protocol; +import okhttp3.Request; +import okhttp3.Response; +import okhttp3.ResponseBody; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; + +class HttpClientTest { + + private HttpClient httpClient; + + private static final MediaType JSON = MediaType.parse("application/json"); + + @BeforeEach + void setUp() { + var config = ClientConfig.withCredentials(CredentialsResolver.fromApiKey("test-key")); + httpClient = new HttpClient(config, new OkHttpClient()); + } + + private Response buildResponse(int code, String contentType, String body) { + var builder = new Response.Builder() + .request(new Request.Builder().url("https://queue.fal.run/test").build()) + .protocol(Protocol.HTTP_1_1) + .code(code) + .message("Error"); + + if (body != null && contentType != null) { + builder.header("content-type", contentType); + builder.body(ResponseBody.create(body, MediaType.parse(contentType))); + } else { + builder.body(ResponseBody.create("", MediaType.parse("text/plain"))); + } + + return builder.build(); + } + + // --- Validation error format: {"detail": [{"msg": "...", ...}]} --- + + @Test + void responseToException_validationError_extractsMessage() { + var body = + "{\"detail\":[{\"loc\":[\"body\",\"prompt\"],\"msg\":\"The prompt may contain intellectual property references that cannot be processed.\",\"type\":\"value_error\"}]}"; + + var ex = httpClient.responseToException(buildResponse(422, "application/json", body)); + + assertThat(ex.getMessage()) + .isEqualTo( + "Request failed with code: 422: The prompt may contain intellectual property references that cannot be processed."); + assertThat(ex.getStatusCode()).isEqualTo(422); + assertThat(ex.getBody()).isEqualTo(body); + } + + @Test + void responseToException_multipleValidationErrors_joinsThem() { + var body = + "{\"detail\":[{\"msg\":\"Field is required\",\"type\":\"missing\"},{\"msg\":\"Invalid format\",\"type\":\"value_error\"}]}"; + + var ex = httpClient.responseToException(buildResponse(422, "application/json", body)); + + assertThat(ex.getMessage()) + .isEqualTo("Request failed with code: 422: Field is required; Invalid format"); + } + + @Test + void responseToException_detailArrayWithItemsMissingMsg_skipsItems() { + var body = + "{\"detail\":[{\"type\":\"missing\"},{\"msg\":\"Has message\",\"type\":\"value_error\"}]}"; + + var ex = httpClient.responseToException(buildResponse(422, "application/json", body)); + + assertThat(ex.getMessage()).isEqualTo("Request failed with code: 422: Has message"); + } + + @Test + void responseToException_detailArrayAllItemsMissingMsg_noDetailInMessage() { + var body = "{\"detail\":[{\"type\":\"missing\"},{\"type\":\"value_error\"}]}"; + + var ex = httpClient.responseToException(buildResponse(422, "application/json", body)); + + assertThat(ex.getMessage()).isEqualTo("Request failed with code: 422"); + } + + @Test + void responseToException_detailAsString_extractsIt() { + var body = "{\"detail\":\"Not authenticated\"}"; + + var ex = httpClient.responseToException(buildResponse(401, "application/json", body)); + + assertThat(ex.getMessage()).isEqualTo("Request failed with code: 401: Not authenticated"); + assertThat(ex.getStatusCode()).isEqualTo(401); + } + + @Test + void responseToException_emptyDetailArray_noDetailInMessage() { + var body = "{\"detail\":[]}"; + + var ex = httpClient.responseToException(buildResponse(422, "application/json", body)); + + assertThat(ex.getMessage()).isEqualTo("Request failed with code: 422"); + } + + // --- {"message": "..."} format --- + + @Test + void responseToException_messageFormat_extractsMessage() { + var body = "{\"message\":\"Internal server error\"}"; + + var ex = httpClient.responseToException(buildResponse(500, "application/json", body)); + + assertThat(ex.getMessage()).isEqualTo("Request failed with code: 500: Internal server error"); + } + + // --- {"error": "..."} format --- + + @Test + void responseToException_errorStringFormat_extractsError() { + var body = "{\"error\":\"Rate limit exceeded\"}"; + + var ex = httpClient.responseToException(buildResponse(429, "application/json", body)); + + assertThat(ex.getMessage()).isEqualTo("Request failed with code: 429: Rate limit exceeded"); + } + + @Test + void responseToException_errorObjectFormat_noDetailInMessage() { + var body = "{\"error\":{\"code\":\"RATE_LIMIT\",\"info\":\"too many requests\"}}"; + + var ex = httpClient.responseToException(buildResponse(429, "application/json", body)); + + assertThat(ex.getMessage()).isEqualTo("Request failed with code: 429"); + assertThat(ex.getBody()).isEqualTo(body); + } + + // --- Priority: detail > message > error --- + + @Test + void responseToException_detailTakesPriorityOverMessage() { + var body = "{\"detail\":\"Auth failed\",\"message\":\"Something went wrong\"}"; + + var ex = httpClient.responseToException(buildResponse(401, "application/json", body)); + + assertThat(ex.getMessage()).isEqualTo("Request failed with code: 401: Auth failed"); + } + + // --- Non-JSON and edge cases --- + + @Test + void responseToException_nonJsonContentType_noBodyParsed() { + var ex = httpClient.responseToException(buildResponse(500, "text/html", "

Error

")); + + assertThat(ex.getMessage()).isEqualTo("Request failed with code: 500"); + assertThat(ex.getBody()).isNull(); + assertThat(ex.getStatusCode()).isEqualTo(500); + } + + @Test + void responseToException_invalidJson_noDetailInMessage() { + var ex = httpClient.responseToException( + buildResponse(500, "application/json", "not json at all")); + + assertThat(ex.getMessage()).isEqualTo("Request failed with code: 500"); + assertThat(ex.getBody()).isEqualTo("not json at all"); + } + + @Test + void responseToException_emptyJsonObject_noDetailInMessage() { + var ex = httpClient.responseToException(buildResponse(500, "application/json", "{}")); + + assertThat(ex.getMessage()).isEqualTo("Request failed with code: 500"); + } + + @Test + void responseToException_jsonArray_noDetailInMessage() { + var ex = httpClient.responseToException(buildResponse(500, "application/json", "[1,2,3]")); + + assertThat(ex.getMessage()).isEqualTo("Request failed with code: 500"); + } + + @Test + void responseToException_emptyBody_noDetailInMessage() { + var ex = httpClient.responseToException(buildResponse(500, "application/json", "")); + + assertThat(ex.getMessage()).isEqualTo("Request failed with code: 500"); + } + + @Test + void responseToException_nullFields_noDetailInMessage() { + var body = "{\"detail\":null,\"message\":null,\"error\":null}"; + + var ex = httpClient.responseToException(buildResponse(500, "application/json", body)); + + assertThat(ex.getMessage()).isEqualTo("Request failed with code: 500"); + } + + @Test + void responseToException_detailNumberNotString_noDetailInMessage() { + var body = "{\"detail\":42}"; + + var ex = httpClient.responseToException(buildResponse(400, "application/json", body)); + + assertThat(ex.getMessage()).isEqualTo("Request failed with code: 400"); + } + + @Test + void responseToException_detailBooleanNotString_noDetailInMessage() { + var body = "{\"detail\":true}"; + + var ex = httpClient.responseToException(buildResponse(400, "application/json", body)); + + assertThat(ex.getMessage()).isEqualTo("Request failed with code: 400"); + } + + // --- requestId header --- + + @Test + void responseToException_preservesRequestId() { + var response = new Response.Builder() + .request(new Request.Builder().url("https://queue.fal.run/test").build()) + .protocol(Protocol.HTTP_1_1) + .code(422) + .message("Error") + .header("X-Fal-Request-Id", "req-123") + .header("content-type", "application/json") + .body(ResponseBody.create("{\"detail\":\"test\"}", JSON)) + .build(); + + var ex = httpClient.responseToException(response); + + assertThat(ex.getRequestId()).isEqualTo("req-123"); + assertThat(ex.getMessage()).isEqualTo("Request failed with code: 422: test"); + } + + @Test + void responseToException_noRequestIdHeader_returnsNull() { + var ex = httpClient.responseToException(buildResponse(500, "application/json", "{}")); + + assertThat(ex.getRequestId()).isNull(); + } + + // --- FalException backward compatibility --- + + @Test + void falException_legacyConstructor_defaultsStatusCodeAndBody() { + var ex = new FalException("some error", "req-1"); + + assertThat(ex.getMessage()).isEqualTo("some error"); + assertThat(ex.getRequestId()).isEqualTo("req-1"); + assertThat(ex.getStatusCode()).isEqualTo(-1); + assertThat(ex.getBody()).isNull(); + } + + @Test + void falException_causeConstructor_defaultsStatusCodeAndBody() { + var cause = new RuntimeException("cause"); + var ex = new FalException("wrapped", cause, "req-2"); + + assertThat(ex.getMessage()).isEqualTo("wrapped"); + assertThat(ex.getCause()).isEqualTo(cause); + assertThat(ex.getStatusCode()).isEqualTo(-1); + assertThat(ex.getBody()).isNull(); + } + + @Test + void falException_throwableOnlyConstructor_defaultsAll() { + var cause = new IOException("network"); + var ex = new FalException(cause); + + assertThat(ex.getCause()).isEqualTo(cause); + assertThat(ex.getRequestId()).isNull(); + assertThat(ex.getStatusCode()).isEqualTo(-1); + assertThat(ex.getBody()).isNull(); + } +}