From 26d603b5647c36f1cda9d0766c93bd0e76e19331 Mon Sep 17 00:00:00 2001 From: SoulPancake Date: Fri, 14 Nov 2025 12:14:30 +0530 Subject: [PATCH 01/10] feat: improve errors, relay api err msg --- .../java/dev/openfga/sdk/errors/FgaError.java | 55 +++++ .../sdk/errors/FgaErrorIntegrationTest.java | 211 ++++++++++++++++++ 2 files changed, 266 insertions(+) create mode 100644 src/test-integration/java/dev/openfga/sdk/errors/FgaErrorIntegrationTest.java diff --git a/src/main/java/dev/openfga/sdk/errors/FgaError.java b/src/main/java/dev/openfga/sdk/errors/FgaError.java index 85f54d88..24d19cbc 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.core.JsonProcessingException; +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; @@ -19,6 +21,13 @@ public class FgaError extends ApiException { private String requestId = null; private String apiErrorCode = null; private String retryAfterHeader = null; + private String apiErrorMessage = null; + private String operationName = null; + + public static class ApiErrorResponse { + public String code; + public String message; + } public FgaError(String message, Throwable cause, int code, HttpHeaders responseHeaders, String responseBody) { super(message, cause, code, responseHeaders, responseBody); @@ -75,6 +84,26 @@ public static Optional getError( error.setAudience(clientCredentials.getApiAudience()); } + error.setOperationName(name); + + // Parse API error response + if (body != null && !body.trim().isEmpty()) { + ObjectMapper mapper = new ObjectMapper(); + try { + ApiErrorResponse resp = mapper.readValue(body, ApiErrorResponse.class); + error.setApiErrorCode(resp.code); + error.setApiErrorMessage(resp.message); + } catch (JsonProcessingException e) { + // Fall back, do nothing + } + } + + // Extract requestId from headers + Optional requestIdOpt = headers.firstValue("x-request-id"); + if (requestIdOpt.isPresent()) { + error.setRequestId(requestIdOpt.get()); + } + // Unknown error return Optional.of(error); } @@ -142,4 +171,30 @@ public void setRetryAfterHeader(String retryAfterHeader) { public String getRetryAfterHeader() { return retryAfterHeader; } + + public void setApiErrorMessage(String apiErrorMessage) { + this.apiErrorMessage = apiErrorMessage; + } + + public String getApiErrorMessage() { + return apiErrorMessage; + } + + public void setOperationName(String operationName) { + this.operationName = operationName; + } + + public String getOperationName() { + return operationName; + } + + @Override + public String getMessage() { + if (apiErrorMessage != null && !apiErrorMessage.isEmpty() && operationName != null) { + String codePart = (apiErrorCode != null && !apiErrorCode.isEmpty()) ? " (" + apiErrorCode + ")" : ""; + return String.format("[%s] %s%s", operationName, apiErrorMessage, codePart); + } else { + return super.getMessage(); + } + } } diff --git a/src/test-integration/java/dev/openfga/sdk/errors/FgaErrorIntegrationTest.java b/src/test-integration/java/dev/openfga/sdk/errors/FgaErrorIntegrationTest.java new file mode 100644 index 00000000..f56c2329 --- /dev/null +++ b/src/test-integration/java/dev/openfga/sdk/errors/FgaErrorIntegrationTest.java @@ -0,0 +1,211 @@ +package dev.openfga.sdk.errors; + +import static org.junit.jupiter.api.Assertions.*; + +import com.fasterxml.jackson.databind.ObjectMapper; +import dev.openfga.sdk.api.client.OpenFgaClient; +import dev.openfga.sdk.api.client.model.ClientTupleKey; +import dev.openfga.sdk.api.client.model.ClientWriteRequest; +import dev.openfga.sdk.api.configuration.ClientConfiguration; +import dev.openfga.sdk.api.model.CreateStoreRequest; +import dev.openfga.sdk.api.model.WriteAuthorizationModelRequest; +import java.io.IOException; +import java.nio.file.Files; +import java.nio.file.Paths; +import java.util.List; +import java.util.concurrent.ExecutionException; +import org.junit.jupiter.api.BeforeAll; +import org.junit.jupiter.api.BeforeEach; +import org.junit.jupiter.api.Test; +import org.junit.jupiter.api.TestInstance; +import org.junit.jupiter.api.TestInstance.Lifecycle; +import org.testcontainers.junit.jupiter.Container; +import org.testcontainers.junit.jupiter.Testcontainers; +import org.testcontainers.openfga.OpenFGAContainer; + +@TestInstance(Lifecycle.PER_CLASS) +@Testcontainers +public class FgaErrorIntegrationTest { + + @Container + private static final OpenFGAContainer openfga = new OpenFGAContainer("openfga/openfga:v1.10.2"); + + private static final ObjectMapper mapper = new ObjectMapper().findAndRegisterModules(); + private String authModelJson; + private OpenFgaClient fga; + + @BeforeAll + public void loadAuthModelJson() throws IOException { + authModelJson = Files.readString(Paths.get("src", "test-integration", "resources", "auth-model.json")); + } + + @BeforeEach + public void initializeApi() throws Exception { + ClientConfiguration apiConfig = new ClientConfiguration().apiUrl(openfga.getHttpEndpoint()); + fga = new OpenFgaClient(apiConfig); + + // Create a store + String storeName = "ErrorTestStore"; + var createStoreResponse = + fga.createStore(new CreateStoreRequest().name(storeName)).get(); + String storeId = createStoreResponse.getId(); + fga.setStoreId(storeId); + + // Write the authorization model + WriteAuthorizationModelRequest writeModelRequest = + mapper.readValue(authModelJson, WriteAuthorizationModelRequest.class); + fga.writeAuthorizationModel(writeModelRequest).get(); + } + + @Test + public void testWriteValidationError() throws Exception { + // Try to write a tuple with invalid type + ClientWriteRequest request = new ClientWriteRequest() + .writes(List.of(new ClientTupleKey() + ._object("invalid_type:readme") + .relation("viewer") + .user("user:anne"))); + + ExecutionException executionException = assertThrows(ExecutionException.class, () -> { + fga.write(request).get(); + }); + + FgaApiValidationError exception = assertInstanceOf(FgaApiValidationError.class, executionException.getCause()); + assertTrue(exception.getMessage().contains("type 'invalid_type' not found")); + assertEquals("validation_error", exception.getApiErrorCode()); + assertTrue(exception.getApiErrorMessage().contains("type 'invalid_type' not found")); + assertEquals("write", exception.getOperationName()); + assertNotNull(exception.getRequestId()); + } + + @Test + public void testWriteValidationErrorWithInvalidRelation() throws Exception { + // Try to write a tuple with valid type but invalid relation + ClientWriteRequest request = new ClientWriteRequest() + .writes(List.of(new ClientTupleKey() + ._object("document:readme") + .relation("invalid_relation") + .user("user:anne"))); + + ExecutionException executionException = assertThrows(ExecutionException.class, () -> { + fga.write(request).get(); + }); + + FgaApiValidationError exception = assertInstanceOf(FgaApiValidationError.class, executionException.getCause()); + // Verify the formatted message includes operation name and API error details + assertTrue(exception.getMessage().contains("write")); + assertTrue(exception.getMessage().contains("relation 'document#invalid_relation' not found")); + assertEquals("validation_error", exception.getApiErrorCode()); + assertNotNull(exception.getApiErrorMessage()); + assertEquals("write", exception.getOperationName()); + assertNotNull(exception.getRequestId()); + } + + @Test + public void testErrorMessageFormattingWithAllFields() throws Exception { + // Verify that when all fields are present, the message is properly formatted + ClientWriteRequest request = new ClientWriteRequest() + .writes(List.of(new ClientTupleKey() + ._object("invalid_type:readme") + .relation("viewer") + .user("user:anne"))); + + ExecutionException executionException = assertThrows(ExecutionException.class, () -> { + fga.write(request).get(); + }); + + FgaApiValidationError exception = assertInstanceOf(FgaApiValidationError.class, executionException.getCause()); + + // Verify the message follows the format: [operationName] apiErrorMessage (apiErrorCode) + String message = exception.getMessage(); + assertTrue(message.startsWith("[write]"), "Message should start with [write]"); + assertTrue(message.contains("type 'invalid_type' not found"), "Message should contain the API error message"); + assertTrue(message.endsWith("(validation_error)"), "Message should end with (validation_error)"); + + // Verify individual fields are accessible + assertEquals("write", exception.getOperationName()); + assertEquals("validation_error", exception.getApiErrorCode()); + assertNotNull(exception.getApiErrorMessage()); + assertNotNull(exception.getRequestId()); + } + + @Test + public void testCheckValidationError() throws Exception { + // Test error handling for check operation to verify operation name is set correctly + var checkRequest = new dev.openfga.sdk.api.client.model.ClientCheckRequest() + ._object("invalid_type:readme") + .relation("viewer") + .user("user:anne"); + + ExecutionException executionException = assertThrows(ExecutionException.class, () -> { + fga.check(checkRequest).get(); + }); + + FgaApiValidationError exception = assertInstanceOf(FgaApiValidationError.class, executionException.getCause()); + + // Verify operation name is "check" not "write" + assertEquals("check", exception.getOperationName()); + assertTrue(exception.getMessage().contains("[check]")); + assertEquals("validation_error", exception.getApiErrorCode()); + assertNotNull(exception.getApiErrorMessage()); + assertNotNull(exception.getRequestId()); + } + + @Test + public void testErrorDetailsAreNotLostInStackTrace() throws Exception { + // Verify that error details are preserved when exception is thrown + // This addresses the issue where details were "buried down in the exception stack" + ClientWriteRequest request = new ClientWriteRequest() + .writes(List.of(new ClientTupleKey() + ._object("invalid_type:readme") + .relation("viewer") + .user("user:anne"))); + + try { + fga.write(request).get(); + fail("Expected ExecutionException to be thrown"); + } catch (ExecutionException e) { + FgaApiValidationError exception = assertInstanceOf(FgaApiValidationError.class, e.getCause()); + + // Verify that calling toString() or getMessage() gives useful information + String errorString = exception.toString(); + assertTrue( + errorString.contains("type 'invalid_type' not found"), + "toString() should contain the API error message"); + + String errorMessage = exception.getMessage(); + assertTrue(errorMessage.contains("[write]"), "getMessage() should contain operation name"); + assertTrue(errorMessage.contains("validation_error"), "getMessage() should contain error code"); + + // Verify the response body is still accessible for custom parsing if needed + assertNotNull(exception.getResponseData(), "Response body should be available"); + } + } + + @Test + public void testMultipleTupleErrorsShowDetailedMessage() throws Exception { + // Test that validation errors for multiple tuples still show useful information + ClientWriteRequest request = new ClientWriteRequest() + .writes(List.of( + new ClientTupleKey() + ._object("invalid_type1:readme") + .relation("viewer") + .user("user:anne"), + new ClientTupleKey() + ._object("invalid_type2:readme") + .relation("viewer") + .user("user:bob"))); + + ExecutionException executionException = assertThrows(ExecutionException.class, () -> { + fga.write(request).get(); + }); + + FgaApiValidationError exception = assertInstanceOf(FgaApiValidationError.class, executionException.getCause()); + + // The error message should be informative even for batch operations + assertTrue(exception.getMessage().contains("write")); + assertEquals("validation_error", exception.getApiErrorCode()); + assertNotNull(exception.getApiErrorMessage()); + assertNotNull(exception.getRequestId()); + } +} From 8c4898d2810f762215c4ae5a503ecfd88fa85a72 Mon Sep 17 00:00:00 2001 From: SoulPancake Date: Fri, 14 Nov 2025 13:09:02 +0530 Subject: [PATCH 02/10] feat: more tcs --- .../sdk/errors/FgaErrorIntegrationTest.java | 170 ++++++++++++++++++ 1 file changed, 170 insertions(+) diff --git a/src/test-integration/java/dev/openfga/sdk/errors/FgaErrorIntegrationTest.java b/src/test-integration/java/dev/openfga/sdk/errors/FgaErrorIntegrationTest.java index f56c2329..da35119f 100644 --- a/src/test-integration/java/dev/openfga/sdk/errors/FgaErrorIntegrationTest.java +++ b/src/test-integration/java/dev/openfga/sdk/errors/FgaErrorIntegrationTest.java @@ -208,4 +208,174 @@ public void testMultipleTupleErrorsShowDetailedMessage() throws Exception { assertNotNull(exception.getApiErrorMessage()); assertNotNull(exception.getRequestId()); } + + @Test + public void testNotFoundError() throws Exception { + ClientConfiguration badConfig = new ClientConfiguration() + .apiUrl(openfga.getHttpEndpoint()) + .storeId("01HVJPQR3TXYZ9NQXABCDEFGHI"); // Non-existent store ID + OpenFgaClient badClient = new OpenFgaClient(badConfig); + + ExecutionException executionException = assertThrows(ExecutionException.class, () -> { + badClient.getStore().get(); + }); + + FgaError exception = assertInstanceOf(FgaError.class, executionException.getCause()); + + assertNotNull(exception.getMessage()); + assertTrue(exception.getMessage().contains("getStore") + || exception.getMessage().contains("[get")); + assertNotNull(exception.getApiErrorCode()); + assertNotNull(exception.getApiErrorMessage()); + assertNotNull(exception.getRequestId()); + } + + @Test + public void testInvalidParameterException() throws Exception { + ClientWriteRequest request = new ClientWriteRequest(); // Empty request, no writes or deletes + + // This should throw FgaInvalidParameterException before making the API call + dev.openfga.sdk.errors.FgaInvalidParameterException exception = + assertThrows(dev.openfga.sdk.errors.FgaInvalidParameterException.class, () -> { + // Trying to write with null storeId by creating a new client without storeId + ClientConfiguration config = new ClientConfiguration().apiUrl(openfga.getHttpEndpoint()); + OpenFgaClient client = new OpenFgaClient(config); + client.write(request).get(); + }); + + assertNotNull(exception.getMessage()); + assertTrue(exception.getMessage().contains("storeId") + || exception.getMessage().contains("parameter")); + } + + @Test + public void testAuthenticationError() throws Exception { + // Test FgaApiAuthenticationError when store doesn't exist (403/401 from API) + // Use a non-existent store ID to trigger authentication/authorization error + ClientConfiguration badConfig = + new ClientConfiguration().apiUrl(openfga.getHttpEndpoint()).storeId("non-existent-store-id-12345"); + OpenFgaClient badClient = new OpenFgaClient(badConfig); + + ClientWriteRequest request = new ClientWriteRequest() + .writes(List.of(new ClientTupleKey() + ._object("document:readme") + .relation("viewer") + .user("user:anne"))); + + ExecutionException executionException = assertThrows(ExecutionException.class, () -> { + badClient.write(request).get(); + }); + + // Could be NotFoundError (404) or AuthenticationError depending on API version + FgaError exception = assertInstanceOf(FgaError.class, executionException.getCause()); + + // Verify error details are populated + assertNotNull(exception.getMessage()); + assertTrue(exception.getMessage().contains("[write]") + || exception.getMessage().contains("write")); + assertNotNull(exception.getApiErrorCode()); + assertNotNull(exception.getRequestId()); + } + + @Test + public void testReadOperationError() throws Exception { + // Test that read operations also surface error details correctly + // Read requires both user and object when object is a type prefix + var readRequest = new dev.openfga.sdk.api.client.model.ClientReadRequest() + ._object("invalid_type:"); // Type prefix without user will fail + + ExecutionException executionException = assertThrows(ExecutionException.class, () -> { + fga.read(readRequest).get(); + }); + + FgaApiValidationError exception = assertInstanceOf(FgaApiValidationError.class, executionException.getCause()); + + // Verify operation name is "read" + assertTrue(exception.getOperationName().equals("read") + || exception.getMessage().contains("read")); + assertEquals("validation_error", exception.getApiErrorCode()); + assertNotNull(exception.getApiErrorMessage()); + assertNotNull(exception.getRequestId()); + } + + @Test + public void testExpandOperationError() throws Exception { + // Test that expand operations surface error details correctly + var expandRequest = new dev.openfga.sdk.api.client.model.ClientExpandRequest() + ._object("invalid_type:readme") + .relation("viewer"); + + ExecutionException executionException = assertThrows(ExecutionException.class, () -> { + fga.expand(expandRequest).get(); + }); + + FgaApiValidationError exception = assertInstanceOf(FgaApiValidationError.class, executionException.getCause()); + + // Verify operation name is "expand" + assertTrue(exception.getOperationName().equals("expand") + || exception.getMessage().contains("expand")); + assertEquals("validation_error", exception.getApiErrorCode()); + assertNotNull(exception.getApiErrorMessage()); + assertNotNull(exception.getRequestId()); + } + + @Test + public void testDifferentErrorCodesAreSurfaced() throws Exception { + // Test that different validation_error subcases are properly surfaced + // Case 1: Invalid type + ClientWriteRequest request1 = new ClientWriteRequest() + .writes(List.of(new ClientTupleKey() + ._object("invalid_type:readme") + .relation("viewer") + .user("user:anne"))); + + ExecutionException ex1 = + assertThrows(ExecutionException.class, () -> fga.write(request1).get()); + FgaApiValidationError error1 = assertInstanceOf(FgaApiValidationError.class, ex1.getCause()); + assertTrue(error1.getApiErrorMessage().contains("type")); + + // Case 2: Invalid relation + ClientWriteRequest request2 = new ClientWriteRequest() + .writes(List.of(new ClientTupleKey() + ._object("document:readme") + .relation("invalid_relation") + .user("user:anne"))); + + ExecutionException ex2 = + assertThrows(ExecutionException.class, () -> fga.write(request2).get()); + FgaApiValidationError error2 = assertInstanceOf(FgaApiValidationError.class, ex2.getCause()); + assertTrue(error2.getApiErrorMessage().contains("relation")); + + // Both should have the same error code but different messages + assertEquals(error1.getApiErrorCode(), error2.getApiErrorCode()); + assertNotEquals(error1.getApiErrorMessage(), error2.getApiErrorMessage()); + } + + @Test + public void testErrorMessageContainsOperationContext() throws Exception { + // Verify that different operations have their names in the error message + + ClientWriteRequest writeReq = new ClientWriteRequest() + .writes(List.of( + new ClientTupleKey()._object("invalid:x").relation("r").user("user:x"))); + + ExecutionException writeEx = + assertThrows(ExecutionException.class, () -> fga.write(writeReq).get()); + FgaError writeError = assertInstanceOf(FgaError.class, writeEx.getCause()); + assertTrue(writeError.getMessage().contains("[write]"), "Write error should contain [write] in message"); + + // Check operation + var checkReq = new dev.openfga.sdk.api.client.model.ClientCheckRequest() + ._object("invalid:x") + .relation("r") + .user("user:x"); + + ExecutionException checkEx = + assertThrows(ExecutionException.class, () -> fga.check(checkReq).get()); + FgaError checkError = assertInstanceOf(FgaError.class, checkEx.getCause()); + assertTrue(checkError.getMessage().contains("[check]"), "Check error should contain [check] in message"); + + // Verify they have different operation names + assertNotEquals(writeError.getOperationName(), checkError.getOperationName()); + } } From c03e30170280cc865b7ab00e04adf50a09e4f488 Mon Sep 17 00:00:00 2001 From: SoulPancake Date: Mon, 17 Nov 2025 17:22:13 +0530 Subject: [PATCH 03/10] feat: log the exception for json processing --- src/main/java/dev/openfga/sdk/errors/FgaError.java | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/main/java/dev/openfga/sdk/errors/FgaError.java b/src/main/java/dev/openfga/sdk/errors/FgaError.java index 24d19cbc..7989c8fc 100644 --- a/src/main/java/dev/openfga/sdk/errors/FgaError.java +++ b/src/main/java/dev/openfga/sdk/errors/FgaError.java @@ -94,7 +94,8 @@ public static Optional getError( error.setApiErrorCode(resp.code); error.setApiErrorMessage(resp.message); } catch (JsonProcessingException e) { - // Fall back, do nothing + // Fall back, do nothing - log the exception for debugging + System.err.println("Failed to parse API error response JSON: " + e.getMessage()); } } From a9617a6e3add3b464ff1dba7c0e84d2474e8c1d7 Mon Sep 17 00:00:00 2001 From: SoulPancake Date: Wed, 19 Nov 2025 13:59:03 +0530 Subject: [PATCH 04/10] feat: address copilot comments --- .../java/dev/openfga/sdk/errors/FgaError.java | 50 ++++++++++++++++--- .../sdk/errors/FgaErrorIntegrationTest.java | 4 +- 2 files changed, 45 insertions(+), 9 deletions(-) diff --git a/src/main/java/dev/openfga/sdk/errors/FgaError.java b/src/main/java/dev/openfga/sdk/errors/FgaError.java index 7989c8fc..7fdf8e63 100644 --- a/src/main/java/dev/openfga/sdk/errors/FgaError.java +++ b/src/main/java/dev/openfga/sdk/errors/FgaError.java @@ -24,9 +24,30 @@ public class FgaError extends ApiException { private String apiErrorMessage = null; private String operationName = null; + private static final ObjectMapper OBJECT_MAPPER = new ObjectMapper(); + public static class ApiErrorResponse { - public String code; - public String message; + @com.fasterxml.jackson.annotation.JsonProperty("code") + private String code; + + @com.fasterxml.jackson.annotation.JsonProperty("message") + private String message; + + public String getCode() { + return code; + } + + public void setCode(String code) { + this.code = code; + } + + public String getMessage() { + return message; + } + + public void setMessage(String message) { + this.message = message; + } } public FgaError(String message, Throwable cause, int code, HttpHeaders responseHeaders, String responseBody) { @@ -88,11 +109,10 @@ public static Optional getError( // Parse API error response if (body != null && !body.trim().isEmpty()) { - ObjectMapper mapper = new ObjectMapper(); try { - ApiErrorResponse resp = mapper.readValue(body, ApiErrorResponse.class); - error.setApiErrorCode(resp.code); - error.setApiErrorMessage(resp.message); + ApiErrorResponse resp = OBJECT_MAPPER.readValue(body, ApiErrorResponse.class); + error.setApiErrorCode(resp.getCode()); + error.setApiErrorMessage(resp.getMessage()); } catch (JsonProcessingException e) { // Fall back, do nothing - log the exception for debugging System.err.println("Failed to parse API error response JSON: " + e.getMessage()); @@ -189,9 +209,25 @@ public String getOperationName() { return operationName; } + /** + * Returns a formatted error message for FgaError. + *

+ * If both {@code apiErrorMessage} and {@code operationName} are present and non-empty, the message is formatted as: + * 

+     *     [operationName] apiErrorMessage (apiErrorCode)
+     *
+ * where {@code apiErrorCode} is included only if present and non-empty. + * Otherwise, falls back to the default parent message. + *

+ * + * @return the formatted error message string + */ @Override public String getMessage() { - if (apiErrorMessage != null && !apiErrorMessage.isEmpty() && operationName != null) { + if (apiErrorMessage != null + && !apiErrorMessage.isEmpty() + && operationName != null + && !operationName.isEmpty()) { String codePart = (apiErrorCode != null && !apiErrorCode.isEmpty()) ? " (" + apiErrorCode + ")" : ""; return String.format("[%s] %s%s", operationName, apiErrorMessage, codePart); } else { diff --git a/src/test-integration/java/dev/openfga/sdk/errors/FgaErrorIntegrationTest.java b/src/test-integration/java/dev/openfga/sdk/errors/FgaErrorIntegrationTest.java index da35119f..c6a6af01 100644 --- a/src/test-integration/java/dev/openfga/sdk/errors/FgaErrorIntegrationTest.java +++ b/src/test-integration/java/dev/openfga/sdk/errors/FgaErrorIntegrationTest.java @@ -291,7 +291,7 @@ public void testReadOperationError() throws Exception { FgaApiValidationError exception = assertInstanceOf(FgaApiValidationError.class, executionException.getCause()); // Verify operation name is "read" - assertTrue(exception.getOperationName().equals("read") + assertTrue("read".equals(exception.getOperationName()) || exception.getMessage().contains("read")); assertEquals("validation_error", exception.getApiErrorCode()); assertNotNull(exception.getApiErrorMessage()); @@ -312,7 +312,7 @@ public void testExpandOperationError() throws Exception { FgaApiValidationError exception = assertInstanceOf(FgaApiValidationError.class, executionException.getCause()); // Verify operation name is "expand" - assertTrue(exception.getOperationName().equals("expand") + assertTrue("expand".equals(exception.getOperationName()) || exception.getMessage().contains("expand")); assertEquals("validation_error", exception.getApiErrorCode()); assertNotNull(exception.getApiErrorMessage()); From a016a328adee19766a84a048cf496d4848ea8c36 Mon Sep 17 00:00:00 2001 From: SoulPancake Date: Wed, 19 Nov 2025 14:52:47 +0530 Subject: [PATCH 05/10] feat: add changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6171468c..ed9fb31f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ ## [Unreleased](https://github.com/openfga/java-sdk/compare/v0.9.3...HEAD) +- Improved error handling and integration test coverage for FgaError and related classes. (#260) + ## v0.9.3 ### [0.9.3](https://github.com/openfga/java-sdk/compare/v0.9.2...v0.9.3)) (2025-11-10) From 9dec79d1fea1b6eae540e76a85ba73eb3d6acc8a Mon Sep 17 00:00:00 2001 From: Anurag Bandyopadhyay Date: Wed, 19 Nov 2025 20:58:22 +0530 Subject: [PATCH 06/10] feat: thorough tests and sync with rfc --- .../java/dev/openfga/sdk/errors/FgaError.java | 130 +++++++- .../sdk/errors/FgaErrorIntegrationTest.java | 309 ++++++++++++++---- .../sdk/api/auth/OAuth2ClientTest.java | 4 +- 3 files changed, 368 insertions(+), 75 deletions(-) diff --git a/src/main/java/dev/openfga/sdk/errors/FgaError.java b/src/main/java/dev/openfga/sdk/errors/FgaError.java index 7fdf8e63..f1e1299c 100644 --- a/src/main/java/dev/openfga/sdk/errors/FgaError.java +++ b/src/main/java/dev/openfga/sdk/errors/FgaError.java @@ -26,6 +26,7 @@ public class FgaError extends ApiException { private static final ObjectMapper OBJECT_MAPPER = new ObjectMapper(); + @com.fasterxml.jackson.annotation.JsonIgnoreProperties(ignoreUnknown = true) public static class ApiErrorResponse { @com.fasterxml.jackson.annotation.JsonProperty("code") private String code; @@ -33,6 +34,9 @@ public static class ApiErrorResponse { @com.fasterxml.jackson.annotation.JsonProperty("message") private String message; + @com.fasterxml.jackson.annotation.JsonProperty("error") + private String error; + public String getCode() { return code; } @@ -42,12 +46,20 @@ public void setCode(String code) { } public String getMessage() { - return message; + return message != null ? message : error; } public void setMessage(String message) { this.message = message; } + + public String getError() { + return error; + } + + public void setError(String error) { + this.error = error; + } } public FgaError(String message, Throwable cause, int code, HttpHeaders responseHeaders, String responseBody) { @@ -83,7 +95,7 @@ public static Optional getError( error = new FgaApiNotFoundError(name, previousError, status, headers, body); } else if (status == TOO_MANY_REQUESTS) { error = new FgaApiRateLimitExceededError(name, previousError, status, headers, body); - } else if (isServerError(status)) { + } else if (HttpStatusCode.isServerError(status)) { error = new FgaApiInternalError(name, previousError, status, headers, body); } else { error = new FgaError(name, previousError, status, headers, body); @@ -212,26 +224,116 @@ public String getOperationName() { /** * Returns a formatted error message for FgaError. *

- * If both {@code apiErrorMessage} and {@code operationName} are present and non-empty, the message is formatted as: + * The message is formatted as: * 

-     *     [operationName] apiErrorMessage (apiErrorCode)
+     *     [operationName] HTTP statusCode apiErrorMessage (apiErrorCode) [request-id: requestId]
      *
- * where {@code apiErrorCode} is included only if present and non-empty. - * Otherwise, falls back to the default parent message. + * Example: [write] HTTP 400 type 'invalid_type' not found (validation_error) [request-id: abc-123] *

* * @return the formatted error message string */ @Override public String getMessage() { - if (apiErrorMessage != null - && !apiErrorMessage.isEmpty() - && operationName != null - && !operationName.isEmpty()) { - String codePart = (apiErrorCode != null && !apiErrorCode.isEmpty()) ? " (" + apiErrorCode + ")" : ""; - return String.format("[%s] %s%s", operationName, apiErrorMessage, codePart); - } else { - return super.getMessage(); + // Use apiErrorMessage if available, otherwise fall back to the original message + String message = (apiErrorMessage != null && !apiErrorMessage.isEmpty()) ? apiErrorMessage : super.getMessage(); + + StringBuilder sb = new StringBuilder(); + + // [operationName] + if (operationName != null && !operationName.isEmpty()) { + sb.append("[").append(operationName).append("] "); + } + + // HTTP 400 + sb.append("HTTP ").append(getStatusCode()).append(" "); + + // type 'invalid_type' not found + if (message != null && !message.isEmpty()) { + sb.append(message); + } + + // (validation_error) + if (apiErrorCode != null && !apiErrorCode.isEmpty()) { + sb.append(" (").append(apiErrorCode).append(")"); } + + // [request-id: abc-123] + if (requestId != null && !requestId.isEmpty()) { + sb.append(" [request-id: ").append(requestId).append("]"); + } + + return sb.toString().trim(); + } + + // --- Helper Methods --- + + /** + * Checks if this is a validation error. + * Reliable error type checking based on error code. + * + * @return true if this is a validation error + */ + public boolean isValidationError() { + return "validation_error".equals(apiErrorCode); + } + + /** + * Checks if this is a not found (404) error. + * + * @return true if this is a 404 error + */ + public boolean isNotFoundError() { + return getStatusCode() == NOT_FOUND; + } + + /** + * Checks if this is an authentication (401) error. + * + * @return true if this is a 401 error + */ + public boolean isAuthenticationError() { + return getStatusCode() == UNAUTHORIZED; + } + + /** + * Checks if this is a rate limit (429) error. + * + * @return true if this is a rate limit error + */ + public boolean isRateLimitError() { + return getStatusCode() == TOO_MANY_REQUESTS || "rate_limit_exceeded".equals(apiErrorCode); + } + + /** + * Checks if this error should be retried. + * 429 (Rate Limit) and 5xx (Server Errors) are typically retryable. + * + * @return true if this error is retryable + */ + public boolean isRetryable() { + int status = getStatusCode(); + // 429 (Rate Limit) and 5xx (Server Errors) are typically retryable. + return status == TOO_MANY_REQUESTS || (status >= 500 && status < 600); + } + + /** + * Checks if this is a client error (4xx). + * + * @return true if this is a 4xx error + */ + public boolean isClientError() { + int status = getStatusCode(); + return status >= 400 && status < 500; + } + + /** + * Checks if this is a server error (5xx). + * + * @return true if this is a 5xx error + */ + public boolean isServerError() { + int status = getStatusCode(); + return status >= 500 && status < 600; } } diff --git a/src/test-integration/java/dev/openfga/sdk/errors/FgaErrorIntegrationTest.java b/src/test-integration/java/dev/openfga/sdk/errors/FgaErrorIntegrationTest.java index c6a6af01..dc33bd11 100644 --- a/src/test-integration/java/dev/openfga/sdk/errors/FgaErrorIntegrationTest.java +++ b/src/test-integration/java/dev/openfga/sdk/errors/FgaErrorIntegrationTest.java @@ -103,7 +103,6 @@ public void testWriteValidationErrorWithInvalidRelation() throws Exception { @Test public void testErrorMessageFormattingWithAllFields() throws Exception { - // Verify that when all fields are present, the message is properly formatted ClientWriteRequest request = new ClientWriteRequest() .writes(List.of(new ClientTupleKey() ._object("invalid_type:readme") @@ -116,22 +115,23 @@ public void testErrorMessageFormattingWithAllFields() throws Exception { FgaApiValidationError exception = assertInstanceOf(FgaApiValidationError.class, executionException.getCause()); - // Verify the message follows the format: [operationName] apiErrorMessage (apiErrorCode) String message = exception.getMessage(); - assertTrue(message.startsWith("[write]"), "Message should start with [write]"); - assertTrue(message.contains("type 'invalid_type' not found"), "Message should contain the API error message"); - assertTrue(message.endsWith("(validation_error)"), "Message should end with (validation_error)"); + assertTrue(message.startsWith("[write]")); + assertTrue(message.contains("HTTP 400")); + assertTrue(message.contains("type 'invalid_type' not found")); + assertTrue(message.contains("(validation_error)")); + assertTrue(message.contains("[request-id: ")); + assertTrue(message.endsWith("]")); - // Verify individual fields are accessible assertEquals("write", exception.getOperationName()); assertEquals("validation_error", exception.getApiErrorCode()); + assertEquals(400, exception.getStatusCode()); assertNotNull(exception.getApiErrorMessage()); assertNotNull(exception.getRequestId()); } @Test public void testCheckValidationError() throws Exception { - // Test error handling for check operation to verify operation name is set correctly var checkRequest = new dev.openfga.sdk.api.client.model.ClientCheckRequest() ._object("invalid_type:readme") .relation("viewer") @@ -143,18 +143,17 @@ public void testCheckValidationError() throws Exception { FgaApiValidationError exception = assertInstanceOf(FgaApiValidationError.class, executionException.getCause()); - // Verify operation name is "check" not "write" assertEquals("check", exception.getOperationName()); - assertTrue(exception.getMessage().contains("[check]")); + assertEquals(400, exception.getStatusCode()); assertEquals("validation_error", exception.getApiErrorCode()); + assertTrue(exception.getMessage().contains("[check]")); + assertTrue(exception.getMessage().contains("HTTP 400")); assertNotNull(exception.getApiErrorMessage()); assertNotNull(exception.getRequestId()); } @Test public void testErrorDetailsAreNotLostInStackTrace() throws Exception { - // Verify that error details are preserved when exception is thrown - // This addresses the issue where details were "buried down in the exception stack" ClientWriteRequest request = new ClientWriteRequest() .writes(List.of(new ClientTupleKey() ._object("invalid_type:readme") @@ -167,24 +166,21 @@ public void testErrorDetailsAreNotLostInStackTrace() throws Exception { } catch (ExecutionException e) { FgaApiValidationError exception = assertInstanceOf(FgaApiValidationError.class, e.getCause()); - // Verify that calling toString() or getMessage() gives useful information String errorString = exception.toString(); - assertTrue( - errorString.contains("type 'invalid_type' not found"), - "toString() should contain the API error message"); + assertTrue(errorString.contains("type 'invalid_type' not found")); String errorMessage = exception.getMessage(); - assertTrue(errorMessage.contains("[write]"), "getMessage() should contain operation name"); - assertTrue(errorMessage.contains("validation_error"), "getMessage() should contain error code"); + assertTrue(errorMessage.contains("[write]")); + assertTrue(errorMessage.contains("HTTP 400")); + assertTrue(errorMessage.contains("validation_error")); - // Verify the response body is still accessible for custom parsing if needed - assertNotNull(exception.getResponseData(), "Response body should be available"); + assertEquals(400, exception.getStatusCode()); + assertNotNull(exception.getResponseData()); } } @Test public void testMultipleTupleErrorsShowDetailedMessage() throws Exception { - // Test that validation errors for multiple tuples still show useful information ClientWriteRequest request = new ClientWriteRequest() .writes(List.of( new ClientTupleKey() @@ -202,18 +198,18 @@ public void testMultipleTupleErrorsShowDetailedMessage() throws Exception { FgaApiValidationError exception = assertInstanceOf(FgaApiValidationError.class, executionException.getCause()); - // The error message should be informative even for batch operations - assertTrue(exception.getMessage().contains("write")); + assertEquals(400, exception.getStatusCode()); assertEquals("validation_error", exception.getApiErrorCode()); + assertTrue(exception.getMessage().contains("[write]")); + assertTrue(exception.getMessage().contains("HTTP 400")); assertNotNull(exception.getApiErrorMessage()); assertNotNull(exception.getRequestId()); } @Test - public void testNotFoundError() throws Exception { - ClientConfiguration badConfig = new ClientConfiguration() - .apiUrl(openfga.getHttpEndpoint()) - .storeId("01HVJPQR3TXYZ9NQXABCDEFGHI"); // Non-existent store ID + public void testGetStoreWithInvalidStoreIdFormat() throws Exception { + ClientConfiguration badConfig = + new ClientConfiguration().apiUrl(openfga.getHttpEndpoint()).storeId("01HVJPQR3TXYZ9NQXABCDEFGHI"); OpenFgaClient badClient = new OpenFgaClient(badConfig); ExecutionException executionException = assertThrows(ExecutionException.class, () -> { @@ -222,22 +218,20 @@ public void testNotFoundError() throws Exception { FgaError exception = assertInstanceOf(FgaError.class, executionException.getCause()); - assertNotNull(exception.getMessage()); - assertTrue(exception.getMessage().contains("getStore") - || exception.getMessage().contains("[get")); - assertNotNull(exception.getApiErrorCode()); - assertNotNull(exception.getApiErrorMessage()); + assertEquals(400, exception.getStatusCode()); + assertEquals("validation_error", exception.getApiErrorCode()); + assertTrue(exception.getApiErrorMessage().contains("does not match regex pattern")); assertNotNull(exception.getRequestId()); + assertTrue(exception.getMessage().contains("HTTP 400")); + assertTrue(exception.getMessage().contains("[getStore]")); } @Test public void testInvalidParameterException() throws Exception { - ClientWriteRequest request = new ClientWriteRequest(); // Empty request, no writes or deletes + ClientWriteRequest request = new ClientWriteRequest(); - // This should throw FgaInvalidParameterException before making the API call dev.openfga.sdk.errors.FgaInvalidParameterException exception = assertThrows(dev.openfga.sdk.errors.FgaInvalidParameterException.class, () -> { - // Trying to write with null storeId by creating a new client without storeId ClientConfiguration config = new ClientConfiguration().apiUrl(openfga.getHttpEndpoint()); OpenFgaClient client = new OpenFgaClient(config); client.write(request).get(); @@ -249,9 +243,7 @@ public void testInvalidParameterException() throws Exception { } @Test - public void testAuthenticationError() throws Exception { - // Test FgaApiAuthenticationError when store doesn't exist (403/401 from API) - // Use a non-existent store ID to trigger authentication/authorization error + public void testInvalidStoreIdFormat() throws Exception { ClientConfiguration badConfig = new ClientConfiguration().apiUrl(openfga.getHttpEndpoint()).storeId("non-existent-store-id-12345"); OpenFgaClient badClient = new OpenFgaClient(badConfig); @@ -266,23 +258,19 @@ public void testAuthenticationError() throws Exception { badClient.write(request).get(); }); - // Could be NotFoundError (404) or AuthenticationError depending on API version FgaError exception = assertInstanceOf(FgaError.class, executionException.getCause()); - // Verify error details are populated - assertNotNull(exception.getMessage()); - assertTrue(exception.getMessage().contains("[write]") - || exception.getMessage().contains("write")); - assertNotNull(exception.getApiErrorCode()); + assertEquals(400, exception.getStatusCode()); + assertEquals("validation_error", exception.getApiErrorCode()); + assertTrue(exception.getMessage().contains("[write]")); + assertTrue(exception.getMessage().contains("HTTP 400")); + assertTrue(exception.getMessage().contains("StoreId")); assertNotNull(exception.getRequestId()); } @Test public void testReadOperationError() throws Exception { - // Test that read operations also surface error details correctly - // Read requires both user and object when object is a type prefix - var readRequest = new dev.openfga.sdk.api.client.model.ClientReadRequest() - ._object("invalid_type:"); // Type prefix without user will fail + var readRequest = new dev.openfga.sdk.api.client.model.ClientReadRequest()._object("invalid_type:"); ExecutionException executionException = assertThrows(ExecutionException.class, () -> { fga.read(readRequest).get(); @@ -290,17 +278,17 @@ public void testReadOperationError() throws Exception { FgaApiValidationError exception = assertInstanceOf(FgaApiValidationError.class, executionException.getCause()); - // Verify operation name is "read" - assertTrue("read".equals(exception.getOperationName()) - || exception.getMessage().contains("read")); + assertEquals("read", exception.getOperationName()); + assertEquals(400, exception.getStatusCode()); assertEquals("validation_error", exception.getApiErrorCode()); + assertTrue(exception.getMessage().contains("[read]")); + assertTrue(exception.getMessage().contains("HTTP 400")); assertNotNull(exception.getApiErrorMessage()); assertNotNull(exception.getRequestId()); } @Test public void testExpandOperationError() throws Exception { - // Test that expand operations surface error details correctly var expandRequest = new dev.openfga.sdk.api.client.model.ClientExpandRequest() ._object("invalid_type:readme") .relation("viewer"); @@ -311,10 +299,11 @@ public void testExpandOperationError() throws Exception { FgaApiValidationError exception = assertInstanceOf(FgaApiValidationError.class, executionException.getCause()); - // Verify operation name is "expand" - assertTrue("expand".equals(exception.getOperationName()) - || exception.getMessage().contains("expand")); + assertEquals("expand", exception.getOperationName()); + assertEquals(400, exception.getStatusCode()); assertEquals("validation_error", exception.getApiErrorCode()); + assertTrue(exception.getMessage().contains("[expand]")); + assertTrue(exception.getMessage().contains("HTTP 400")); assertNotNull(exception.getApiErrorMessage()); assertNotNull(exception.getRequestId()); } @@ -353,8 +342,6 @@ public void testDifferentErrorCodesAreSurfaced() throws Exception { @Test public void testErrorMessageContainsOperationContext() throws Exception { - // Verify that different operations have their names in the error message - ClientWriteRequest writeReq = new ClientWriteRequest() .writes(List.of( new ClientTupleKey()._object("invalid:x").relation("r").user("user:x"))); @@ -362,9 +349,11 @@ public void testErrorMessageContainsOperationContext() throws Exception { ExecutionException writeEx = assertThrows(ExecutionException.class, () -> fga.write(writeReq).get()); FgaError writeError = assertInstanceOf(FgaError.class, writeEx.getCause()); - assertTrue(writeError.getMessage().contains("[write]"), "Write error should contain [write] in message"); + assertEquals("write", writeError.getOperationName()); + assertEquals(400, writeError.getStatusCode()); + assertTrue(writeError.getMessage().contains("[write]")); + assertTrue(writeError.getMessage().contains("HTTP 400")); - // Check operation var checkReq = new dev.openfga.sdk.api.client.model.ClientCheckRequest() ._object("invalid:x") .relation("r") @@ -373,9 +362,209 @@ public void testErrorMessageContainsOperationContext() throws Exception { ExecutionException checkEx = assertThrows(ExecutionException.class, () -> fga.check(checkReq).get()); FgaError checkError = assertInstanceOf(FgaError.class, checkEx.getCause()); - assertTrue(checkError.getMessage().contains("[check]"), "Check error should contain [check] in message"); + assertEquals("check", checkError.getOperationName()); + assertEquals(400, checkError.getStatusCode()); + assertTrue(checkError.getMessage().contains("[check]")); + assertTrue(checkError.getMessage().contains("HTTP 400")); - // Verify they have different operation names assertNotEquals(writeError.getOperationName(), checkError.getOperationName()); } + + // --- Tests for New Helper Methods --- + + @Test + public void testIsValidationErrorHelper() throws Exception { + ClientWriteRequest request = new ClientWriteRequest() + .writes(List.of(new ClientTupleKey() + ._object("invalid_type:readme") + .relation("viewer") + .user("user:anne"))); + + ExecutionException executionException = assertThrows(ExecutionException.class, () -> { + fga.write(request).get(); + }); + + FgaError exception = assertInstanceOf(FgaError.class, executionException.getCause()); + + assertEquals(400, exception.getStatusCode()); + assertEquals("validation_error", exception.getApiErrorCode()); + assertTrue(exception.isValidationError()); + assertTrue(exception.isClientError()); + assertFalse(exception.isServerError()); + assertFalse(exception.isRetryable()); + } + + @Test + public void testIsNotFoundErrorHelper() throws Exception { + var tempStoreResponse = fga.createStore(new CreateStoreRequest().name("TempStoreForNotFoundTest")) + .get(); + String tempStoreId = tempStoreResponse.getId(); + + ClientConfiguration tempConfig = new ClientConfiguration().apiUrl(openfga.getHttpEndpoint()); + OpenFgaClient tempClient = new OpenFgaClient(tempConfig); + tempClient.setStoreId(tempStoreId); + tempClient.deleteStore().get(); + + ExecutionException executionException = assertThrows(ExecutionException.class, () -> { + tempClient.getStore().get(); + }); + + FgaError exception = assertInstanceOf(FgaError.class, executionException.getCause()); + + assertEquals(404, exception.getStatusCode()); + assertEquals("store_id_not_found", exception.getApiErrorCode()); + assertTrue(exception.isNotFoundError()); + assertTrue(exception.isClientError()); + assertFalse(exception.isServerError()); + assertFalse(exception.isRetryable()); + assertFalse(exception.isValidationError()); + assertTrue(exception.getMessage().contains("HTTP 404")); + assertTrue(exception.getMessage().contains("[getStore]")); + } + + @Test + public void testIsClientErrorHelper() throws Exception { + ClientWriteRequest request = new ClientWriteRequest() + .writes(List.of(new ClientTupleKey() + ._object("invalid_type:readme") + .relation("viewer") + .user("user:anne"))); + + ExecutionException executionException = assertThrows(ExecutionException.class, () -> { + fga.write(request).get(); + }); + + FgaError exception = assertInstanceOf(FgaError.class, executionException.getCause()); + + assertEquals(400, exception.getStatusCode()); + assertTrue(exception.isClientError()); + assertFalse(exception.isServerError()); + } + + @Test + public void testErrorCategorizationHelpers() throws Exception { + ClientConfiguration badConfig = + new ClientConfiguration().apiUrl(openfga.getHttpEndpoint()).storeId("non-existent-store-id"); + OpenFgaClient badClient = new OpenFgaClient(badConfig); + + ClientWriteRequest request = new ClientWriteRequest() + .writes(List.of(new ClientTupleKey() + ._object("document:readme") + .relation("viewer") + .user("user:anne"))); + + ExecutionException executionException = assertThrows(ExecutionException.class, () -> { + badClient.write(request).get(); + }); + + FgaError exception = assertInstanceOf(FgaError.class, executionException.getCause()); + + assertEquals(400, exception.getStatusCode()); + assertEquals("validation_error", exception.getApiErrorCode()); + assertTrue(exception.isClientError()); + assertTrue(exception.isValidationError()); + assertFalse(exception.isServerError()); + assertFalse(exception.isRetryable()); + assertFalse(exception.isAuthenticationError()); + assertFalse(exception.isNotFoundError()); + } + + @Test + public void testIsRetryableHelper() throws Exception { + ClientWriteRequest request = new ClientWriteRequest() + .writes(List.of(new ClientTupleKey() + ._object("invalid_type:readme") + .relation("viewer") + .user("user:anne"))); + + ExecutionException executionException = assertThrows(ExecutionException.class, () -> { + fga.write(request).get(); + }); + + FgaError exception = assertInstanceOf(FgaError.class, executionException.getCause()); + + assertEquals(400, exception.getStatusCode()); + assertFalse(exception.isRetryable()); + } + + @Test + public void testHelperMethodsConsistency() throws Exception { + ClientWriteRequest request = new ClientWriteRequest() + .writes(List.of(new ClientTupleKey() + ._object("invalid_type:readme") + .relation("viewer") + .user("user:anne"))); + + ExecutionException executionException = assertThrows(ExecutionException.class, () -> { + fga.write(request).get(); + }); + + FgaError exception = assertInstanceOf(FgaError.class, executionException.getCause()); + + assertEquals(400, exception.getStatusCode()); + assertTrue(exception.isClientError()); + assertFalse(exception.isServerError()); + assertTrue(exception.isValidationError()); + assertFalse(exception.isRetryable()); + } + + @Test + public void testErrorCodeFieldsAccessibility() throws Exception { + ClientWriteRequest request = new ClientWriteRequest() + .writes(List.of(new ClientTupleKey() + ._object("invalid_type:readme") + .relation("viewer") + .user("user:anne"))); + + ExecutionException executionException = assertThrows(ExecutionException.class, () -> { + fga.write(request).get(); + }); + + FgaError exception = assertInstanceOf(FgaError.class, executionException.getCause()); + + assertEquals(400, exception.getStatusCode()); + assertEquals("validation_error", exception.getApiErrorCode()); + assertEquals("write", exception.getOperationName()); + assertNotNull(exception.getApiErrorMessage()); + assertNotNull(exception.getRequestId()); + assertTrue(exception.getRequestId().matches("[a-zA-Z0-9-]+")); + } + + @Test + public void testMessageFormatConsistency() throws Exception { + ClientWriteRequest writeReq = new ClientWriteRequest() + .writes(List.of( + new ClientTupleKey()._object("invalid:x").relation("r").user("user:x"))); + + ExecutionException writeEx = + assertThrows(ExecutionException.class, () -> fga.write(writeReq).get()); + FgaError writeError = assertInstanceOf(FgaError.class, writeEx.getCause()); + + var checkReq = new dev.openfga.sdk.api.client.model.ClientCheckRequest() + ._object("invalid:x") + .relation("r") + .user("user:x"); + + ExecutionException checkEx = + assertThrows(ExecutionException.class, () -> fga.check(checkReq).get()); + FgaError checkError = assertInstanceOf(FgaError.class, checkEx.getCause()); + + String writeMsg = writeError.getMessage(); + String checkMsg = checkError.getMessage(); + + assertTrue(writeMsg.matches("\\[\\w+\\] HTTP \\d{3} .+")); + assertTrue(checkMsg.matches("\\[\\w+\\] HTTP \\d{3} .+")); + + assertEquals(400, writeError.getStatusCode()); + assertEquals(400, checkError.getStatusCode()); + assertTrue(writeMsg.contains("[write]")); + assertTrue(writeMsg.contains("HTTP 400")); + assertTrue(writeMsg.contains("(validation_error)")); + assertTrue(writeMsg.contains("[request-id: ")); + + assertTrue(checkMsg.contains("[check]")); + assertTrue(checkMsg.contains("HTTP 400")); + assertTrue(checkMsg.contains("(validation_error)")); + assertTrue(checkMsg.contains("[request-id: ")); + } } diff --git a/src/test/java/dev/openfga/sdk/api/auth/OAuth2ClientTest.java b/src/test/java/dev/openfga/sdk/api/auth/OAuth2ClientTest.java index 8fcae337..45c112ed 100644 --- a/src/test/java/dev/openfga/sdk/api/auth/OAuth2ClientTest.java +++ b/src/test/java/dev/openfga/sdk/api/auth/OAuth2ClientTest.java @@ -160,7 +160,9 @@ public void exchangeOAuth2TokenWithRetriesFailure(WireMockRuntimeInfo wm) throws var exception = assertThrows(java.util.concurrent.ExecutionException.class, () -> auth0.getAccessToken() .get()); - assertEquals("dev.openfga.sdk.errors.FgaApiRateLimitExceededError: exchangeToken", exception.getMessage()); + assertTrue(exception.getMessage().contains("FgaApiRateLimitExceededError")); + assertTrue(exception.getMessage().contains("exchangeToken")); + assertTrue(exception.getMessage().contains("HTTP 429")); verify(3, postRequestedFor(urlEqualTo("/oauth/token"))); } From 13eadc9f4ce60c05dacb87eaec7c3757be66d7e8 Mon Sep 17 00:00:00 2001 From: SoulPancake Date: Wed, 26 Nov 2025 12:08:38 +0530 Subject: [PATCH 07/10] feat: address comments and use consts --- .../java/dev/openfga/sdk/errors/FgaError.java | 30 +++- .../sdk/errors/FgaErrorIntegrationTest.java | 143 ++++++++++-------- 2 files changed, 100 insertions(+), 73 deletions(-) diff --git a/src/main/java/dev/openfga/sdk/errors/FgaError.java b/src/main/java/dev/openfga/sdk/errors/FgaError.java index f1e1299c..1ebdb2f7 100644 --- a/src/main/java/dev/openfga/sdk/errors/FgaError.java +++ b/src/main/java/dev/openfga/sdk/errors/FgaError.java @@ -2,8 +2,12 @@ import static dev.openfga.sdk.errors.HttpStatusCode.*; +import com.fasterxml.jackson.annotation.JsonInclude; import com.fasterxml.jackson.core.JsonProcessingException; +import com.fasterxml.jackson.databind.DeserializationFeature; import com.fasterxml.jackson.databind.ObjectMapper; +import com.fasterxml.jackson.databind.SerializationFeature; +import com.fasterxml.jackson.datatype.jsr310.JavaTimeModule; import dev.openfga.sdk.api.configuration.Configuration; import dev.openfga.sdk.api.configuration.CredentialsMethod; import dev.openfga.sdk.constants.FgaConstants; @@ -11,6 +15,7 @@ import java.net.http.HttpRequest; import java.net.http.HttpResponse; import java.util.Optional; +import org.openapitools.jackson.nullable.JsonNullableModule; public class FgaError extends ApiException { private String method = null; @@ -24,10 +29,24 @@ public class FgaError extends ApiException { private String apiErrorMessage = null; private String operationName = null; - private static final ObjectMapper OBJECT_MAPPER = new ObjectMapper(); + private static final ObjectMapper OBJECT_MAPPER = createConfiguredObjectMapper(); + + private static ObjectMapper createConfiguredObjectMapper() { + ObjectMapper mapper = new ObjectMapper(); + mapper.setSerializationInclusion(JsonInclude.Include.NON_NULL); + mapper.configure(DeserializationFeature.FAIL_ON_UNKNOWN_PROPERTIES, false); + mapper.configure(DeserializationFeature.FAIL_ON_INVALID_SUBTYPE, false); + mapper.disable(SerializationFeature.WRITE_DATES_AS_TIMESTAMPS); + mapper.enable(SerializationFeature.WRITE_ENUMS_USING_TO_STRING); + mapper.enable(DeserializationFeature.READ_ENUMS_USING_TO_STRING); + mapper.disable(DeserializationFeature.ADJUST_DATES_TO_CONTEXT_TIME_ZONE); + mapper.registerModule(new JavaTimeModule()); + mapper.registerModule(new JsonNullableModule()); + return mapper; + } @com.fasterxml.jackson.annotation.JsonIgnoreProperties(ignoreUnknown = true) - public static class ApiErrorResponse { + private static class ApiErrorResponse { @com.fasterxml.jackson.annotation.JsonProperty("code") private String code; @@ -312,9 +331,7 @@ public boolean isRateLimitError() { * @return true if this error is retryable */ public boolean isRetryable() { - int status = getStatusCode(); - // 429 (Rate Limit) and 5xx (Server Errors) are typically retryable. - return status == TOO_MANY_REQUESTS || (status >= 500 && status < 600); + return HttpStatusCode.isRetryable(getStatusCode()); } /** @@ -333,7 +350,6 @@ public boolean isClientError() { * @return true if this is a 5xx error */ public boolean isServerError() { - int status = getStatusCode(); - return status >= 500 && status < 600; + return HttpStatusCode.isServerError(getStatusCode()); } } diff --git a/src/test-integration/java/dev/openfga/sdk/errors/FgaErrorIntegrationTest.java b/src/test-integration/java/dev/openfga/sdk/errors/FgaErrorIntegrationTest.java index dc33bd11..21b7aec3 100644 --- a/src/test-integration/java/dev/openfga/sdk/errors/FgaErrorIntegrationTest.java +++ b/src/test-integration/java/dev/openfga/sdk/errors/FgaErrorIntegrationTest.java @@ -31,6 +31,17 @@ public class FgaErrorIntegrationTest { private static final OpenFGAContainer openfga = new OpenFGAContainer("openfga/openfga:v1.10.2"); private static final ObjectMapper mapper = new ObjectMapper().findAndRegisterModules(); + + // Test constants + private static final String ERROR_CODE_VALIDATION_ERROR = "validation_error"; + private static final String ERROR_CODE_STORE_ID_NOT_FOUND = "store_id_not_found"; + private static final String OPERATION_WRITE = "write"; + private static final String OPERATION_CHECK = "check"; + private static final String OPERATION_READ = "read"; + private static final String OPERATION_EXPAND = "expand"; + private static final int HTTP_STATUS_BAD_REQUEST = 400; + private static final int HTTP_STATUS_NOT_FOUND = 404; + private String authModelJson; private OpenFgaClient fga; @@ -58,7 +69,7 @@ public void initializeApi() throws Exception { } @Test - public void testWriteValidationError() throws Exception { + public void writeValidationError() throws Exception { // Try to write a tuple with invalid type ClientWriteRequest request = new ClientWriteRequest() .writes(List.of(new ClientTupleKey() @@ -72,14 +83,14 @@ public void testWriteValidationError() throws Exception { FgaApiValidationError exception = assertInstanceOf(FgaApiValidationError.class, executionException.getCause()); assertTrue(exception.getMessage().contains("type 'invalid_type' not found")); - assertEquals("validation_error", exception.getApiErrorCode()); + assertEquals(ERROR_CODE_VALIDATION_ERROR, exception.getApiErrorCode()); assertTrue(exception.getApiErrorMessage().contains("type 'invalid_type' not found")); - assertEquals("write", exception.getOperationName()); + assertEquals(OPERATION_WRITE, exception.getOperationName()); assertNotNull(exception.getRequestId()); } @Test - public void testWriteValidationErrorWithInvalidRelation() throws Exception { + public void writeValidationErrorWithInvalidRelation() throws Exception { // Try to write a tuple with valid type but invalid relation ClientWriteRequest request = new ClientWriteRequest() .writes(List.of(new ClientTupleKey() @@ -95,14 +106,14 @@ public void testWriteValidationErrorWithInvalidRelation() throws Exception { // Verify the formatted message includes operation name and API error details assertTrue(exception.getMessage().contains("write")); assertTrue(exception.getMessage().contains("relation 'document#invalid_relation' not found")); - assertEquals("validation_error", exception.getApiErrorCode()); + assertEquals(ERROR_CODE_VALIDATION_ERROR, exception.getApiErrorCode()); assertNotNull(exception.getApiErrorMessage()); - assertEquals("write", exception.getOperationName()); + assertEquals(OPERATION_WRITE, exception.getOperationName()); assertNotNull(exception.getRequestId()); } @Test - public void testErrorMessageFormattingWithAllFields() throws Exception { + public void errorMessageFormattingWithAllFields() throws Exception { ClientWriteRequest request = new ClientWriteRequest() .writes(List.of(new ClientTupleKey() ._object("invalid_type:readme") @@ -119,19 +130,19 @@ public void testErrorMessageFormattingWithAllFields() throws Exception { assertTrue(message.startsWith("[write]")); assertTrue(message.contains("HTTP 400")); assertTrue(message.contains("type 'invalid_type' not found")); - assertTrue(message.contains("(validation_error)")); + assertTrue(message.contains("(" + ERROR_CODE_VALIDATION_ERROR + ")")); assertTrue(message.contains("[request-id: ")); assertTrue(message.endsWith("]")); - assertEquals("write", exception.getOperationName()); - assertEquals("validation_error", exception.getApiErrorCode()); - assertEquals(400, exception.getStatusCode()); + assertEquals(OPERATION_WRITE, exception.getOperationName()); + assertEquals(ERROR_CODE_VALIDATION_ERROR, exception.getApiErrorCode()); + assertEquals(HTTP_STATUS_BAD_REQUEST, exception.getStatusCode()); assertNotNull(exception.getApiErrorMessage()); assertNotNull(exception.getRequestId()); } @Test - public void testCheckValidationError() throws Exception { + public void checkValidationError() throws Exception { var checkRequest = new dev.openfga.sdk.api.client.model.ClientCheckRequest() ._object("invalid_type:readme") .relation("viewer") @@ -143,9 +154,9 @@ public void testCheckValidationError() throws Exception { FgaApiValidationError exception = assertInstanceOf(FgaApiValidationError.class, executionException.getCause()); - assertEquals("check", exception.getOperationName()); - assertEquals(400, exception.getStatusCode()); - assertEquals("validation_error", exception.getApiErrorCode()); + assertEquals(OPERATION_CHECK, exception.getOperationName()); + assertEquals(HTTP_STATUS_BAD_REQUEST, exception.getStatusCode()); + assertEquals(ERROR_CODE_VALIDATION_ERROR, exception.getApiErrorCode()); assertTrue(exception.getMessage().contains("[check]")); assertTrue(exception.getMessage().contains("HTTP 400")); assertNotNull(exception.getApiErrorMessage()); @@ -153,7 +164,7 @@ public void testCheckValidationError() throws Exception { } @Test - public void testErrorDetailsAreNotLostInStackTrace() throws Exception { + public void errorDetailsAreNotLostInStackTrace() throws Exception { ClientWriteRequest request = new ClientWriteRequest() .writes(List.of(new ClientTupleKey() ._object("invalid_type:readme") @@ -172,15 +183,15 @@ public void testErrorDetailsAreNotLostInStackTrace() throws Exception { String errorMessage = exception.getMessage(); assertTrue(errorMessage.contains("[write]")); assertTrue(errorMessage.contains("HTTP 400")); - assertTrue(errorMessage.contains("validation_error")); + assertTrue(errorMessage.contains(ERROR_CODE_VALIDATION_ERROR)); - assertEquals(400, exception.getStatusCode()); + assertEquals(HTTP_STATUS_BAD_REQUEST, exception.getStatusCode()); assertNotNull(exception.getResponseData()); } } @Test - public void testMultipleTupleErrorsShowDetailedMessage() throws Exception { + public void multipleTupleErrorsShowDetailedMessage() throws Exception { ClientWriteRequest request = new ClientWriteRequest() .writes(List.of( new ClientTupleKey() @@ -198,8 +209,8 @@ public void testMultipleTupleErrorsShowDetailedMessage() throws Exception { FgaApiValidationError exception = assertInstanceOf(FgaApiValidationError.class, executionException.getCause()); - assertEquals(400, exception.getStatusCode()); - assertEquals("validation_error", exception.getApiErrorCode()); + assertEquals(HTTP_STATUS_BAD_REQUEST, exception.getStatusCode()); + assertEquals(ERROR_CODE_VALIDATION_ERROR, exception.getApiErrorCode()); assertTrue(exception.getMessage().contains("[write]")); assertTrue(exception.getMessage().contains("HTTP 400")); assertNotNull(exception.getApiErrorMessage()); @@ -207,7 +218,7 @@ public void testMultipleTupleErrorsShowDetailedMessage() throws Exception { } @Test - public void testGetStoreWithInvalidStoreIdFormat() throws Exception { + public void getStoreWithInvalidStoreIdFormat() throws Exception { ClientConfiguration badConfig = new ClientConfiguration().apiUrl(openfga.getHttpEndpoint()).storeId("01HVJPQR3TXYZ9NQXABCDEFGHI"); OpenFgaClient badClient = new OpenFgaClient(badConfig); @@ -218,8 +229,8 @@ public void testGetStoreWithInvalidStoreIdFormat() throws Exception { FgaError exception = assertInstanceOf(FgaError.class, executionException.getCause()); - assertEquals(400, exception.getStatusCode()); - assertEquals("validation_error", exception.getApiErrorCode()); + assertEquals(HTTP_STATUS_BAD_REQUEST, exception.getStatusCode()); + assertEquals(ERROR_CODE_VALIDATION_ERROR, exception.getApiErrorCode()); assertTrue(exception.getApiErrorMessage().contains("does not match regex pattern")); assertNotNull(exception.getRequestId()); assertTrue(exception.getMessage().contains("HTTP 400")); @@ -227,7 +238,7 @@ public void testGetStoreWithInvalidStoreIdFormat() throws Exception { } @Test - public void testInvalidParameterException() throws Exception { + public void invalidParameterException() throws Exception { ClientWriteRequest request = new ClientWriteRequest(); dev.openfga.sdk.errors.FgaInvalidParameterException exception = @@ -243,7 +254,7 @@ public void testInvalidParameterException() throws Exception { } @Test - public void testInvalidStoreIdFormat() throws Exception { + public void invalidStoreIdFormat() throws Exception { ClientConfiguration badConfig = new ClientConfiguration().apiUrl(openfga.getHttpEndpoint()).storeId("non-existent-store-id-12345"); OpenFgaClient badClient = new OpenFgaClient(badConfig); @@ -260,8 +271,8 @@ public void testInvalidStoreIdFormat() throws Exception { FgaError exception = assertInstanceOf(FgaError.class, executionException.getCause()); - assertEquals(400, exception.getStatusCode()); - assertEquals("validation_error", exception.getApiErrorCode()); + assertEquals(HTTP_STATUS_BAD_REQUEST, exception.getStatusCode()); + assertEquals(ERROR_CODE_VALIDATION_ERROR, exception.getApiErrorCode()); assertTrue(exception.getMessage().contains("[write]")); assertTrue(exception.getMessage().contains("HTTP 400")); assertTrue(exception.getMessage().contains("StoreId")); @@ -269,7 +280,7 @@ public void testInvalidStoreIdFormat() throws Exception { } @Test - public void testReadOperationError() throws Exception { + public void readOperationError() throws Exception { var readRequest = new dev.openfga.sdk.api.client.model.ClientReadRequest()._object("invalid_type:"); ExecutionException executionException = assertThrows(ExecutionException.class, () -> { @@ -278,9 +289,9 @@ public void testReadOperationError() throws Exception { FgaApiValidationError exception = assertInstanceOf(FgaApiValidationError.class, executionException.getCause()); - assertEquals("read", exception.getOperationName()); - assertEquals(400, exception.getStatusCode()); - assertEquals("validation_error", exception.getApiErrorCode()); + assertEquals(OPERATION_READ, exception.getOperationName()); + assertEquals(HTTP_STATUS_BAD_REQUEST, exception.getStatusCode()); + assertEquals(ERROR_CODE_VALIDATION_ERROR, exception.getApiErrorCode()); assertTrue(exception.getMessage().contains("[read]")); assertTrue(exception.getMessage().contains("HTTP 400")); assertNotNull(exception.getApiErrorMessage()); @@ -288,7 +299,7 @@ public void testReadOperationError() throws Exception { } @Test - public void testExpandOperationError() throws Exception { + public void expandOperationError() throws Exception { var expandRequest = new dev.openfga.sdk.api.client.model.ClientExpandRequest() ._object("invalid_type:readme") .relation("viewer"); @@ -299,9 +310,9 @@ public void testExpandOperationError() throws Exception { FgaApiValidationError exception = assertInstanceOf(FgaApiValidationError.class, executionException.getCause()); - assertEquals("expand", exception.getOperationName()); - assertEquals(400, exception.getStatusCode()); - assertEquals("validation_error", exception.getApiErrorCode()); + assertEquals(OPERATION_EXPAND, exception.getOperationName()); + assertEquals(HTTP_STATUS_BAD_REQUEST, exception.getStatusCode()); + assertEquals(ERROR_CODE_VALIDATION_ERROR, exception.getApiErrorCode()); assertTrue(exception.getMessage().contains("[expand]")); assertTrue(exception.getMessage().contains("HTTP 400")); assertNotNull(exception.getApiErrorMessage()); @@ -309,7 +320,7 @@ public void testExpandOperationError() throws Exception { } @Test - public void testDifferentErrorCodesAreSurfaced() throws Exception { + public void differentErrorCodesAreSurfaced() throws Exception { // Test that different validation_error subcases are properly surfaced // Case 1: Invalid type ClientWriteRequest request1 = new ClientWriteRequest() @@ -341,7 +352,7 @@ public void testDifferentErrorCodesAreSurfaced() throws Exception { } @Test - public void testErrorMessageContainsOperationContext() throws Exception { + public void errorMessageContainsOperationContext() throws Exception { ClientWriteRequest writeReq = new ClientWriteRequest() .writes(List.of( new ClientTupleKey()._object("invalid:x").relation("r").user("user:x"))); @@ -349,8 +360,8 @@ public void testErrorMessageContainsOperationContext() throws Exception { ExecutionException writeEx = assertThrows(ExecutionException.class, () -> fga.write(writeReq).get()); FgaError writeError = assertInstanceOf(FgaError.class, writeEx.getCause()); - assertEquals("write", writeError.getOperationName()); - assertEquals(400, writeError.getStatusCode()); + assertEquals(OPERATION_WRITE, writeError.getOperationName()); + assertEquals(HTTP_STATUS_BAD_REQUEST, writeError.getStatusCode()); assertTrue(writeError.getMessage().contains("[write]")); assertTrue(writeError.getMessage().contains("HTTP 400")); @@ -362,8 +373,8 @@ public void testErrorMessageContainsOperationContext() throws Exception { ExecutionException checkEx = assertThrows(ExecutionException.class, () -> fga.check(checkReq).get()); FgaError checkError = assertInstanceOf(FgaError.class, checkEx.getCause()); - assertEquals("check", checkError.getOperationName()); - assertEquals(400, checkError.getStatusCode()); + assertEquals(OPERATION_CHECK, checkError.getOperationName()); + assertEquals(HTTP_STATUS_BAD_REQUEST, checkError.getStatusCode()); assertTrue(checkError.getMessage().contains("[check]")); assertTrue(checkError.getMessage().contains("HTTP 400")); @@ -373,7 +384,7 @@ public void testErrorMessageContainsOperationContext() throws Exception { // --- Tests for New Helper Methods --- @Test - public void testIsValidationErrorHelper() throws Exception { + public void isValidationErrorHelper() throws Exception { ClientWriteRequest request = new ClientWriteRequest() .writes(List.of(new ClientTupleKey() ._object("invalid_type:readme") @@ -386,8 +397,8 @@ public void testIsValidationErrorHelper() throws Exception { FgaError exception = assertInstanceOf(FgaError.class, executionException.getCause()); - assertEquals(400, exception.getStatusCode()); - assertEquals("validation_error", exception.getApiErrorCode()); + assertEquals(HTTP_STATUS_BAD_REQUEST, exception.getStatusCode()); + assertEquals(ERROR_CODE_VALIDATION_ERROR, exception.getApiErrorCode()); assertTrue(exception.isValidationError()); assertTrue(exception.isClientError()); assertFalse(exception.isServerError()); @@ -395,7 +406,7 @@ public void testIsValidationErrorHelper() throws Exception { } @Test - public void testIsNotFoundErrorHelper() throws Exception { + public void isNotFoundErrorHelper() throws Exception { var tempStoreResponse = fga.createStore(new CreateStoreRequest().name("TempStoreForNotFoundTest")) .get(); String tempStoreId = tempStoreResponse.getId(); @@ -411,8 +422,8 @@ public void testIsNotFoundErrorHelper() throws Exception { FgaError exception = assertInstanceOf(FgaError.class, executionException.getCause()); - assertEquals(404, exception.getStatusCode()); - assertEquals("store_id_not_found", exception.getApiErrorCode()); + assertEquals(HTTP_STATUS_NOT_FOUND, exception.getStatusCode()); + assertEquals(ERROR_CODE_STORE_ID_NOT_FOUND, exception.getApiErrorCode()); assertTrue(exception.isNotFoundError()); assertTrue(exception.isClientError()); assertFalse(exception.isServerError()); @@ -423,7 +434,7 @@ public void testIsNotFoundErrorHelper() throws Exception { } @Test - public void testIsClientErrorHelper() throws Exception { + public void isClientErrorHelper() throws Exception { ClientWriteRequest request = new ClientWriteRequest() .writes(List.of(new ClientTupleKey() ._object("invalid_type:readme") @@ -436,13 +447,13 @@ public void testIsClientErrorHelper() throws Exception { FgaError exception = assertInstanceOf(FgaError.class, executionException.getCause()); - assertEquals(400, exception.getStatusCode()); + assertEquals(HTTP_STATUS_BAD_REQUEST, exception.getStatusCode()); assertTrue(exception.isClientError()); assertFalse(exception.isServerError()); } @Test - public void testErrorCategorizationHelpers() throws Exception { + public void errorCategorizationHelpers() throws Exception { ClientConfiguration badConfig = new ClientConfiguration().apiUrl(openfga.getHttpEndpoint()).storeId("non-existent-store-id"); OpenFgaClient badClient = new OpenFgaClient(badConfig); @@ -459,8 +470,8 @@ public void testErrorCategorizationHelpers() throws Exception { FgaError exception = assertInstanceOf(FgaError.class, executionException.getCause()); - assertEquals(400, exception.getStatusCode()); - assertEquals("validation_error", exception.getApiErrorCode()); + assertEquals(HTTP_STATUS_BAD_REQUEST, exception.getStatusCode()); + assertEquals(ERROR_CODE_VALIDATION_ERROR, exception.getApiErrorCode()); assertTrue(exception.isClientError()); assertTrue(exception.isValidationError()); assertFalse(exception.isServerError()); @@ -470,7 +481,7 @@ public void testErrorCategorizationHelpers() throws Exception { } @Test - public void testIsRetryableHelper() throws Exception { + public void isRetryableHelper() throws Exception { ClientWriteRequest request = new ClientWriteRequest() .writes(List.of(new ClientTupleKey() ._object("invalid_type:readme") @@ -483,12 +494,12 @@ public void testIsRetryableHelper() throws Exception { FgaError exception = assertInstanceOf(FgaError.class, executionException.getCause()); - assertEquals(400, exception.getStatusCode()); + assertEquals(HTTP_STATUS_BAD_REQUEST, exception.getStatusCode()); assertFalse(exception.isRetryable()); } @Test - public void testHelperMethodsConsistency() throws Exception { + public void helperMethodsConsistency() throws Exception { ClientWriteRequest request = new ClientWriteRequest() .writes(List.of(new ClientTupleKey() ._object("invalid_type:readme") @@ -501,7 +512,7 @@ public void testHelperMethodsConsistency() throws Exception { FgaError exception = assertInstanceOf(FgaError.class, executionException.getCause()); - assertEquals(400, exception.getStatusCode()); + assertEquals(HTTP_STATUS_BAD_REQUEST, exception.getStatusCode()); assertTrue(exception.isClientError()); assertFalse(exception.isServerError()); assertTrue(exception.isValidationError()); @@ -509,7 +520,7 @@ public void testHelperMethodsConsistency() throws Exception { } @Test - public void testErrorCodeFieldsAccessibility() throws Exception { + public void errorCodeFieldsAccessibility() throws Exception { ClientWriteRequest request = new ClientWriteRequest() .writes(List.of(new ClientTupleKey() ._object("invalid_type:readme") @@ -522,16 +533,16 @@ public void testErrorCodeFieldsAccessibility() throws Exception { FgaError exception = assertInstanceOf(FgaError.class, executionException.getCause()); - assertEquals(400, exception.getStatusCode()); - assertEquals("validation_error", exception.getApiErrorCode()); - assertEquals("write", exception.getOperationName()); + assertEquals(HTTP_STATUS_BAD_REQUEST, exception.getStatusCode()); + assertEquals(ERROR_CODE_VALIDATION_ERROR, exception.getApiErrorCode()); + assertEquals(OPERATION_WRITE, exception.getOperationName()); assertNotNull(exception.getApiErrorMessage()); assertNotNull(exception.getRequestId()); assertTrue(exception.getRequestId().matches("[a-zA-Z0-9-]+")); } @Test - public void testMessageFormatConsistency() throws Exception { + public void messageFormatConsistency() throws Exception { ClientWriteRequest writeReq = new ClientWriteRequest() .writes(List.of( new ClientTupleKey()._object("invalid:x").relation("r").user("user:x"))); @@ -555,16 +566,16 @@ public void testMessageFormatConsistency() throws Exception { assertTrue(writeMsg.matches("\\[\\w+\\] HTTP \\d{3} .+")); assertTrue(checkMsg.matches("\\[\\w+\\] HTTP \\d{3} .+")); - assertEquals(400, writeError.getStatusCode()); - assertEquals(400, checkError.getStatusCode()); + assertEquals(HTTP_STATUS_BAD_REQUEST, writeError.getStatusCode()); + assertEquals(HTTP_STATUS_BAD_REQUEST, checkError.getStatusCode()); assertTrue(writeMsg.contains("[write]")); assertTrue(writeMsg.contains("HTTP 400")); - assertTrue(writeMsg.contains("(validation_error)")); + assertTrue(writeMsg.contains("(" + ERROR_CODE_VALIDATION_ERROR + ")")); assertTrue(writeMsg.contains("[request-id: ")); assertTrue(checkMsg.contains("[check]")); assertTrue(checkMsg.contains("HTTP 400")); - assertTrue(checkMsg.contains("(validation_error)")); + assertTrue(checkMsg.contains("(" + ERROR_CODE_VALIDATION_ERROR + ")")); assertTrue(checkMsg.contains("[request-id: ")); } } From 26ec73f962f321b61e03c54a93b9352dec72f7b1 Mon Sep 17 00:00:00 2001 From: Anurag Bandyopadhyay Date: Wed, 26 Nov 2025 14:38:00 +0530 Subject: [PATCH 08/10] feat: apply changelog suggestion from @Copilot Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 45894923..f2a2095a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,7 @@ ## [Unreleased](https://github.com/openfga/java-sdk/compare/v0.9.3...HEAD) +### Changed - Improved error handling and integration test coverage for FgaError and related classes. (#260) ## v0.9.3 From bef34e9a45bc968c16ac5401c64498f2e3474adf Mon Sep 17 00:00:00 2001 From: SoulPancake Date: Fri, 28 Nov 2025 14:56:43 +0530 Subject: [PATCH 09/10] feat: fallback to using resp body as err msg when parsing fails --- src/main/java/dev/openfga/sdk/errors/FgaError.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/dev/openfga/sdk/errors/FgaError.java b/src/main/java/dev/openfga/sdk/errors/FgaError.java index 1ebdb2f7..37bd59f1 100644 --- a/src/main/java/dev/openfga/sdk/errors/FgaError.java +++ b/src/main/java/dev/openfga/sdk/errors/FgaError.java @@ -145,8 +145,8 @@ public static Optional getError( error.setApiErrorCode(resp.getCode()); error.setApiErrorMessage(resp.getMessage()); } catch (JsonProcessingException e) { - // Fall back, do nothing - log the exception for debugging - System.err.println("Failed to parse API error response JSON: " + e.getMessage()); + // Fall back to using raw response body as error message if parsing fails + error.setApiErrorMessage(body); } } From cdb089c212cd1411fb98a37cde21aee1fe2a2902 Mon Sep 17 00:00:00 2001 From: SoulPancake Date: Fri, 28 Nov 2025 15:14:21 +0530 Subject: [PATCH 10/10] fix: use unknown err code and msg raw resp --- .../java/dev/openfga/sdk/errors/FgaError.java | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/src/main/java/dev/openfga/sdk/errors/FgaError.java b/src/main/java/dev/openfga/sdk/errors/FgaError.java index 37bd59f1..ef23ef9f 100644 --- a/src/main/java/dev/openfga/sdk/errors/FgaError.java +++ b/src/main/java/dev/openfga/sdk/errors/FgaError.java @@ -18,6 +18,8 @@ import org.openapitools.jackson.nullable.JsonNullableModule; public class FgaError extends ApiException { + private static final String UNKNOWN_ERROR_CODE = "unknown_error"; + private String method = null; private String requestUrl = null; private String clientId = null; @@ -145,8 +147,9 @@ public static Optional getError( error.setApiErrorCode(resp.getCode()); error.setApiErrorMessage(resp.getMessage()); } catch (JsonProcessingException e) { - // Fall back to using raw response body as error message if parsing fails - error.setApiErrorMessage(body); + // Wrap unparseable response + error.setApiErrorCode(UNKNOWN_ERROR_CODE); + error.setApiErrorMessage("Unable to parse error response. Raw response: " + body); } } @@ -297,6 +300,16 @@ public boolean isValidationError() { return "validation_error".equals(apiErrorCode); } + /** + * Checks if this is an unknown error due to unparseable response. + * This occurs when the error response could not be parsed as JSON. + * + * @return true if this is an unknown error + */ + public boolean isUnknownError() { + return UNKNOWN_ERROR_CODE.equals(apiErrorCode); + } + /** * Checks if this is a not found (404) error. *