-
Notifications
You must be signed in to change notification settings - Fork 3k
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
fix(ui): Adding view, forms GraphQL query, remove showing a fallback error message on unhandled GraphQL error #11084
Changes from 3 commits
9de7001
0682baf
398eae6
5e28d74
bc513e3
8a92d84
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -1032,6 +1032,8 @@ private void configureQueryResolvers(final RuntimeWiring.Builder builder) { | |||||||||||||||||||
.dataFetcher("mlModel", getResolver(mlModelType)) | ||||||||||||||||||||
.dataFetcher("mlModelGroup", getResolver(mlModelGroupType)) | ||||||||||||||||||||
.dataFetcher("assertion", getResolver(assertionType)) | ||||||||||||||||||||
.dataFetcher("form", getResolver(formType)) | ||||||||||||||||||||
.dataFetcher("view", getResolver(dataHubViewType)) | ||||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Add error handling for the new data fetcher. The new data fetcher for + .dataFetcher("view", env -> {
+ try {
+ return getResolver(dataHubViewType).get(env);
+ } catch (Exception e) {
+ log.error("Failed to fetch view data", e);
+ throw new RuntimeException("Error fetching view data", e);
+ }
+ }) Committable suggestion
Suggested change
|
||||||||||||||||||||
.dataFetcher("listPolicies", new ListPoliciesResolver(this.entityClient)) | ||||||||||||||||||||
.dataFetcher("getGrantedPrivileges", new GetGrantedPrivilegesResolver()) | ||||||||||||||||||||
.dataFetcher("listUsers", new ListUsersResolver(this.entityClient)) | ||||||||||||||||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -31,13 +31,14 @@ const errorLink = onError((error) => { | |
window.location.replace(`${PageRoutes.AUTHENTICATE}?redirect_uri=${encodeURIComponent(currentPath)}`); | ||
} | ||
} | ||
if (graphQLErrors && graphQLErrors.length) { | ||
const firstError = graphQLErrors[0]; | ||
const { extensions } = firstError; | ||
const errorCode = extensions && (extensions.code as number); | ||
// Fallback in case the calling component does not handle. | ||
message.error(`${firstError.message} (code ${errorCode})`, 3); | ||
} | ||
// Disabled behavior for now -> Components are expected to handle their errors. | ||
// if (graphQLErrors && graphQLErrors.length) { | ||
// const firstError = graphQLErrors[0]; | ||
// const { extensions } = firstError; | ||
// const errorCode = extensions && (extensions.code as number); | ||
// // Fallback in case the calling component does not handle. | ||
// message.error(`${firstError.message} (code ${errorCode})`, 3); // TODO: Decide if we want this back. | ||
// } | ||
Comment on lines
+33
to
+40
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Reminder: Reconsider the necessity of centralized error handling. The TODO comment indicates that the necessity of this error handling should be reconsidered in the future. Do you want me to open a GitHub issue to track this task? |
||
}); | ||
|
||
const client = new ApolloClient({ | ||
|
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.
Add error handling for the new data fetcher.
The new data fetcher for
form
should include error handling to ensure robustness.Committable suggestion