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

Handle rejected promises #104

Open
yoadsn opened this issue Jan 23, 2017 · 5 comments
Open

Handle rejected promises #104

yoadsn opened this issue Jan 23, 2017 · 5 comments
Labels

Comments

@yoadsn
Copy link

yoadsn commented Jan 23, 2017

I must be missing something here.
If I return a rejected promise from the loader of a component, nothing seems to work as expected.

On the server side, loadOnServer itself is not rejected but rather it continues to resolve the promise and so the component tree gets rendered without the required data. I then get other exceptions due to the missing data which does reject the entire promise. (as expected).

loadOnServer({ ...renderProps, store }).then(() => {
  // Even when the loader promise is rejected this code runs..
})
.catch(err => {
 // this would run eventually when something breaks in a component due to the missing data..
})

On the client side, I am not even sure what to expect - clearly there is no extension point where I can change the routing and redirect to an error page. How should that be handled on the client?

<ReactReduxProvider store={store}>
  <Router  {...renderProps}
    render={(props) => <ReduxAsyncConnect {...props}
                        render={applyRouterMiddleware(useScroll(cusotmScrollBehaviour))}
                   />}
    routes={getRoutes(context)}/>
</ReactReduxProvider>

Any insights on this would be great (I was digging in the sources for a while, but I don't think there is any special handling for this situation)
Thanks!!

@vpanjganj
Copy link

I think you can populate your state with something like error: true or loaded: false. Then check the state in the component and redirect before it mounts. I've been struggling with the exact same problem for the last 3 days. This also has been discussed in Issue #16 .

@yoadsn
Copy link
Author

yoadsn commented Jan 29, 2017

@vpanjganj I was trying to avoid the approach you suggested. I think rendering of the component should not be aware of a data load failure. Duplication of the error handling code at every container component hints at that.
loadOnServer can easily give back an indication of any rejected promises (requires a code change of course). It's the client side rendering which poses a challenge since react router's approach is not inherently callback based and so one would have to re-implement the data loading code in the onEnter callbacks.. that means abandoning redux-connect and re-implementing it. I think I might do just that :/

@vpanjganj
Copy link

@yoadsn ,I think if a component (UI) has to visually respond to a data load failure, it is enough reason to include that in the state and implement the behaviour in the component. To address your concern about redundant code to handle data errors; you can use Higher-ordered components to reuse the logic anywhere necessary.

@AVVS
Copy link
Member

AVVS commented Feb 8, 2017

Thats correct ;)

@AVVS AVVS added the question label Feb 8, 2017
@tswaters
Copy link

If someone is looking for a catch behaviour, you can pull out the error from state and throw if it exists.... something like the following should work. This tosses the first error encountered, but could easily be reworked into aggregating all the errors into an array.

loadOnServer({ ...renderProps, store }).then(() => {
  const {loadState} = store.getState().reduxAsyncConnect
  const err = Object.keys(loadState).reduce((memo, key) => memo || loadState[key].error, null)
  if (err) { throw error }
  // whatever else
})
.catch(err => {
  // a wild error handler appeared!
  next(err)
})

I'd much prefer if it automagically went into the catch block myself, but can appreciate the reasoning / breaking changes / potential for multiple errors, etc. for not doing it like that.

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

No branches or pull requests

4 participants