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

Improvement idea: bubble errors from clientMiddleware #359

Open
quicksnap opened this issue Oct 16, 2015 · 5 comments
Open

Improvement idea: bubble errors from clientMiddleware #359

quicksnap opened this issue Oct 16, 2015 · 5 comments

Comments

@quicksnap
Copy link
Collaborator

See comparison: (Edit: link was here, but whoops, I force-pushed over my master branch)

First, I changed next to dispatch within the promise callbacks. Doesn't seem necessary to call next in these contexts. Am I wrong?

Next, by default we will just swallow errors and return them as success by default. But, if you pass in bubbleErrors: true, instead the promise will throw, and you can catch as in my example.

This is useful for cases where you'd want to grab an error and display in a special way, such as calling dispatch to send it to a global error component.

Thoughts?

@erikras
Copy link
Owner

erikras commented Oct 16, 2015

Very interesting. I like how it's a feature that you needn't know about unless you want to use it.

PR that baby on in here.

@quicksnap
Copy link
Collaborator Author

I've been messing with error handling.. I have another idea that wraps dispatcher to catch errors by default. I'll have code examples here soon.

@quicksnap
Copy link
Collaborator Author

Ok, here's the updated idea:

By default, clientMiddleware will return a rejected promise. If the user neglects to handle the error via then or catch, it will throw an error.

To make things easier, there are two utilities added in safeDispatch for wrapping and handling the error, but still allowing usage of then.

@quicksnap
Copy link
Collaborator Author

Link: master...quicksnap:idea123

@vyorkin
Copy link

vyorkin commented Dec 10, 2015

@erikras Why do you need this catch? Isn't it a duplication of the same thing? Maybe I've missed smth, but as I can see it could be enough to handle just a promise rejection without catch handler:

new Promise((resolve, reject) => {
  resolve('good');
}).then(
  result => console.log('yo: ', result),
  error => console.log('oops: ', error)
);

new Promise((resolve, reject) => {
  reject('bad');
}).then(
  result => console.log('yo: ', result),
  error => console.log('oops: ', error)
);

new Promise((resolve, reject) => {
  throw new Error('too bad');
}).then(
  result => console.log('yo: ', result),
  error => console.log('oops: ', error)
);

new Promise((resolve, reject) => resolve('good'))
  .then(result => console.log('yo: ', result))
  .catch(error => console.log('oops: ', error));;

new Promise((resolve, reject) => reject('bad'))
  .then(result => console.log('yo: ', result))
  .catch(error => console.log('oops: ', error));;

new Promise((resolve, reject) => {
  throw new Error('too bad');
})
.then(result => console.log('yo: ', result))
.catch(error => console.log('oops: ', error));;

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

No branches or pull requests

3 participants