[NIFI-13099] status history error handling#8703
Conversation
|
Will review... |
rfellows
force-pushed
the
NIFI-13099-statusHistoryErrorHandling
branch
from
8e3233b to
23d4d37
Compare
| })), | ||
|
|
||
| on(systemDiagnosticsApiError, (state, { error }) => ({ | ||
| on(systemDiagnosticsBannerError, (state) => ({ |
There was a problem hiding this comment.
Should we also be setting status to error when reported through the snackbar?
| of( | ||
| ComponentStateActions.componentStateApiError({ | ||
| error: error.error | ||
| ErrorActions.snackBarError({ |
There was a problem hiding this comment.
Should this be a banner when it happens following clearComponentState?
| catchError((errorResponse: HttpErrorResponse) => { | ||
| if (request.source === 'cluster-listing') { | ||
| return of( | ||
| ErrorActions.snackBarError({ |
There was a problem hiding this comment.
Should this be a banner on top of the cluster page since what is failing to load the content for the page (if they are on one of the system diagnostics tabs)?
There was a problem hiding this comment.
Good point. Probably yes. However I don't think we want to make it conditional based on which tab is open.
There was a problem hiding this comment.
After further consideration, this presents a potential issue with the cluster listing and the system diagnostics dialog are both open and both showing the same banner error(s). Sticking with snackbar errors in this case until we have contextual banner errors.
|
|
||
| export interface SystemDiagnosticsRequest { | ||
| nodewise: boolean; | ||
| errorStrategy: 'banner' | 'snackbar'; |
There was a problem hiding this comment.
I don't think the errorStrategy is needed when opening this from navigation. Can we either have a different action payload for reload (where errorStrategy is needed) or have it be optional here.
| request: { | ||
| nodewise: false | ||
| nodewise: false, | ||
| errorStrategy: 'banner' |
There was a problem hiding this comment.
errorStrategy shouldn't be needed for getSystemDiagnosticsAndOpenDialog.
| loadClusterListingSuccess$ = createEffect(() => | ||
| this.actions$.pipe( | ||
| ofType(ClusterListingActions.loadClusterListingSuccess), | ||
| concatLatestFrom(() => this.store.select(selectCurrentUser)), |
There was a problem hiding this comment.
concatLatestFrom is imported from the wrong package. I also noticed that systemDiagnosticsService is injected in the constructor here but is not used anymore.
rfellows
force-pushed
the
NIFI-13099-statusHistoryErrorHandling
branch
from
0c5bd80 to
2fb3aca
Compare
| ErrorActions.snackBarError({ | ||
| error: `Failed to load System Diagnostics. - [${ | ||
| errorResponse.error || errorResponse.status | ||
| }]` | ||
| }) |
There was a problem hiding this comment.
I think this needs to be using systemDiagnosticsSnackbarError otherwise the status stays in loading.
| export const selectStatusHistoryStatus = createSelector( | ||
| selectStatusHistoryState, | ||
| (state: StatusHistoryState) => state.status | ||
| ); |
There was a problem hiding this comment.
This is new in this PR and is not used.
* [NIFI-13099] - Error handling for Status History * Error handling for current user * Error handling for extension types * Error handling for flow configuration * Error handling for component state * Error handling for cluster summary * Error handling for System Diagnostics * review feedback * use SystemDiagnosticsActions.systemDiagnosticsSnackbarError * review feedback * review feedback * use snackbar This closes apache#8703
* [NIFI-13099] - Error handling for Status History * Error handling for current user * Error handling for extension types * Error handling for flow configuration * Error handling for component state * Error handling for cluster summary * Error handling for System Diagnostics * review feedback * use SystemDiagnosticsActions.systemDiagnosticsSnackbarError * review feedback * review feedback * use snackbar This closes apache#8703
2 participants
NIFI-13099
Introduced error handling for: