Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Error on update/patch call crashes the UI #4276

Closed
salauddinn opened this issue Jan 13, 2020 · 7 comments
Closed

Error on update/patch call crashes the UI #4276

salauddinn opened this issue Jan 13, 2020 · 7 comments
Assignees

Comments

@salauddinn
Copy link

What you were expecting:
Expecting Not to crash the ui when dataProvider update throws error.

What happened instead:
When error dataProvider update throws error ui is crashing. where in the case of create it was handled.
Steps to reproduce:

  1. Go to the comments in sandbox.
  2. Click on any comment
  3. Try to update comment then ui crashes with Uncaught Conflict.
    where as create works fine on error thrown

Related code:
https://codesandbox.io/s/pedantic-stonebraker-4twty

Environment

  • React-admin version:3.1.1
  • Last version that did not exhibit the issue (if applicable):
  • React version:^16.8.6
  • Browser: Chrome(79.0.3945.88)
  • Stack trace (in case of a JS error):

`Conflict

in Transition (created by ForwardRef(Grow))
in ForwardRef(Grow) (created by ForwardRef(Snackbar))
in div (created by ForwardRef(ClickAwayListener))
in ForwardRef(ClickAwayListener) (created by ForwardRef(Snackbar))
in ForwardRef(Snackbar) (created by WithStyles(ForwardRef(Snackbar)))
in WithStyles(ForwardRef(Snackbar)) (created by Notification)
in Notification (created by Layout)
in div (created by Layout)
in div (created by Layout)
in Layout (created by WithStyles(Layout))
in WithStyles(Layout) (created by Context.Consumer)
in withRouter(WithStyles(Layout)) (created by ConnectFunction)
in ConnectFunction (created by LayoutWithTheme)
in ThemeProvider (created by LayoutWithTheme)
in LayoutWithTheme (at Layout.js:42)
in _default (created by Context.Consumer)
in Route (created by CoreAdminRouter)
in Switch (created by CoreAdminRouter)
in div (created by CoreAdminRouter)
in CoreAdminRouter (created by Context.Consumer)
in Route (created by AdminUI)
in Switch (created by AdminUI)
in AdminUI (created by Admin)
in Router (created by ConnectedRouter)
in ConnectedRouter (created by Context.Consumer)
in ConnectedRouterWithContext (created by ConnectFunction)
in ConnectFunction (created by AdminContext)
in TranslationProvider (created by AdminContext)
in Provider (created by AdminContext)
in AdminContext (created by Admin)
in Admin (at src/index.js:19)`
@salauddinn salauddinn changed the title Error on UPDATE call crashes the UI Error on update/patch call crashes the UI Jan 13, 2020
@fzaninotto
Copy link
Member

Your dataProvider should not throw an error, but return a failed promise. I realize the documentation is misleading on this point, so I'm marking this as a documentation issue.

@salauddinn
Copy link
Author

salauddinn commented Jan 13, 2020

Your dataProvider should not throw an error, but return a failed promise. I realize the documentation is misleading on this point, so I'm marking this as a documentation issue.

Thank you.
But why it's not crashing for dataProvider create method and only crashing for dataProvider update. why it's behaving differently for create and update ?

@fzaninotto
Copy link
Member

The behavior in case the dataProvider throws isn't specified. It's supposed to return a Promise.

@estephan
Copy link

I understand that the dataProvider should return a rejected promise on errors instead of throwing, but I think what @salauddinn is seeing here is a possible bug.

Please have a look at the following example: https://codesandbox.io/s/nifty-brahmagupta-963kt
I have modified the dataProvider to return a rejected promise for updates and creates of comments. Even though the dataProvider is returning a rejected promise the UI still crashes (only for updates).

I believe create and update behave differently here because the update, in our case, is executing the performUndoableQuery code.
I have also modified the dataProvider to return a rejected promise for updates to users. Notice if you update a user the UI doesn't crash. This is because I have added undoable={false} to the users <Edit/>.

@fzaninotto you'll be able to understand the code better, but it seems like something is expecting this line to throw a string not an object. If you change that line from throw error; to throw error.message it seems to solve the problem, but I don't think that's the right fix.

@djhi
Copy link
Collaborator

djhi commented Jan 15, 2020

Indeed, I'll take a look. Thanks for the sandbox!

@fzaninotto
Copy link
Member

I can't reproduce the error with the update on master. Perhaps it's already been fixed since 3.0?

@fzaninotto
Copy link
Member

The error that appears on CodeSandbox is actually an artifact of CodeSandbox. It's because useDataProvider returns a rejected Promise when the dataProvider fails. It doesn't happen outside of CodeSandbox, in development as in production.

I consider that #4291 fixes this issue, so I'm closing it. Thanks for the report!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants