From bee98e1ba05d675018a8f108b04b684c1b8b13a4 Mon Sep 17 00:00:00 2001 From: Daniel Valdivia <18384552+dvaldivia@users.noreply.github.com> Date: Fri, 9 Dec 2022 11:18:19 -0800 Subject: [PATCH] fix: race Condition on Object Browser via Websocket (#2492) Signed-off-by: Daniel Valdivia <18384552+dvaldivia@users.noreply.github.com> --- .../Buckets/BucketDetails/BrowserHandler.tsx | 38 +++++++++++-------- .../Buckets/ListBuckets/ListBuckets.tsx | 4 +- .../Objects/ListObjects/ListObjects.tsx | 14 +++---- .../Objects/ListObjects/ListObjectsTable.tsx | 4 +- .../Objects/ListObjects/RewindEnable.tsx | 4 +- .../ObjectBrowser/objectBrowserSlice.ts | 4 +- 6 files changed, 37 insertions(+), 31 deletions(-) diff --git a/portal-ui/src/screens/Console/Buckets/BucketDetails/BrowserHandler.tsx b/portal-ui/src/screens/Console/Buckets/BucketDetails/BrowserHandler.tsx index fb687580d..d3791a8ff 100644 --- a/portal-ui/src/screens/Console/Buckets/BucketDetails/BrowserHandler.tsx +++ b/portal-ui/src/screens/Console/Buckets/BucketDetails/BrowserHandler.tsx @@ -42,7 +42,7 @@ import { setIsVersioned, setLoadingLocking, setLoadingObjectInfo, - setLoadingObjectsList, + setLoadingObjects, setLoadingRecords, setLoadingVersioning, setLoadingVersions, @@ -82,12 +82,16 @@ const styles = (theme: Theme) => let objectsWS: WebSocket; let currentRequestID: number = 0; let errorCounter: number = 0; +let wsInFlight: boolean = false; const initWSConnection = ( - onMessageCallback: (message: IMessageEvent) => void, openCallback?: () => void, - notAvailableCallback?: () => void + onMessageCallback?: (message: IMessageEvent) => void ) => { + if (wsInFlight) { + return; + } + wsInFlight = true; const url = new URL(window.location.toString()); const isDev = process.env.NODE_ENV === "development"; const port = isDev ? "9090" : url.port; @@ -103,25 +107,28 @@ const initWSConnection = ( ); objectsWS.onopen = () => { + wsInFlight = false; if (openCallback) { openCallback(); } errorCounter = 0; }; + if (onMessageCallback) { + objectsWS.onmessage = onMessageCallback; + } + const reconnectFn = () => { if (errorCounter <= 5) { - initWSConnection(onMessageCallback, openCallback); + initWSConnection(openCallback, onMessageCallback); errorCounter += 1; } else { console.error("Websocket not available."); - if (notAvailableCallback) { - notAvailableCallback(); - } } }; objectsWS.onclose = () => { + wsInFlight = false; console.warn("Websocket Disconnected. Attempting Reconnection..."); // We reconnect after 3 seconds @@ -129,6 +136,7 @@ const initWSConnection = ( }; objectsWS.onerror = () => { + wsInFlight = false; console.error("Error in websocket connection. Attempting reconnection..."); // We reconnect after 3 seconds @@ -136,8 +144,6 @@ const initWSConnection = ( }; }; -initWSConnection(() => {}); - const BrowserHandler = () => { const dispatch = useAppDispatch(); const navigate = useNavigate(); @@ -201,10 +207,10 @@ const BrowserHandler = () => { const obOnly = !!features?.includes("object-browser-only"); /*WS Request Handlers*/ - objectsWS.onmessage = useCallback( + const onMessageCallBack = useCallback( (message: IMessageEvent) => { // reset start status - dispatch(setLoadingObjectsList(false)); + dispatch(setLoadingObjects(false)); const response: WebsocketResponse = JSON.parse(message.data.toString()); if (currentRequestID === response.request_id) { @@ -250,7 +256,7 @@ const BrowserHandler = () => { // This indicates final messages is received. if (response.request_end) { - dispatch(setLoadingObjectsList(false)); + dispatch(setLoadingObjects(false)); dispatch(setLoadingRecords(false)); return; } @@ -283,7 +289,7 @@ const BrowserHandler = () => { // We store the new ID for the requestID currentRequestID = newRequestID; } catch (e) { - console.log(e); + console.error(e); } } else { // Socket is disconnected, we request reconnection but will need to recreate call @@ -291,10 +297,10 @@ const BrowserHandler = () => { initWSRequest(path, date); }; - initWSConnection(dupRequest); + initWSConnection(dupRequest, onMessageCallBack); } }, - [bucketName, rewindEnabled, showDeleted, dispatch] + [bucketName, rewindEnabled, showDeleted, dispatch, onMessageCallBack] ); useEffect(() => { @@ -394,7 +400,7 @@ const BrowserHandler = () => { initWSRequest(pathPrefix, requestDate); } else { - dispatch(setLoadingObjectsList(false)); + dispatch(setLoadingObjects(false)); } // eslint-disable-next-line react-hooks/exhaustive-deps }, [ diff --git a/portal-ui/src/screens/Console/Buckets/ListBuckets/ListBuckets.tsx b/portal-ui/src/screens/Console/Buckets/ListBuckets/ListBuckets.tsx index cafa428b4..b6e6d6036 100644 --- a/portal-ui/src/screens/Console/Buckets/ListBuckets/ListBuckets.tsx +++ b/portal-ui/src/screens/Console/Buckets/ListBuckets/ListBuckets.tsx @@ -64,7 +64,7 @@ import { selFeatures } from "../../consoleSlice"; import AutoColorIcon from "../../Common/Components/AutoColorIcon"; import TooltipWrapper from "../../Common/TooltipWrapper/TooltipWrapper"; import AButton from "../../Common/AButton/AButton"; -import { setLoadingObjectsList } from "../../ObjectBrowser/objectBrowserSlice"; +import { setLoadingObjects } from "../../ObjectBrowser/objectBrowserSlice"; const styles = (theme: Theme) => createStyles({ @@ -124,7 +124,7 @@ const ListBuckets = ({ classes }: IListBucketsProps) => { .then((res: BucketList) => { setLoading(false); setRecords(res.buckets || []); - dispatch(setLoadingObjectsList(true)); + dispatch(setLoadingObjects(true)); }) .catch((err: ErrorResponseHandler) => { setLoading(false); diff --git a/portal-ui/src/screens/Console/Buckets/ListBuckets/Objects/ListObjects/ListObjects.tsx b/portal-ui/src/screens/Console/Buckets/ListBuckets/Objects/ListObjects/ListObjects.tsx index 89756fa10..b1605a7b3 100644 --- a/portal-ui/src/screens/Console/Buckets/ListBuckets/Objects/ListObjects/ListObjects.tsx +++ b/portal-ui/src/screens/Console/Buckets/ListBuckets/Objects/ListObjects/ListObjects.tsx @@ -96,7 +96,7 @@ import { resetMessages, resetRewind, setDownloadRenameModal, - setLoadingObjectsList, + setLoadingObjects, setLoadingRecords, setLoadingVersions, setNewObject, @@ -317,7 +317,7 @@ const ListObjects = () => { useEffect(() => { dispatch(setSearchObjects("")); - dispatch(setLoadingObjectsList(true)); + dispatch(setLoadingObjects(true)); dispatch(setSelectedObjects([])); }, [simplePath, dispatch]); @@ -424,7 +424,7 @@ const ListObjects = () => { if (refresh) { dispatch(setSnackBarMessage(`Objects deleted successfully.`)); dispatch(setSelectedObjects([])); - dispatch(setLoadingObjectsList(true)); + dispatch(setLoadingObjects(true)); } }; @@ -595,7 +595,7 @@ const ListObjects = () => { }; xhr.onloadend = () => { if (files.length === 0) { - dispatch(setLoadingObjectsList(true)); + dispatch(setLoadingObjects(true)); } }; xhr.onabort = () => { @@ -650,7 +650,7 @@ const ListObjects = () => { dispatch(setErrorSnackMessage(err)); } // We force objects list reload after all promises were handled - dispatch(setLoadingObjectsList(true)); + dispatch(setLoadingObjects(true)); dispatch(setSelectedObjects([])); }); }; @@ -736,7 +736,7 @@ const ListObjects = () => { dispatch(setSelectedObjects([])); if (forceRefresh) { - dispatch(setLoadingObjectsList(true)); + dispatch(setLoadingObjects(true)); } }; @@ -949,7 +949,7 @@ const ListObjects = () => { } else { dispatch(resetMessages()); dispatch(setLoadingRecords(true)); - dispatch(setLoadingObjectsList(true)); + dispatch(setLoadingObjects(true)); } }} disabled={ diff --git a/portal-ui/src/screens/Console/Buckets/ListBuckets/Objects/ListObjects/ListObjectsTable.tsx b/portal-ui/src/screens/Console/Buckets/ListBuckets/Objects/ListObjects/ListObjectsTable.tsx index 041e979d7..89720cacc 100644 --- a/portal-ui/src/screens/Console/Buckets/ListBuckets/Objects/ListObjects/ListObjectsTable.tsx +++ b/portal-ui/src/screens/Console/Buckets/ListBuckets/Objects/ListObjects/ListObjectsTable.tsx @@ -27,7 +27,7 @@ import { AppState, useAppDispatch } from "../../../../../../store"; import { selFeatures } from "../../../../consoleSlice"; import { encodeURLString } from "../../../../../../common/utils"; import { - setLoadingObjectsList, + setLoadingObjects, setLoadingVersions, setObjectDetailsView, setSelectedObjects, @@ -168,7 +168,7 @@ const ListObjectsTable = () => { const newSortDirection = get(sortData, "sortDirection", "DESC"); setCurrentSortField(sortData.sortBy); setSortDirection(newSortDirection); - dispatch(setLoadingObjectsList(true)); + dispatch(setLoadingObjects(true)); }; const selectAllItems = () => { diff --git a/portal-ui/src/screens/Console/Buckets/ListBuckets/Objects/ListObjects/RewindEnable.tsx b/portal-ui/src/screens/Console/Buckets/ListBuckets/Objects/ListObjects/RewindEnable.tsx index 3933d0e55..311fe9356 100644 --- a/portal-ui/src/screens/Console/Buckets/ListBuckets/Objects/ListObjects/RewindEnable.tsx +++ b/portal-ui/src/screens/Console/Buckets/ListBuckets/Objects/ListObjects/RewindEnable.tsx @@ -25,7 +25,7 @@ import FormSwitchWrapper from "../../../../Common/FormComponents/FormSwitchWrapp import { AppState, useAppDispatch } from "../../../../../../store"; import { resetRewind, - setLoadingObjectsList, + setLoadingObjects, setRewindEnable, } from "../../../../ObjectBrowser/objectBrowserSlice"; @@ -73,7 +73,7 @@ const RewindEnable = ({ }) ); } - dispatch(setLoadingObjectsList(true)); + dispatch(setLoadingObjects(true)); closeModalAndRefresh(); }; diff --git a/portal-ui/src/screens/Console/ObjectBrowser/objectBrowserSlice.ts b/portal-ui/src/screens/Console/ObjectBrowser/objectBrowserSlice.ts index 82d22235a..e0276b240 100644 --- a/portal-ui/src/screens/Console/ObjectBrowser/objectBrowserSlice.ts +++ b/portal-ui/src/screens/Console/ObjectBrowser/objectBrowserSlice.ts @@ -233,7 +233,7 @@ export const objectBrowserSlice = createSlice({ setSearchObjects: (state, action: PayloadAction) => { state.searchObjects = action.payload; }, - setLoadingObjectsList: (state, action: PayloadAction) => { + setLoadingObjects: (state, action: PayloadAction) => { state.loadingObjects = action.payload; }, setSearchVersions: (state, action: PayloadAction) => { @@ -352,7 +352,7 @@ export const { openList, closeList, setSearchObjects, - setLoadingObjectsList, + setLoadingObjects, cancelObjectInList, setSearchVersions, setSelectedVersion,