fix: improve CDC error handling to distinguish user-not-found from connection errors#4991
Open
Varsha0506 wants to merge 1 commit into
Open
fix: improve CDC error handling to distinguish user-not-found from connection errors#4991Varsha0506 wants to merge 1 commit into
Varsha0506 wants to merge 1 commit into
Conversation
…nnection errors - OpenMetadataClient.getUserByFqn: catch FeignException.NotFound specifically for actual 404 (user not found), re-throw other exceptions as OpenMetadataClientException with original error details - FabricCatalogManagementController: add OpenMetadataClientException handler in both publishCatalogRequest and updatePublishedCatalogRequest to return accurate error messages (connection/auth issues vs user not found) - CdcPush.js: display actual backend error message instead of hardcoded 'User didn't log in to CDC' for all 400 responses Co-Authored-By: deloitte.supriya <deloitte.supriya@mercedes-benz.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes misleading "User didn't log in to CDC" error that appeared even when the user was logged into the CDC portal. The root cause was
OpenMetadataClient.getUserByFqn()catching all exceptions asEntityNotFoundException, meaning network timeouts, auth failures, or any OpenMetadata API error was misreported as "user not found."Changes across 3 files:
OpenMetadataClient.java: Split the catch-allExceptionhandler intoFeignException.NotFound(actual 404 →EntityNotFoundException) vs all other exceptions (→OpenMetadataClientExceptionwith original error preserved).FabricCatalogManagementController.java: AddedOpenMetadataClientExceptioncatch block in bothpublishCatalogRequestandupdatePublishedCatalogRequest, returning HTTP 500 with an accurate "unable to connect to CDC" message. UpdatedEntityNotFoundExceptionhandler to include the user ID in the message. Also fixed copy-paste issue inupdatePublishedCatalogRequestwhere error messages and logs incorrectly said "publish" instead of "update."CdcPush.js: Frontend now displays the actual backend error message instead of a hardcoded "User didn't log in to CDC" string for all 400 responses.Review & Testing Checklist for Human
FeignException.NotFoundis the correct exception type for 404 responses from the OpenMetadata Feign client. If the client wraps this differently, the fix will misclassify real "not found" errors as connection errors. Check imports and the Feign client configuration.BaseFabricCatalogManagementService.validateAndProcessOwners()— it also callsgetUserByFqn()and catchesEntityNotFoundException. With this change, non-404 errors will now throwOpenMetadataClientExceptioninstead, which is not caught in that method. Verify this won't cause unhandled exceptions.OpenMetadataClientExceptionhandler includese.getMessage()in the HTTP response body. Confirm this doesn't leak sensitive internal details (hostnames, ports, stack traces) to end users.Notes
getUserByFqnto cover both the 404 and non-404 exception paths.