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

remove redundant try...catches #240

Merged
merged 1 commit into from
Jun 26, 2018

Conversation

bekzod
Copy link
Contributor

@bekzod bekzod commented Jun 25, 2018

No description provided.

@bekzod bekzod force-pushed the cleanup-promises branch from d6da648 to df186c7 Compare June 25, 2018 12:28
Copy link
Owner

@fenichelar fenichelar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe the try/catch in makeRequest is required to achieve the desired behavior of handling both JSON and HTML errors.

@bekzod bekzod force-pushed the cleanup-promises branch from df186c7 to bf20ef7 Compare June 25, 2018 13:57
@bekzod
Copy link
Contributor Author

bekzod commented Jun 25, 2018

@fenichelar I think I fixed that now :)

@fenichelar
Copy link
Owner

@bekzod Can you please update to pass linter? Maybe something like this:

return fetch(url, {
  method: 'POST',
  headers: assign({
    'Accept': 'application/json',
    'Content-Type': 'application/json'
  }, headers),
  body: JSON.stringify(data)
}).then(response => {
  const res = {
    statusText: response.statusText,
    status: response.status,
    headers: response.headers
  };
  response.text().then(text => {
    res.text = text;
    try {
      res.json = JSON.parse(text);
    } catch (e) {
      return reject(res);
    }
    if (response.ok) {
      return resolve(res);
    } else {
      return reject(res);
    }
  }).catch(() => {
    return reject(res);
  });
}).catch(error => {
  return reject(error);
});

@bekzod bekzod force-pushed the cleanup-promises branch from bf20ef7 to 84b7158 Compare June 25, 2018 19:56
@bekzod bekzod force-pushed the cleanup-promises branch from 84b7158 to 568e0bf Compare June 25, 2018 20:10
@bekzod
Copy link
Contributor Author

bekzod commented Jun 25, 2018

@fenichelar done

@fenichelar fenichelar merged commit 75c476c into fenichelar:master Jun 26, 2018
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

Successfully merging this pull request may close these issues.

2 participants