myartsev / ember-simple-auth-jwt

Ember Simple Auth extension for JWT
MIT License
8 stars 3 forks source link

Responses that can not be parsed lead to total failure #16

Open arvraham opened 7 years ago

arvraham commented 7 years ago

When making requests to a backend, it goes like this:

 fetch(url, options).then((response) => {
        response.text().then((text) => {
        let json = text ? JSON.parse(text) : {};
        if (!response.ok) {
            response.responseJSON = json;
            reject(response);
       // ...

If server responses with something other than valid json JSON.parse will crash.

I thought about it a while and came to believe that authenticator mustn't rely solely on response bodies. Status codes should be evaluated before going into detail. For the authenticator it is important to know whether a token is valid or not. Logging reasons is nice and often useful but it mustn't have side effects.

By the way: Fetch responses support parsing json directly:

   fetch(url, options).then((response) => {
          // didn't test, just a guess
          try {
               return response.json();
          } catch (ex) {
               return {};
          }
          // ...
myartsev commented 7 years ago

Agreed - this could be more solid. Note that we do expect a valid JSON response from the server conforming to a certain format, otherwise, how can we parse what the token value is? But yes, it shouldn't crash!

Feel free to send a PR with what you have in mind and we'll take it from there. Thanks!