added /login redirect in case of error 401 from API #757
Conversation
| @@ -82,7 +82,7 @@ export function request(method, uri, data, options = {}) { | |||
| .then( | |||
| response => response.body, | |||
| err => { | |||
| if (err.statusCode == 401) { | |||
| if (err.message === 'Unauthorized') { | |||
There was a problem hiding this comment.
Why this change has been introduced ? Does err not have statusCode field ?
There was a problem hiding this comment.
err is an instance of Error object, and it appeared that it doesnt have statusCode field.
Only accessible parameter allowing to determine the error type is the message.
There was a problem hiding this comment.
Actually err has status field which you could use. (but not statusCode, I think statusCode has been added when we have isomorphic-fetch instead of superagent). https://github.com/visionmedia/superagent/blob/master/lib/client.js#L426
And err.message is equals to response.statusText so, I think both approaches could work here
There was a problem hiding this comment.
And what about fixing this bug in api-js and reuse this library here instead of fixing bugs it two places at one time ? cc @jmataya
There was a problem hiding this comment.
I see lack of TypeScript here...
There was a problem hiding this comment.
In that case I'd better change the if statement to check the err.status instead of message, its better to stick to how it was done originally.
There was a problem hiding this comment.
@narma i'm feeling the same, bro
There was a problem hiding this comment.
To be honest I didn't get the second question.
|
I think we can merge this PR and decide about api-js reusage later |
|
OK, can I merge it? |
|
merged ) |
Fixed the bug with invalid JWT.
When API call responds 401 error, system now redirects user to /admin/login page.
https://trello.com/c/4sNkIU69