Skip to content

Commit 6e19897

Browse files
Fix undo saga crash (#8868)
Noticed via airbrake. - https://scm.airbrake.io/projects/95438/groups/4109902292512992645?filters=%7B%22order%22%3A%22last_notice%22%2C%22resolved%22%3A%22false%22%2C%22search%22%3A%22root%20saga%22%7D&tab=notice-detail - https://scm.airbrake.io/projects/95438/groups/4103951939158207307?filters=%7B%22order%22%3A%22last_notice%22%2C%22resolved%22%3A%22false%22%2C%22search%22%3A%22root%20saga%22%7D&tab=overview The FE sometimes seems to fire update segment groups batch actions that are empty, resulting in the undo saga crashing. I coded a little fix for this. > I added multiple safety guards against this bug: > 1. The segments tab can no longer trigger an `BatchUpdateGroupsAndSegmentsAction` with an empty actions array. > 2. The undo saga is resilient against `BatchUpdateGroupsAndSegmentsAction` actions with an empty payload array. ### URL of deployed dev instance (used for testing): - https://___.webknossos.xyz ### Steps to test: - On master - create an empty segment group - on that group, change the color of all children via context menu on that empty group -> watch wk crash - on this branch, do the same -> the color changing is a no-op without wk crashing. ### Issues: - contributes to #8715 ------ (Please delete unneeded items, merge only when none are left open) - [x] Added changelog entry (create a `$PR_NUMBER.md` file in `unreleased_changes` or use `./tools/create-changelog-entry.py`)
1 parent ba139b2 commit 6e19897

File tree

3 files changed

+12
-4
lines changed

3 files changed

+12
-4
lines changed

frontend/javascripts/viewer/model/sagas/undo_saga.ts

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -409,13 +409,17 @@ export function* manageUndoStates(): Saga<never> {
409409
}
410410
}
411411
} else if (setSegmentGroups || batchUpdateGroupsAndSegments) {
412-
if (setSegmentGroups?.calledFromUndoSaga) {
412+
if (
413+
setSegmentGroups?.calledFromUndoSaga ||
414+
(setSegmentGroups == null && (batchUpdateGroupsAndSegments?.payload?.length ?? 0) <= 0)
415+
) {
413416
// Ignore this action as it was dispatched from within this saga.
417+
// Or in case the BatchUpdateGroupsAndSegmentsAction does not have a payload and thus is a no-op.
414418
continue;
415419
}
416420
shouldClearRedoState = true;
417421
const layerName =
418-
setSegmentGroups?.layerName || batchUpdateGroupsAndSegments?.payload[0].layerName;
422+
setSegmentGroups?.layerName || batchUpdateGroupsAndSegments?.payload?.[0]?.layerName;
419423
if (layerName == null) {
420424
throw new Error("Could not find layer name for action.");
421425
}

frontend/javascripts/viewer/view/right-border-tabs/segments_tab/segments_view.tsx

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -286,7 +286,9 @@ const mapDispatchToProps = (dispatch: Dispatch<any>) => ({
286286
const actions = segmentIds.map((segmentId) =>
287287
updateSegmentAction(segmentId, segmentShape, layerName, undefined, createsNewUndoState),
288288
);
289-
Store.dispatch(batchUpdateGroupsAndSegmentsAction(actions));
289+
if (actions.length > 0) {
290+
Store.dispatch(batchUpdateGroupsAndSegmentsAction(actions));
291+
}
290292
},
291293

292294
updateSegment(
@@ -1340,7 +1342,7 @@ class SegmentsView extends React.Component<Props, State> {
13401342
if (visibleSegmentationLayer == null) return;
13411343
const relevantSegments =
13421344
groupId != null ? this.getSegmentsOfGroupRecursively(groupId) : this.getSelectedSegments();
1343-
if (relevantSegments == null) return;
1345+
if (relevantSegments == null || relevantSegments.length < 1) return;
13441346

13451347
const actions = relevantSegments.map((segment) =>
13461348
updateSegmentAction(segment.id, { color: color }, visibleSegmentationLayer.name),

unreleased_changes/8868.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
### Fixed
2+
- WK crashing when changing the color of an empty segment group.

0 commit comments

Comments
 (0)