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

Add catches to all promises #474

Open
hukka opened this issue Nov 24, 2017 · 3 comments
Open

Add catches to all promises #474

hukka opened this issue Nov 24, 2017 · 3 comments

Comments

@hukka
Copy link
Contributor

hukka commented Nov 24, 2017

It seems that in some cases promises don't have a .catch() attached on any level. This means that any throws will get silently ignored. For example https://github.com/City-of-Helsinki/kerrokantasi-ui/blob/master/src/actions/user.js#L9 can fail with HTTP level error, or the JSON simply isn't what it should be, or https://github.com/City-of-Helsinki/kerrokantasi-ui/blob/master/src/components/BaseCommentForm.js#L80 might read a malformed base64 (this wasn't an exhaustive list).

@akx
Copy link
Contributor

akx commented Nov 24, 2017

Probably not all promises, but those where it makes sense.

Also, care should be taken to figure out what should be done in those catch handlers.

@hukka
Copy link
Contributor Author

hukka commented Nov 24, 2017

Well, I wonder if it's easier to judge everything case by case and whenever the relevant code is modified compared, than having logging handlers for everything. Sure, it is a bit of code bloat, but coding errors could happen anywhere.

@akx
Copy link
Contributor

akx commented Nov 24, 2017

Maybe we oughta just slap on an unhandledrejection handler then?

window.addEventListener("unhandledrejection", function(err, promise) { 
  
});

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

2 participants