Bug #22143
closedProcess page crash
Files
Updated by Peter Amstutz 3 months ago
https://workbench.tordo.arvadosapi.com/processes/tordo-xvhdp-xe0kp4vd0qjyx6f
I think I was seeing this on the #13327 or #22132 branch, those branches don't don't do anything related to the collection panel file listing, so I'm not sure what's going on.
Updated by Peter Amstutz 3 months ago
- Assigned To changed from Stephen Smith to Lisa Knox
Updated by Lisa Knox 2 months ago
22143-process-page-crash @ 62c20a4bdb2f56ead82573ff7590ada4614b92da
developer-run-tests-services-workbench2: #1235
- ✅ All agreed upon points are implemented / addressed.
- ✅ Anything not implemented (discovered or discussed during work) has a follow-up story.
- ✅ Code is tested and passing, both automated and manual, what manual testing was done is described
- - Documentation has been updated.
- - Behaves appropriately at the intended scale (describe intended scale).
- ✅ Considered backwards and forwards compatibility issues between client and server.
no changes - ✅ Follows our coding standards and GUI style guidelines.
- I was able to reproduce the bug at one point but then I was unable to reproduce it again. I believe when I reproduced it, I had switched from one branch to another with a different package.json without re-installing, but I am unable to confirm this. I addressed what I could by adding a null check where the stack trace points, but until we can reproduce it we can't be 100% sure that it's fixed.
Updated by Stephen Smith 2 months ago
I think this is ok as a workaround, but I'm also wondering about one of the uses of mapNodeValue
. setNodeValue
and all uses of setNodeValueWith
explicitly pass in anonymous functions for mapFn, and the last use, mapTreeValues
, is mostly called passing in an anonymous function, the only exception being 2 uses where it uses services.collectionService.extendFileURL
as the mapFn. I noticed that one of these in collection-panel-files-actions.ts
references the collectionService using servicesProvider.getServices().collectionService.extendFileURL
instead of using a thunk to access services, which results in extendFileURL
being inferred as an any type, and I'm wondering if maybe there isn't a guarantee that getServices() is reliable. So I would suggest trying to change that action into a thunk which seems to satisfy the type system better and makes mapped
be a proper tree type instead of any
. I have no clue if this will fix the issue but I think it would be good to do as long as the collection file browser seems unaffected, which from what I can tell should be fine.
--- a/services/workbench2/src/components/collection-panel-files/collection-panel-files.tsx
+++ b/services/workbench2/src/components/collection-panel-files/collection-panel-files.tsx
@@ -335,12 +335,12 @@ export const CollectionPanelFiles = withStyles(styles)(
fetchData([leftKey, rightKey], true);
}
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [currentPDH]);
React.useEffect(() => {
if (rightData) {
const filtered = rightData.filter(({ name }) => name.indexOf(rightSearch) > -1);
- setCollectionFiles(filtered, false)(dispatch);
+ dispatch(setCollectionFiles(filtered, false));
}
}, [rightData, dispatch, rightSearch]);
diff --git a/services/workbench2/src/store/collection-panel/collection-panel-files/collection-panel-files-actions.ts b/services/workbench2/src/store/collection-panel/collection-panel-files/collection-panel-files-actions.ts
index 547f1534d1..4e943e42f7 100644
--- a/services/workbench2/src/store/collection-panel/collection-panel-files/collection-panel-files-actions.ts
+++ b/services/workbench2/src/store/collection-panel/collection-panel-files/collection-panel-files-actions.ts
@@ -29,10 +29,10 @@ export type CollectionPanelFilesAction = UnionOf<typeof collectionPanelFilesActi
export const COLLECTION_PANEL_LOAD_FILES = 'collectionPanelLoadFiles';
-export const setCollectionFiles = (files, joinParents = true) => (dispatch: any) => {
+export const setCollectionFiles = (files, joinParents = true) => (dispatch: Dispatch, getState: () => RootState, services: ServiceRepository) => {
const tree = createCollectionFilesTree(files, joinParents);
const sorted = sortFilesTree(tree);
- const mapped = mapTreeValues(servicesProvider.getServices().collectionService.extendFileURL)(sorted);
+ const mapped = mapTreeValues(services.collectionService.extendFileURL)(sorted);
dispatch(collectionPanelFilesAction.SET_COLLECTION_FILES(mapped));
};
Updated by Lisa Knox 2 months ago
22143-process-page-crash @ commit: 89d4d8b32e7783855ed8ab3b2ea3861a3e60b28f
developer-run-tests-services-workbench2: #1238
I took your suggestion and also added typing for the `pathData` variable while I was there.
Updated by Peter Amstutz 2 months ago
- Target version changed from Development 2024-10-09 sprint to Development 2024-10-23 sprint
Updated by Lisa Knox 2 months ago
- Status changed from In Progress to Resolved
Applied in changeset arvados|28269c566bbb668342aa73196250dfd3907f58c5.