React-beautiful-dnd: Improve how we handle errors

[lakesare]

Bug or feature request?

Vague, I'd consider it a bug.

Expected behavior

React-beautiful-dnd doesn't touch errors that don't relate to it.

Actual behavior

Instead of one error, I get 5 bulky errors that have nothing do with react-beautiful-dnd.

What version of React are you using?

16.7.0.

What version of react-beautiful-dnd are you running?

10.0.3

What browser are you using?

Chrome.

[Butch-1337]

Any updates on this issue?

[ghost]

Have recreated the issue where it appears a drag is occuring all the time, at least the error message says so. Can be found here. npm-run-start then click the button

[cnrudd]

Seeing same issue on:
React 16.8.4
react-beautiful-dnd: 11.03
Chrome: 74.0.3729.169

[alexreardon]

I am a bit busy right now so i cannot pour myself into this one. If somebody wants to take a deeper look at this one just let me know

[thesuperhomie]

I'm happy to take this on if nobody else has any objections?

[alexreardon]

Go for it!

Here is the logic now:

• any errors on window: cancel the current drag
• any invariant errors: cancel the drag and swallow the error 👎
• any other errors: cancel the drag and throw the error (which can lead to a double error print as it hits the window handler)

Ideally:

• we would capture any internal errors and recover without throwing
• we would not double print other errors

[thesuperhomie]

Awesome thanks for the context!

Just a heads up had to edit the rollup config to this:

const commonjsArgs = {
  include: 'node_modules/**',
  // needed for react-is via react-redux
  // https://stackoverflow.com/questions/50080893/rollup-error-isvalidelementtype-is-not-exported-by-node-modules-react-is-inde/50098540
  namedExports: {
    'node_modules/react-redux/node_modules/react-is/index.js': [
      'isValidElementType',
      'isContextConsumer',
    ],
    'node_modules/react-is/index.js': [
      'isValidElementType',
      'isContextConsumer',
    ],
    'node_modules/react-redux/es/components/connectAdvanced.js': [
      'isContextConsumer',
    ],
  },
};

for the build to run for me. Not sure if it's happening to anyone else contributing but thought I'd let you know :)

[alexreardon]

I think we need to fix this. Right now things suck.

What we want to avoid

• logging errors that have nothing to do with rbd
• recovering from errors in componentDidCatch that have nothing to do with rbd
• double logging of errors

The plan

componentDidCatch

• any error source: If a drag is occurring, cancel the drag
• rbd error: log the error
• rbd error:
  
  
  
  • recovery mode (default): swallow the error and try to recover using .setState({})
  
  
  • abort mode: throw the error and don't recover using .setState({})
  
  
• non-rbd error: same as abort mode: throw the error and don't recover using .setState({})

Window error event listener

• any error source: If a drag is occurring, cancel the drag
• rbd error: log the error

That's all I think

In order to know what errors are caused by our invariant's we will need to (Potentially) move away from tiny-invariant so we can throw our own custom error type (something like RbdInvariant)

[alexreardon]

Given this involves some error recovery behaviour changes, it would be great to get it into our next breaking change version #1317

[alexreardon]

Shipping in 12.0!