Skip to content

Commit 768783c

Browse files
SoulPancakeSoulPancake
committed
feat: address copilot comments
1 parent 56698c1 commit 768783c

2 files changed

Lines changed: 78 additions & 58 deletions

File tree

‎src/main/java/dev/openfga/sdk/errors/FgaApiValidationError.java‎

Lines changed: 31 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -20,12 +20,34 @@ public class FgaApiValidationError extends FgaError {
2020
public FgaApiValidationError(
2121
String message, Throwable cause, int code, HttpHeaders responseHeaders, String responseBody) {
2222
super(message, cause, code, responseHeaders, responseBody);
23-
parseValidationDetails(responseBody);
23+
parseValidationDetails(responseBody, null);
2424
}
2525

2626
public FgaApiValidationError(String message, int code, HttpHeaders responseHeaders, String responseBody) {
2727
super(message, code, responseHeaders, responseBody);
28-
parseValidationDetails(responseBody);
28+
parseValidationDetails(responseBody, null);
29+
}
30+
31+
/**
32+
* Constructor that accepts a pre-parsed JsonNode to avoid re-parsing the response body.
33+
* This is more efficient when the JSON has already been parsed by the parent class.
34+
*
35+
* @param message The error message
36+
* @param cause The underlying cause (if any)
37+
* @param code The HTTP status code
38+
* @param responseHeaders The response headers
39+
* @param responseBody The raw response body
40+
* @param parsedJson The already-parsed JSON root node (may be null)
41+
*/
42+
public FgaApiValidationError(
43+
String message,
44+
Throwable cause,
45+
int code,
46+
HttpHeaders responseHeaders,
47+
String responseBody,
48+
JsonNode parsedJson) {
49+
super(message, cause, code, responseHeaders, responseBody);
50+
parseValidationDetails(responseBody, parsedJson);
2951
}
3052

3153
/**
@@ -36,14 +58,16 @@ public FgaApiValidationError(String message, int code, HttpHeaders responseHeade
3658
* The application should not rely on these fields for critical logic.
3759
*
3860
* @param responseBody The API error response body
61+
* @param parsedJson The already-parsed JSON root node (may be null, in which case we parse it)
3962
*/
40-
private void parseValidationDetails(String responseBody) {
63+
private void parseValidationDetails(String responseBody, JsonNode parsedJson) {
4164
if (responseBody == null || responseBody.trim().isEmpty()) {
4265
return;
4366
}
4467

4568
try {
46-
JsonNode root = getErrorMapper().readTree(responseBody);
69+
// Use the pre-parsed JSON node if available, otherwise parse it
70+
JsonNode root = parsedJson != null ? parsedJson : getErrorMapper().readTree(responseBody);
4771
String message = root.has("message") ? root.get("message").asText() : null;
4872

4973
if (message != null) {
@@ -73,7 +97,8 @@ else if (message.contains(TYPE_PREFIX) && message.contains(NOT_FOUND_SUFFIX)) {
7397
else if (message.contains(CHECK_REQUEST_TUPLE_KEY_PREFIX)) {
7498
int start =
7599
message.indexOf(CHECK_REQUEST_TUPLE_KEY_PREFIX) + CHECK_REQUEST_TUPLE_KEY_PREFIX.length();
76-
int end = message.indexOf(":", start);
100+
// Search for ": " (colon followed by space) for more robust matching
101+
int end = message.indexOf(": ", start);
77102
if (end > start) {
78103
this.invalidField = message.substring(start, end);
79104
addMetadata("invalid_field", invalidField);
@@ -82,7 +107,7 @@ else if (message.contains(CHECK_REQUEST_TUPLE_KEY_PREFIX)) {
82107
// Parse patterns like: "invalid TupleKey.User: value does not match regex..."
83108
else if (message.contains(TUPLE_KEY_PREFIX)) {
84109
int start = message.indexOf(TUPLE_KEY_PREFIX) + TUPLE_KEY_PREFIX.length();
85-
int end = message.indexOf(":", start);
110+
int end = message.indexOf(": ", start);
86111
if (end > start) {
87112
this.invalidField = message.substring(start, end);
88113
addMetadata("invalid_field", invalidField);

‎src/main/java/dev/openfga/sdk/errors/FgaError.java‎

Lines changed: 47 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -50,58 +50,53 @@ public FgaError(String message, int code, HttpHeaders responseHeaders, String re
5050
}
5151

5252
/**
53-
* Parse the API error response body to extract the error message and code.
54-
* @param methodName The API method name that was called
55-
* @param responseBody The response body JSON string
56-
* @return A descriptive error message
53+
* Container for parsed error response data.
5754
*/
58-
private static String parseErrorMessage(String methodName, String responseBody) {
59-
if (responseBody == null || responseBody.trim().isEmpty()) {
60-
return methodName;
61-
}
62-
63-
try {
64-
JsonNode jsonNode = ERROR_MAPPER.readTree(responseBody);
65-
66-
// Try to extract message field
67-
JsonNode messageNode = jsonNode.get("message");
68-
String message = (messageNode != null && !messageNode.isNull()) ? messageNode.asText() : null;
69-
70-
// If we have a message, return it, otherwise fall back to method name
71-
if (message != null && !message.trim().isEmpty()) {
72-
return message;
73-
}
74-
} catch (Exception e) {
75-
// If parsing fails, fall back to the method name
76-
// This is intentional to ensure errors are still reported even if the response format is unexpected
55+
private static class ParsedErrorResponse {
56+
final String message;
57+
final String code;
58+
final JsonNode rootNode;
59+
60+
ParsedErrorResponse(String message, String code, JsonNode rootNode) {
61+
this.message = message;
62+
this.code = code;
63+
this.rootNode = rootNode;
7764
}
78-
79-
return methodName;
8065
}
8166

8267
/**
83-
* Extract the API error code from the response body.
68+
* Parse the API error response body once to extract the error message, code, and root JSON node.
69+
* This method parses the JSON only once and extracts all needed fields, improving efficiency.
70+
*
71+
* @param methodName The API method name that was called
8472
* @param responseBody The response body JSON string
85-
* @return The error code, or null if not found
73+
* @return ParsedErrorResponse containing message, code, and root JSON node
8674
*/
87-
private static String extractErrorCode(String responseBody) {
75+
private static ParsedErrorResponse parseErrorResponse(String methodName, String responseBody) {
8876
if (responseBody == null || responseBody.trim().isEmpty()) {
89-
return null;
77+
return new ParsedErrorResponse(methodName, null, null);
9078
}
9179

9280
try {
93-
JsonNode jsonNode = ERROR_MAPPER.readTree(responseBody);
81+
JsonNode rootNode = ERROR_MAPPER.readTree(responseBody);
9482

95-
JsonNode codeNode = jsonNode.get("code");
96-
if (codeNode != null && !codeNode.isNull()) {
97-
return codeNode.asText();
98-
}
83+
// Extract message field
84+
JsonNode messageNode = rootNode.get("message");
85+
String message = (messageNode != null && !messageNode.isNull()) ? messageNode.asText() : null;
86+
87+
// Extract code field
88+
JsonNode codeNode = rootNode.get("code");
89+
String code = (codeNode != null && !codeNode.isNull()) ? codeNode.asText() : null;
90+
91+
// If we have a message, use it, otherwise fall back to method name
92+
String finalMessage = (message != null && !message.trim().isEmpty()) ? message : methodName;
93+
94+
return new ParsedErrorResponse(finalMessage, code, rootNode);
9995
} catch (Exception e) {
100-
// If parsing fails, return null
101-
// This is intentional - we still want to report the error even if we can't extract the code
96+
// If parsing fails, fall back to the method name
97+
// This is intentional to ensure errors are still reported even if the response format is unexpected
98+
return new ParsedErrorResponse(methodName, null, null);
10299
}
103-
104-
return null;
105100
}
106101

107102
public static Optional<FgaError> getError(
@@ -120,22 +115,23 @@ public static Optional<FgaError> getError(
120115
final String body = response.body();
121116
final var headers = response.headers();
122117

123-
// Parse the error message from the response body
124-
final String errorMessage = parseErrorMessage(name, body);
118+
// Parse the error response once to extract message, code, and JSON node
119+
final ParsedErrorResponse parsedResponse = parseErrorResponse(name, body);
125120
final FgaError error;
126121

127122
if (status == BAD_REQUEST || status == UNPROCESSABLE_ENTITY) {
128-
error = new FgaApiValidationError(errorMessage, previousError, status, headers, body);
123+
error = new FgaApiValidationError(
124+
parsedResponse.message, previousError, status, headers, body, parsedResponse.rootNode);
129125
} else if (status == UNAUTHORIZED || status == FORBIDDEN) {
130-
error = new FgaApiAuthenticationError(errorMessage, previousError, status, headers, body);
126+
error = new FgaApiAuthenticationError(parsedResponse.message, previousError, status, headers, body);
131127
} else if (status == NOT_FOUND) {
132-
error = new FgaApiNotFoundError(errorMessage, previousError, status, headers, body);
128+
error = new FgaApiNotFoundError(parsedResponse.message, previousError, status, headers, body);
133129
} else if (status == TOO_MANY_REQUESTS) {
134-
error = new FgaApiRateLimitExceededError(errorMessage, previousError, status, headers, body);
130+
error = new FgaApiRateLimitExceededError(parsedResponse.message, previousError, status, headers, body);
135131
} else if (isServerError(status)) {
136-
error = new FgaApiInternalError(errorMessage, previousError, status, headers, body);
132+
error = new FgaApiInternalError(parsedResponse.message, previousError, status, headers, body);
137133
} else {
138-
error = new FgaError(errorMessage, previousError, status, headers, body);
134+
error = new FgaError(parsedResponse.message, previousError, status, headers, body);
139135
}
140136

141137
error.setMethod(request.method());
@@ -144,18 +140,17 @@ public static Optional<FgaError> getError(
144140
// Set the operation name
145141
error.setOperationName(name);
146142

147-
// Extract and set API error code from response body
148-
String apiErrorCode = extractErrorCode(body);
149-
if (apiErrorCode != null) {
150-
error.setApiErrorCode(apiErrorCode);
143+
// Set API error code if extracted from response
144+
if (parsedResponse.code != null) {
145+
error.setApiErrorCode(parsedResponse.code);
151146
}
152147

153148
// Set the API error message (same as what was parsed for the constructor)
154149
// This allows getMessage() to return a formatted version
155-
if (!errorMessage.equals(name)) {
150+
if (!parsedResponse.message.equals(name)) {
156151
// Only set apiErrorMessage if we actually got a message from the API
157152
// (not just falling back to the operation name)
158-
error.setApiErrorMessage(errorMessage);
153+
error.setApiErrorMessage(parsedResponse.message);
159154
}
160155

161156
// Extract and set request ID from response headers if present

0 commit comments

Comments
 (0)