fix: make FgaApiAuthenticationError inherit from FgaApiError#327
fix: make FgaApiAuthenticationError inherit from FgaApiError#327
Conversation
|
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 Use the checkbox below for a quick retry:
WalkthroughFgaApiAuthenticationError now extends FgaApiError instead of FgaError, enabling proper polymorphism for API error handling. Public properties are removed since they are inherited from the parent class. A test validates the instanceof relationship. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related issues
🚥 Pre-merge checks | ✅ 6✅ Passed checks (6 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches🧪 Generate unit tests (beta)
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 #327 +/- ##
==========================================
+ Coverage 89.67% 90.15% +0.47%
==========================================
Files 25 25
Lines 1492 1483 -9
Branches 279 255 -24
==========================================
- Hits 1338 1337 -1
+ Misses 94 88 -6
+ Partials 60 58 -2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
This PR fixes the inheritance hierarchy for FgaApiAuthenticationError to properly extend FgaApiError instead of FgaError, ensuring authentication failures participate in shared API error handling patterns.
Changes:
- Updated
FgaApiAuthenticationErrorto inherit fromFgaApiErrorinstead ofFgaError - Removed duplicate property declarations that are now inherited from
FgaApiError - Simplified constructor to call
super(err)and set message afterward - Added regression test to verify
FgaApiAuthenticationError instanceof FgaApiError
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| errors.ts | Changed class inheritance and removed duplicate properties (statusCode, statusText, method, requestURL, responseData, responseHeader, requestId), simplified constructor |
| tests/errors-authentication.test.ts | Added new test file with regression test asserting proper instanceof FgaApiError behavior |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
tests/errors-authentication.test.ts (1)
5-30: Consider adding assertions for inherited and auth-specific properties.The
instanceofcheck validates the fix for#326, but since the synthetic error already has all the fields, a few extra assertions would guard against future regressions in property propagation through the new inheritance chain.💡 Suggested additional assertions
const err = new FgaApiAuthenticationError(axiosError); expect(err).toBeInstanceOf(FgaApiError); + expect(err).toBeInstanceOf(Error); + + // Inherited from FgaApiError + expect(err.statusCode).toBe(401); + expect(err.statusText).toBe("Unauthorized"); + expect(err.requestURL).toBe("https://issuer.fga.example/oauth/token"); + + // Auth-specific properties + expect(err.clientId).toBe("client-id"); + expect(err.audience).toBe("api-audience"); + expect(err.grantType).toBe("client_credentials"); + expect(err.apiErrorCode).toBe("auth_error"); + + // Custom message + expect(err.message).toBe("FGA Authentication Error. Unauthorized");
|
Addressed the review suggestion to strengthen the regression assertions. Implemented in commit 4099c22:
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Summary
Makes
FgaApiAuthenticationErrorinherit fromFgaApiErrorso auth failures participate in shared API error handling.Changes
FgaApiAuthenticationError instanceof FgaApiErrorFgaApiErrorclientId,audience,grantType) and auth-specific messagePotential Breaking Change
FgaApiAuthenticationErroris now alsoinstanceof FgaApiError.catchlogic that checksFgaApiErrorbeforeFgaApiAuthenticationErrormay now take a different branch than before.Fixes #326