-
Notifications
You must be signed in to change notification settings - Fork 102
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 error dialogs for uncaught network errors #634
Conversation
src/store/network/index.js
Outdated
const returnVal = method(...props, dispatch, getState, api); | ||
if (returnVal.catch) { | ||
returnVal.catch((e) => dispatch(screen.actions.showError(e))); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you forgot to return returnVal
here
src/store/network/networkActions.js
Outdated
@@ -6,100 +6,99 @@ import createLogger from '../../utils/logger'; | |||
import ActionTypes from './actionTypes'; | |||
import history from '../wallet/history'; | |||
|
|||
import network from './'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this would cause a circular dependency?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, but webpack doesnt seem to care...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
really hard to make this work without this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you have these actions in this file?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yes, but they are wrapped in the index.js
I could wrap them within this file, and call the wrapped variant instead I suppose
Codecov Report
@@ Coverage Diff @@
## master #634 +/- ##
==========================================
+ Coverage 27.38% 27.38% +<.01%
==========================================
Files 157 158 +1
Lines 2991 2998 +7
Branches 404 404
==========================================
+ Hits 819 821 +2
- Misses 1949 1955 +6
+ Partials 223 222 -1
Continue to review full report at Codecov.
|
@whilei addresses #614