-
Notifications
You must be signed in to change notification settings - Fork 21
feat: improve error messaging #258
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
feat: parse error msg feat: update changelog Removed two feature entries from the unreleased section. feat: set operation name and err msg
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. WalkthroughThe PR enhances error handling in the SDK by introducing JSON parsing utilities to extract detailed error messages and codes from API response bodies. It enriches Changes
Sequence DiagramsequenceDiagram
participant Client
participant SDK as SDK<br/>(getError)
participant Parser as JSON Parser<br/>(ERROR_MAPPER)
participant Error as FgaError
Client->>SDK: HTTP Response + Request metadata
SDK->>SDK: Determine error type<br/>(status code)
SDK->>Parser: Parse response body
alt Parsing succeeds
Parser-->>SDK: errorMessage, errorCode
else Parsing fails
Parser-->>SDK: defaults (empty/null)
end
SDK->>Error: Create error instance<br/>(type, errorMessage)
SDK->>Error: Enrich with metadata<br/>(operationName, apiErrorCode,<br/>requestId, retryAfter)
Error-->>Client: Return populated<br/>FgaError
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #258 +/- ##
============================================
+ Coverage 36.47% 37.46% +0.99%
- Complexity 1144 1196 +52
============================================
Files 188 188
Lines 7192 7322 +130
Branches 824 854 +30
============================================
+ Hits 2623 2743 +120
+ Misses 4462 4456 -6
- Partials 107 123 +16 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR improves error messaging in the SDK by parsing actual API error messages from response bodies instead of only exposing operation names. It adds two new fields (apiErrorMessage and operationName) to the FgaError class while maintaining backward compatibility.
- Extracts error messages and codes from JSON response bodies
- Adds comprehensive test coverage for error parsing scenarios
- Maintains fallback behavior to operation name when parsing fails
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
src/main/java/dev/openfga/sdk/errors/FgaError.java |
Adds JSON parsing methods to extract error messages and codes from API responses, introduces new fields apiErrorMessage and operationName with getters/setters |
src/test/java/dev/openfga/sdk/errors/FgaErrorTest.java |
New comprehensive test suite covering various error scenarios including validation errors, internal errors, fallback cases, and edge cases with empty/non-JSON bodies |
CHANGELOG.md |
Documents the feature addition in the unreleased section |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 8 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
src/main/java/dev/openfga/sdk/errors/FgaApiValidationError.java
Outdated
Show resolved
Hide resolved
src/main/java/dev/openfga/sdk/errors/FgaApiValidationError.java
Outdated
Show resolved
Hide resolved
src/main/java/dev/openfga/sdk/errors/FgaApiValidationError.java
Outdated
Show resolved
Hide resolved
src/main/java/dev/openfga/sdk/errors/FgaApiValidationError.java
Outdated
Show resolved
Hide resolved
| .get()); | ||
|
|
||
| assertEquals("dev.openfga.sdk.errors.FgaApiRateLimitExceededError: exchangeToken", exception.getMessage()); | ||
| // The error message now includes the formatted message from getMessage() override |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be fine IMO
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Description
SDK errors currently only expose the operation name (e.g., "check") rather than the API's actual error message. This makes debugging difficult when errors occur.
Changes
Added fields to FgaError:
apiErrorMessage - Parsed error message from API response body (null if unavailable)
operationName - Operation that failed (e.g., "check", "write")
Enhanced getError() parsing:
Sets apiErrorMessage when successfully parsed from response JSON
Sets operationName for all errors
Maintains existing fallback behavior (operation name) for empty/non-JSON responses
Preserves backward compatibility:
getMessage() behavior unchanged - returns parsed message or operation name as before
All existing tests pass without modification
New fields are nullable and additive only
What problem is being solved?
How is it being solved?
What changes are made to solve it?
References
Review Checklist
mainSummary by CodeRabbit
Release Notes
Bug Fixes
Tests