Http error handling #1273
Http error handling #1273
Conversation
|
Replace #1257 .. was getting too messy |
|
@jpetitcolas All yours :) |
|
Fix #1031 |
| ``` | ||
| Bind the decorator to your application: | ||
| ```js | ||
| myApp.decorator('httpErrorService',require('HttpErrorDecorator')); |
There was a problem hiding this comment.
s/httpErrorService/HttpErrorService/
|
Tests are green. @Kmaschta, I let you merge? |
| this.handle404Error(event, error); | ||
| break; | ||
| case 403: | ||
| this.handle403Error(error); |
There was a problem hiding this comment.
Please, be consistent and give the same API for all the handlers: (event, error) or (error, event).
I'm even in favor of (error, event, toState, toParams, fromState, fromParams).
It will be more customable for the next developers and the doc will be more clear too.
There was a problem hiding this comment.
if you do this I get sonar style check errors because the parameters are not used in the method (Unused function parameters should be removed javascript : UnusedFunctionArgument)
Nothing is mentionned about this in google guidelines.
Dev can override this in the decorator anyway
| @@ -38,6 +38,7 @@ export default function translate($translateProvider) { | |||
| 'NEXT': 'Next »', | |||
| 'DETAIL': 'Detail', | |||
| 'STATE_CHANGE_ERROR': 'State change error: {{ message }}', | |||
| 'STATE_FORBIDDEN_ERROR': 'A server 403 error occured: {{ message }}', | |||
There was a problem hiding this comment.
A server 403 error occured to A permission error occured?
There was a problem hiding this comment.
Well 403 error could be something else than permission, so not sure. I will let you debate with @jpetitcolas as this was is wording for the error when we started discussing this issue (#1031 (comment))
There was a problem hiding this comment.
Let use current message, as it will be useful for users to report problem to their developper. Moreover, it is easily overridable now.
Hopefully the final chapter :)
I have modified the documentation example and using similar code our deployment