Project

General

Profile

Actions

Bug #18116

closed

Advanced search -> Search by properties -> Add crashes

Added by Peter Amstutz over 2 years ago. Updated over 2 years ago.

Status:
Resolved
Priority:
Normal
Assigned To:
Category:
Workbench2
Target version:
Story points:
-
Release relationship:
Auto

Description

Searching by property values in advanced search causes the app to crash.

  1. Open advanced search
  2. Select a property key
  3. select a property value
  4. Click on "Add"
  5. The page goes blank. The console has an error back trace but it is not very helpful because the code has been minified. Tested on pirca and ce8i5.

Subtasks 1 (0 open1 closed)

Task #18158: Review 18116-fix-chips-bugsResolvedStephen Smith10/08/2021Actions

Related issues

Related to Arvados - Bug #18257: Chips error in workflow File/Directory array inputsResolvedLucas Di Pentima12/01/2021Actions
Actions #1

Updated by Peter Amstutz over 2 years ago

  • Description updated (diff)
  • Subject changed from Advanced search -> Search by properties -> Add crashes to "Advanced search -> Search by properties -> Add" crash
Actions #2

Updated by Peter Amstutz over 2 years ago

  • Assigned To set to Stephen Smith
  • Subject changed from "Advanced search -> Search by properties -> Add" crash to Advanced search -> Search by properties -> Add crashes
Actions #3

Updated by Stephen Smith over 2 years ago

  • Status changed from New to In Progress
Actions #4

Updated by Lucas Di Pentima over 2 years ago

Haven't found the reason for the app crash but I narrowed it down to the following:

diff --git a/src/components/chips/chips.tsx b/src/components/chips/chips.tsx
index 2a6fafc3..28b1f456 100644
--- a/src/components/chips/chips.tsx
+++ b/src/components/chips/chips.tsx
@@ -45,7 +45,7 @@ export const Chips = withStyles(styles)(

         renderChip = (value: Value, index: number) =>
             <Grid item key={index}>
-                <this.chip {...{ value }} />
+                {/* <this.chip {...{ value }} /> */}
             </Grid>

         type = 'chip';

To add more mystery to the subject, the Chips component is used on other parts of the app, and AFAICT doesn't make the app crash there.

Actions #5

Updated by Lucas Di Pentima over 2 years ago

There's evidently something wrong with chips.tsx code, and it seems to me that's unnecessarily complicated. This update makes things work:

diff --git a/src/components/chips/chips.tsx b/src/components/chips/chips.tsx
index 2a6fafc3..1f3d655b 100644
--- a/src/components/chips/chips.tsx
+++ b/src/components/chips/chips.tsx
@@ -43,10 +43,13 @@ export const Chips = withStyles(styles)(
             </Grid>;
         }

-        renderChip = (value: Value, index: number) =>
-            <Grid item key={index}>
-                <this.chip {...{ value }} />
+        renderChip = (value: Value, index: number) => {
+            const { deletable, getLabel } = this.props;
+            return <Grid item key={index}>
+                <Chip onDelete={deletable ? this.deleteValue(value) : undefined}
+                    label={getLabel !== undefined ? getLabel(value) : value} />
             </Grid>
+        }

         type = 'chip';

There's cleanup to do and confirming other users of the <Chips> component still work, and it would be good to know why the previous version failed on production only.

Actions #6

Updated by Peter Amstutz over 2 years ago

  • Target version changed from 2021-09-15 sprint to 2021-09-29 sprint
Actions #7

Updated by Stephen Smith over 2 years ago

Changes so far at arvados-workbench2|441fa734bfe5d3158f19d3172aac9dc09e03b800
Tests developer-tests-workbench2: #487

  • Applied the workaround from above
  • Added guards for null values in several places

There still seems to be an error in directory-array-input and file-array-input.

Debugging with this patch

diff --git a/src/views/run-process-panel/inputs/directory-array-input.tsx b/src/views/run-process-panel/inputs/directory-array-input.tsx
index 1b04718d..6a47dae1 100644
--- a/src/views/run-process-panel/inputs/directory-array-input.tsx
+++ b/src/views/run-process-panel/inputs/directory-array-input.tsx
@@ -239,7 +239,7 @@ const DirectoryArrayInputComponent = connect(mapStateToProps)(
                 maxWidth='md' >
                 <DialogTitle>Choose collections</DialogTitle>
                 <DialogContent>
-                    <this.dialogContent />
+                    {/*<this.dialogContent />*/}
                 </DialogContent>
                 <DialogActions>
                     <Button onClick={this.closeDialog}>Cancel</Button>
@@ -272,29 +272,10 @@ const DirectoryArrayInputComponent = connect(mapStateToProps)(
             },
         })

-        dialogContent = withStyles(this.dialogContentStyles)(
-            ({ classes }: WithStyles<DialogContentCssRules>) =>
-                <div className={classes.root}>
-                    <div className={classes.tree}>
-                        <ProjectsTreePicker
-                            pickerId={this.props.commandInput.id}
-                            includeCollections
-                            showSelection
-                            options={this.props.options}
-                            toggleItemSelection={this.refreshDirectories} />
-                    </div>
-                    <Divider />
-                    <div className={classes.chips}>
-                        <Typography variant='subtitle1'>Selected collections ({this.state.directories.length}):</Typography>
-                        <Chips
-                            orderable
-                            deletable
-                            values={this.state.directories}
-                            onChange={this.setDirectories}
-                            getLabel={(directory: CollectionResource) => directory.name} />
-                    </div>
-                </div>
-        );
+        dialogContent = () =>
+            <div>
+              Hello
+            </div>

     });

The error is gone but uncommenting this.dialogContent makes the invariant error happen again, despite stripping down dialogContent.

Invariant Violation: Minified React error #130; visit https://reactjs.org/docs/error-decoder.html?invariant=130&args[]=undefined&args[]= for the full message or use the non-minified dev environment for full errors and additional helpful warnings. 
    React 28
        a
        s
        Xr
        yi
        mo
        Do
        Fa
        za
        Ss
        xs
        Ms
        Mn
        kn
        vr
        Ra
        Fa
        za
        Ss
        xs
        ms
        Ya
        Ts
        As
        render
        Fs
        Ds
        Fs
        render
    1087 index.tsx:169
    promise callback*1087 index.tsx:105
    a run-process:1
    609 main.5f2f9600.chunk.js:1
    a run-process:1
    t run-process:1
    r run-process:1
    <anonymous> main.5f2f9600.chunk.js:1
Actions #8

Updated by Lucas Di Pentima over 2 years ago

Updates at arvados-workbench2|52c68cf3

I was able to make some progress in getting a standard workaround, but I haven't been able to correctly pass the values to the Chip component by instantiating it in this different way; maybe this is helpful for you to detect the underlying problem.

The code revert on chips.tsx makes the Chip show itself without value on the dialog. Maybe I'm missing something super obvious there.

Actions #9

Updated by Lucas Di Pentima over 2 years ago

The latest commit without this problem is arvados-workbench2|7edd5a7 which corresponds to version 2.2.1

Actions #10

Updated by Peter Amstutz over 2 years ago

  • Release set to 42
Actions #11

Updated by Peter Amstutz over 2 years ago

  • Target version changed from 2021-09-29 sprint to 2021-10-13 sprint
Actions #12

Updated by Stephen Smith over 2 years ago

  • Related to Bug #18257: Chips error in workflow File/Directory array inputs added
Actions #13

Updated by Stephen Smith over 2 years ago

Current changes at arvados-workbench2|52c68cf3ac73e4155dcadc6de452e87922afb30c branch 18116-fix-chips-bugs

Tests are failing on the workflow portion: developer-tests-workbench2: #494

Curiously, the current main branch passes all tests on jenkins but fails locally both in dev mode and when built with yarn build, and I can see that the issue with the workflow portion exists in ce8i5 so I don't think this is actually a regression.

Actions #14

Updated by Lucas Di Pentima over 2 years ago

  • What I think we want to merge is arvados-workbench2|4eff29a, do you agree? The rest should be taken care of on the other branch, IMO.
  • I've launched a test run for this commit at: developer-tests-workbench2: #496
  • Also, I've made a new branch 18257-chips-component-issue from this one, so we don't lose those changes.
  • If you also want to merge the guards on arvados-workbench2|441fa73, please go ahead if they don't break the tests.
Actions #16

Updated by Stephen Smith over 2 years ago

  • Status changed from In Progress to Resolved
Actions

Also available in: Atom PDF