diff --git a/CHANGELOG.md b/CHANGELOG.md index 6171468c..3752ee27 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,7 @@ # Changelog ## [Unreleased](https://github.com/openfga/java-sdk/compare/v0.9.3...HEAD) +- feat: Improve error messaging by parsing error details from resp bodies (#256) ## v0.9.3 diff --git a/src/main/java/dev/openfga/sdk/errors/FgaError.java b/src/main/java/dev/openfga/sdk/errors/FgaError.java index 85f54d88..8c76c401 100644 --- a/src/main/java/dev/openfga/sdk/errors/FgaError.java +++ b/src/main/java/dev/openfga/sdk/errors/FgaError.java @@ -2,6 +2,8 @@ import static dev.openfga.sdk.errors.HttpStatusCode.*; +import com.fasterxml.jackson.databind.JsonNode; +import com.fasterxml.jackson.databind.ObjectMapper; import dev.openfga.sdk.api.configuration.Configuration; import dev.openfga.sdk.api.configuration.CredentialsMethod; import dev.openfga.sdk.constants.FgaConstants; @@ -11,6 +13,8 @@ import java.util.Optional; public class FgaError extends ApiException { + private static final ObjectMapper ERROR_MAPPER = new ObjectMapper(); + private String method = null; private String requestUrl = null; private String clientId = null; @@ -28,6 +32,62 @@ public FgaError(String message, int code, HttpHeaders responseHeaders, String re super(message, code, responseHeaders, responseBody); } + /** + * Parse the API error response body to extract the error message and code. + * @param methodName The API method name that was called + * @param responseBody The response body JSON string + * @return A descriptive error message + */ + private static String parseErrorMessage(String methodName, String responseBody) { + if (responseBody == null || responseBody.trim().isEmpty()) { + return methodName; + } + + try { + JsonNode jsonNode = ERROR_MAPPER.readTree(responseBody); + + // Try to extract message field + JsonNode messageNode = jsonNode.get("message"); + String message = (messageNode != null && !messageNode.isNull()) ? messageNode.asText() : null; + + // If we have a message, return it, otherwise fall back to method name + if (message != null && !message.trim().isEmpty()) { + return message; + } + } catch (Exception e) { + // If parsing fails, fall back to the method name + // This is intentional to ensure errors are still reported even if the response format is unexpected + } + + return methodName; + } + + /** + * Extract the API error code from the response body. + * @param responseBody The response body JSON string + * @return The error code, or null if not found + */ + private static String extractErrorCode(String responseBody) { + if (responseBody == null || responseBody.trim().isEmpty()) { + return null; + } + + try { + JsonNode jsonNode = ERROR_MAPPER.readTree(responseBody); + + // Try to extract code field + JsonNode codeNode = jsonNode.get("code"); + if (codeNode != null && !codeNode.isNull()) { + return codeNode.asText(); + } + } catch (Exception e) { + // If parsing fails, return null + // This is intentional - we still want to report the error even if we can't extract the code + } + + return null; + } + public static Optional getError( String name, HttpRequest request, @@ -43,25 +103,43 @@ public static Optional getError( final String body = response.body(); final var headers = response.headers(); + + // Parse the error message from the response body + final String errorMessage = parseErrorMessage(name, body); final FgaError error; if (status == BAD_REQUEST || status == UNPROCESSABLE_ENTITY) { - error = new FgaApiValidationError(name, previousError, status, headers, body); + error = new FgaApiValidationError(errorMessage, previousError, status, headers, body); } else if (status == UNAUTHORIZED || status == FORBIDDEN) { - error = new FgaApiAuthenticationError(name, previousError, status, headers, body); + error = new FgaApiAuthenticationError(errorMessage, previousError, status, headers, body); } else if (status == NOT_FOUND) { - error = new FgaApiNotFoundError(name, previousError, status, headers, body); + error = new FgaApiNotFoundError(errorMessage, previousError, status, headers, body); } else if (status == TOO_MANY_REQUESTS) { - error = new FgaApiRateLimitExceededError(name, previousError, status, headers, body); + error = new FgaApiRateLimitExceededError(errorMessage, previousError, status, headers, body); } else if (isServerError(status)) { - error = new FgaApiInternalError(name, previousError, status, headers, body); + error = new FgaApiInternalError(errorMessage, previousError, status, headers, body); } else { - error = new FgaError(name, previousError, status, headers, body); + error = new FgaError(errorMessage, previousError, status, headers, body); } error.setMethod(request.method()); error.setRequestUrl(configuration.getApiUrl()); + // Extract and set API error code from response body + String apiErrorCode = extractErrorCode(body); + if (apiErrorCode != null) { + error.setApiErrorCode(apiErrorCode); + } + + // Extract and set request ID from response headers if present + // Common request ID header names + Optional requestId = headers.firstValue("X-Request-Id") + .or(() -> headers.firstValue("x-request-id")) + .or(() -> headers.firstValue("Request-Id")); + if (requestId.isPresent()) { + error.setRequestId(requestId.get()); + } + // Extract and set Retry-After header if present Optional retryAfter = headers.firstValue(FgaConstants.RETRY_AFTER_HEADER_NAME); if (retryAfter.isPresent()) { @@ -135,6 +213,15 @@ public String getApiErrorCode() { return apiErrorCode; } + /** + * Get the API error code. + * This is an alias for getApiErrorCode() for convenience. + * @return The API error code from the response + */ + public String getCode() { + return apiErrorCode; + } + public void setRetryAfterHeader(String retryAfterHeader) { this.retryAfterHeader = retryAfterHeader; } diff --git a/src/test/java/dev/openfga/sdk/errors/FgaErrorTest.java b/src/test/java/dev/openfga/sdk/errors/FgaErrorTest.java new file mode 100644 index 00000000..90d4216e --- /dev/null +++ b/src/test/java/dev/openfga/sdk/errors/FgaErrorTest.java @@ -0,0 +1,326 @@ +package dev.openfga.sdk.errors; + +import static org.assertj.core.api.Assertions.assertThat; + +import dev.openfga.sdk.api.configuration.ClientConfiguration; +import java.net.URI; +import java.net.http.HttpClient; +import java.net.http.HttpHeaders; +import java.net.http.HttpRequest; +import java.net.http.HttpResponse; +import java.util.List; +import java.util.Map; +import java.util.Optional; +import org.junit.jupiter.api.Test; + +class FgaErrorTest { + + @Test + void shouldParseValidationErrorMessageFromResponseBody() { + // Given + String responseBody = "{\"code\":\"validation_error\",\"message\":\"invalid relation 'foo'\"}"; + HttpResponse response = createMockResponse(400, responseBody, Map.of()); + + HttpRequest request = HttpRequest.newBuilder() + .uri(URI.create("http://localhost:8080/test")) + .POST(HttpRequest.BodyPublishers.noBody()) + .build(); + + ClientConfiguration config = new ClientConfiguration().apiUrl("http://localhost:8080"); + + // When + Optional errorOpt = FgaError.getError("write", request, config, response, null); + + // Then + assertThat(errorOpt).isPresent(); + FgaError error = errorOpt.get(); + assertThat(error).isInstanceOf(FgaApiValidationError.class); + assertThat(error.getMessage()).isEqualTo("invalid relation 'foo'"); + assertThat(error.getCode()).isEqualTo("validation_error"); + assertThat(error.getApiErrorCode()).isEqualTo("validation_error"); + assertThat(error.getStatusCode()).isEqualTo(400); + assertThat(error.getMethod()).isEqualTo("POST"); + } + + @Test + void shouldParseInternalErrorMessageFromResponseBody() { + // Given + String responseBody = "{\"code\":\"internal_error\",\"message\":\"database connection failed\"}"; + HttpResponse response = createMockResponse(500, responseBody, Map.of()); + + HttpRequest request = HttpRequest.newBuilder() + .uri(URI.create("http://localhost:8080/test")) + .GET() + .build(); + + ClientConfiguration config = new ClientConfiguration().apiUrl("http://localhost:8080"); + + // When + Optional errorOpt = FgaError.getError("check", request, config, response, null); + + // Then + assertThat(errorOpt).isPresent(); + FgaError error = errorOpt.get(); + assertThat(error).isInstanceOf(FgaApiInternalError.class); + assertThat(error.getMessage()).isEqualTo("database connection failed"); + assertThat(error.getCode()).isEqualTo("internal_error"); + assertThat(error.getStatusCode()).isEqualTo(500); + assertThat(error.getMethod()).isEqualTo("GET"); + } + + @Test + void shouldParseNotFoundErrorMessageFromResponseBody() { + // Given + String responseBody = "{\"code\":\"store_id_not_found\",\"message\":\"store not found\"}"; + HttpResponse response = createMockResponse(404, responseBody, Map.of()); + + HttpRequest request = HttpRequest.newBuilder() + .uri(URI.create("http://localhost:8080/test")) + .GET() + .build(); + + ClientConfiguration config = new ClientConfiguration().apiUrl("http://localhost:8080"); + + // When + Optional errorOpt = FgaError.getError("getStore", request, config, response, null); + + // Then + assertThat(errorOpt).isPresent(); + FgaError error = errorOpt.get(); + assertThat(error).isInstanceOf(FgaApiNotFoundError.class); + assertThat(error.getMessage()).isEqualTo("store not found"); + assertThat(error.getCode()).isEqualTo("store_id_not_found"); + assertThat(error.getStatusCode()).isEqualTo(404); + } + + @Test + void shouldFallBackToMethodNameWhenMessageIsMissing() { + // Given + String responseBody = "{\"code\":\"validation_error\"}"; + HttpResponse response = createMockResponse(400, responseBody, Map.of()); + + HttpRequest request = HttpRequest.newBuilder() + .uri(URI.create("http://localhost:8080/test")) + .POST(HttpRequest.BodyPublishers.noBody()) + .build(); + + ClientConfiguration config = new ClientConfiguration().apiUrl("http://localhost:8080"); + + // When + Optional errorOpt = FgaError.getError("write", request, config, response, null); + + // Then + assertThat(errorOpt).isPresent(); + FgaError error = errorOpt.get(); + assertThat(error.getMessage()).isEqualTo("write"); + assertThat(error.getCode()).isEqualTo("validation_error"); + } + + @Test + void shouldFallBackToMethodNameWhenResponseBodyIsEmpty() { + // Given + String responseBody = ""; + HttpResponse response = createMockResponse(500, responseBody, Map.of()); + + HttpRequest request = HttpRequest.newBuilder() + .uri(URI.create("http://localhost:8080/test")) + .POST(HttpRequest.BodyPublishers.noBody()) + .build(); + + ClientConfiguration config = new ClientConfiguration().apiUrl("http://localhost:8080"); + + // When + Optional errorOpt = FgaError.getError("write", request, config, response, null); + + // Then + assertThat(errorOpt).isPresent(); + FgaError error = errorOpt.get(); + assertThat(error.getMessage()).isEqualTo("write"); + assertThat(error.getCode()).isNull(); + } + + @Test + void shouldFallBackToMethodNameWhenResponseBodyIsNotJson() { + // Given + String responseBody = "Server Error"; + HttpResponse response = createMockResponse(500, responseBody, Map.of()); + + HttpRequest request = HttpRequest.newBuilder() + .uri(URI.create("http://localhost:8080/test")) + .POST(HttpRequest.BodyPublishers.noBody()) + .build(); + + ClientConfiguration config = new ClientConfiguration().apiUrl("http://localhost:8080"); + + // When + Optional errorOpt = FgaError.getError("write", request, config, response, null); + + // Then + assertThat(errorOpt).isPresent(); + FgaError error = errorOpt.get(); + assertThat(error.getMessage()).isEqualTo("write"); + assertThat(error.getCode()).isNull(); + } + + @Test + void shouldExtractRequestIdFromHeaders() { + // Given + String responseBody = "{\"code\":\"validation_error\",\"message\":\"invalid tuple\"}"; + Map> headers = Map.of("X-Request-Id", List.of("abc-123-def-456")); + HttpResponse response = createMockResponse(400, responseBody, headers); + + HttpRequest request = HttpRequest.newBuilder() + .uri(URI.create("http://localhost:8080/test")) + .POST(HttpRequest.BodyPublishers.noBody()) + .build(); + + ClientConfiguration config = new ClientConfiguration().apiUrl("http://localhost:8080"); + + // When + Optional errorOpt = FgaError.getError("write", request, config, response, null); + + // Then + assertThat(errorOpt).isPresent(); + FgaError error = errorOpt.get(); + assertThat(error.getRequestId()).isEqualTo("abc-123-def-456"); + } + + @Test + void shouldHandleUnprocessableEntityAsValidationError() { + // Given + String responseBody = "{\"code\":\"invalid_tuple\",\"message\":\"tuple validation failed\"}"; + HttpResponse response = createMockResponse(422, responseBody, Map.of()); + + HttpRequest request = HttpRequest.newBuilder() + .uri(URI.create("http://localhost:8080/test")) + .POST(HttpRequest.BodyPublishers.noBody()) + .build(); + + ClientConfiguration config = new ClientConfiguration().apiUrl("http://localhost:8080"); + + // When + Optional errorOpt = FgaError.getError("write", request, config, response, null); + + // Then + assertThat(errorOpt).isPresent(); + FgaError error = errorOpt.get(); + assertThat(error).isInstanceOf(FgaApiValidationError.class); + assertThat(error.getMessage()).isEqualTo("tuple validation failed"); + assertThat(error.getCode()).isEqualTo("invalid_tuple"); + assertThat(error.getStatusCode()).isEqualTo(422); + } + + @Test + void shouldHandleAuthenticationError() { + // Given + String responseBody = "{\"code\":\"auth_failed\",\"message\":\"authentication failed\"}"; + HttpResponse response = createMockResponse(401, responseBody, Map.of()); + + HttpRequest request = HttpRequest.newBuilder() + .uri(URI.create("http://localhost:8080/test")) + .GET() + .build(); + + ClientConfiguration config = new ClientConfiguration().apiUrl("http://localhost:8080"); + + // When + Optional errorOpt = FgaError.getError("read", request, config, response, null); + + // Then + assertThat(errorOpt).isPresent(); + FgaError error = errorOpt.get(); + assertThat(error).isInstanceOf(FgaApiAuthenticationError.class); + assertThat(error.getMessage()).isEqualTo("authentication failed"); + assertThat(error.getCode()).isEqualTo("auth_failed"); + assertThat(error.getStatusCode()).isEqualTo(401); + } + + @Test + void shouldHandleRateLimitError() { + // Given + String responseBody = "{\"code\":\"rate_limit_exceeded\",\"message\":\"too many requests\"}"; + HttpResponse response = createMockResponse(429, responseBody, Map.of()); + + HttpRequest request = HttpRequest.newBuilder() + .uri(URI.create("http://localhost:8080/test")) + .GET() + .build(); + + ClientConfiguration config = new ClientConfiguration().apiUrl("http://localhost:8080"); + + // When + Optional errorOpt = FgaError.getError("check", request, config, response, null); + + // Then + assertThat(errorOpt).isPresent(); + FgaError error = errorOpt.get(); + assertThat(error).isInstanceOf(FgaApiRateLimitExceededError.class); + assertThat(error.getMessage()).isEqualTo("too many requests"); + assertThat(error.getCode()).isEqualTo("rate_limit_exceeded"); + assertThat(error.getStatusCode()).isEqualTo(429); + } + + @Test + void shouldReturnEmptyForSuccessfulResponse() { + // Given + HttpResponse response = createMockResponse(200, "{}", Map.of()); + + HttpRequest request = HttpRequest.newBuilder() + .uri(URI.create("http://localhost:8080/test")) + .GET() + .build(); + + ClientConfiguration config = new ClientConfiguration().apiUrl("http://localhost:8080"); + + // When + Optional errorOpt = FgaError.getError("read", request, config, response, null); + + // Then + assertThat(errorOpt).isEmpty(); + } + + // Helper method to create mock HttpResponse + private HttpResponse createMockResponse(int statusCode, String body, Map> headers) { + return new HttpResponse() { + @Override + public int statusCode() { + return statusCode; + } + + @Override + public HttpRequest request() { + return null; + } + + @Override + public Optional> previousResponse() { + return Optional.empty(); + } + + @Override + public HttpHeaders headers() { + return HttpHeaders.of(headers, (k, v) -> true); + } + + @Override + public String body() { + return body; + } + + @Override + public Optional sslSession() { + return Optional.empty(); + } + + @Override + public URI uri() { + return URI.create("http://localhost:8080/test"); + } + + @Override + public HttpClient.Version version() { + return HttpClient.Version.HTTP_1_1; + } + }; + } +} \ No newline at end of file