From d6538e3ec3a545258ff9073ef88501fe2c75a4f0 Mon Sep 17 00:00:00 2001 From: Peter Amstutz Date: Mon, 4 Nov 2019 13:17:44 -0500 Subject: [PATCH] 15736: Improve error handling for multisite search * A failed search of one cluster does not prevent seeing results from other clusters * Session validation are reported to the user --- src/store/auth/auth-action-session.ts | 64 ++++++++++++++++--- .../search-results-middleware-service.ts | 56 +++++++--------- 2 files changed, 78 insertions(+), 42 deletions(-) diff --git a/src/store/auth/auth-action-session.ts b/src/store/auth/auth-action-session.ts index ae698192..9190dc1a 100644 --- a/src/store/auth/auth-action-session.ts +++ b/src/store/auth/auth-action-session.ts @@ -14,6 +14,7 @@ import { normalizeURLPath } from "~/common/url"; import { Session, SessionStatus } from "~/models/session"; import { progressIndicatorActions } from "~/store/progress-indicator/progress-indicator-actions"; import { AuthService, UserDetailsResponse } from "~/services/auth-service/auth-service"; +import { snackbarActions, SnackbarKind } from "~/store/snackbar/snackbar-actions"; import * as jsSHA from "jssha"; const getClusterInfo = async (origin: string): Promise<{ clusterId: string, baseUrl: string } | null> => { @@ -84,11 +85,13 @@ const getUserDetails = async (baseUrl: string, token: string): Promise { const shaObj = new jsSHA("SHA-1", "TEXT"); const [ver, uuid, secret] = token.split("/"); if (ver !== "v2") { - throw new Error("Must be a v2 token"); + throw new Error(invalidV2Token); } let salted = secret; if (uuid.substr(0, 5) !== clusterId) { @@ -140,19 +143,31 @@ export const validateSession = (session: Session, activeSession: Session) => if (!info) { throw new Error(`Could not get config for ${session.remoteHost}`); } + + let fail: Error | null = null; try { const { user, token } = await validateCluster(info, session.token); setupSession(info.baseUrl, user, token); - } catch { + } catch (e) { + fail = new Error(`Getting current user for ${session.remoteHost}: ${e.message}`); try { const { user, token } = await validateCluster(info, activeSession.token); setupSession(info.baseUrl, user, token); - } catch { } + fail = null; + } catch (e) { + if (e.message === invalidV2Token) { + fail = new Error(`Getting current user for ${session.remoteHost}: ${e.message}`); + } + } } session.status = SessionStatus.VALIDATED; dispatch(authActions.UPDATE_SESSION(session)); + if (fail) { + throw fail; + } + return session; }; @@ -164,7 +179,23 @@ export const validateSessions = () => dispatch(progressIndicatorActions.START_WORKING("sessionsValidation")); for (const session of sessions) { if (session.status === SessionStatus.INVALIDATED) { - await dispatch(validateSession(session, activeSession)); + try { + /* Here we are dispatching a function, not an + action. This is legal (it calls the + function with a 'Dispatch' object as the + first parameter) but the typescript + annotations don't understand this case, so + we get an error from typescript unless + override it using Dispatch. This + pattern is used in a bunch of different + places in Workbench2. */ + await dispatch(validateSession(session, activeSession)); + } catch (e) { + dispatch(snackbarActions.OPEN_SNACKBAR({ + message: e.message, + kind: SnackbarKind.ERROR + })); + } } } services.authService.saveSessions(sessions); @@ -186,7 +217,11 @@ export const addSession = (remoteHost: string, token?: string, sendToLogin?: boo if (useToken) { const info = await getRemoteHostInfo(remoteHost); if (!info) { - return Promise.reject(`Could not get config for ${remoteHost}`); + dispatch(snackbarActions.OPEN_SNACKBAR({ + message: `Could not get config for ${remoteHost}`, + kind: SnackbarKind.ERROR + })); + return; } try { @@ -221,24 +256,33 @@ export const addSession = (remoteHost: string, token?: string, sendToLogin?: boo } } } - return Promise.reject("Could not validate cluster"); + return Promise.reject(new Error("Could not validate cluster")); }; export const toggleSession = (session: Session) => - async (dispatch: Dispatch, getState: () => RootState, services: ServiceRepository) => { - let s = { ...session }; + async (dispatch: Dispatch, getState: () => RootState, services: ServiceRepository) => { + const s: Session = { ...session }; if (session.loggedIn) { s.loggedIn = false; + dispatch(authActions.UPDATE_SESSION(s)); } else { const sessions = getState().auth.sessions; const activeSession = getActiveSession(sessions); if (activeSession) { - s = await dispatch(validateSession(s, activeSession)) as Session; + try { + await dispatch(validateSession(s, activeSession)); + } catch (e) { + dispatch(snackbarActions.OPEN_SNACKBAR({ + message: e.message, + kind: SnackbarKind.ERROR + })); + s.loggedIn = false; + dispatch(authActions.UPDATE_SESSION(s)); + } } } - dispatch(authActions.UPDATE_SESSION(s)); services.authService.saveSessions(getState().auth.sessions); }; diff --git a/src/store/search-results-panel/search-results-middleware-service.ts b/src/store/search-results-panel/search-results-middleware-service.ts index 85f12c3f..84e68ab0 100644 --- a/src/store/search-results-panel/search-results-middleware-service.ts +++ b/src/store/search-results-panel/search-results-middleware-service.ts @@ -42,37 +42,29 @@ export class SearchResultsMiddlewareService extends DataExplorerMiddlewareServic return; } - try { - const params = getParams(dataExplorer, searchValue); - - // TODO: if one of these fails, no results will be returned. - const responses = await Promise.all(sessions.map(session => - this.services.groupsService.contents('', params, session) - )); - - const initial = { - itemsAvailable: 0, - items: [] as GroupContentsResource[], - kind: '', - offset: 0, - limit: 10 - }; - - const mergedResponse = responses.reduce((merged, current) => ({ - ...merged, - itemsAvailable: merged.itemsAvailable + current.itemsAvailable, - items: merged.items.concat(current.items) - }), initial); - - api.dispatch(updateResources(mergedResponse.items)); - - api.dispatch(criteriaChanged - ? setItems(mergedResponse) - : appendItems(mergedResponse)); - - } catch { - api.dispatch(couldNotFetchSearchResults()); + const params = getParams(dataExplorer, searchValue); + + const initial = { + itemsAvailable: 0, + items: [] as GroupContentsResource[], + kind: '', + offset: 0, + limit: 10 + }; + + if (criteriaChanged) { + api.dispatch(setItems(initial)); } + + sessions.map(session => + this.services.groupsService.contents('', params, session) + .then((response) => { + api.dispatch(updateResources(response.items)); + api.dispatch(appendItems(response)); + }).catch(() => { + api.dispatch(couldNotFetchSearchResults(session.clusterId)); + }) + ); } } @@ -118,8 +110,8 @@ export const appendItems = (listResults: ListResults) => items: listResults.items.map(resource => resource.uuid), }); -const couldNotFetchSearchResults = () => +const couldNotFetchSearchResults = (cluster: string) => snackbarActions.OPEN_SNACKBAR({ - message: `Could not fetch search results for some sessions.`, + message: `Could not fetch search results from ${cluster}.`, kind: SnackbarKind.ERROR }); -- 2.30.2