Skip to content

Commit

Permalink
fix(core): Reject promise and try-catch instead of resolving homemade…
Browse files Browse the repository at this point in the history
  • Loading branch information
sedubois authored and mxstbr committed Nov 2, 2016
1 parent d95249b commit d32282f
Show file tree
Hide file tree
Showing 4 changed files with 21 additions and 28 deletions.
13 changes: 6 additions & 7 deletions app/containers/HomePage/sagas.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,12 @@ export function* getRepos() {
const username = yield select(selectUsername());
const requestURL = `https://api.github.com/users/${username}/repos?type=all&sort=updated`;

// Call our request helper (see 'utils/request')
const repos = yield call(request, requestURL);

if (!repos.err) {
yield put(reposLoaded(repos.data, username));
} else {
yield put(repoLoadingError(repos.err));
try {
// Call our request helper (see 'utils/request')
const repos = yield call(request, requestURL);
yield put(reposLoaded(repos, username));
} catch (err) {
yield put(repoLoadingError(err));
}
}

Expand Down
22 changes: 9 additions & 13 deletions app/containers/HomePage/tests/sagas.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,23 +34,19 @@ describe('getRepos Saga', () => {
});

it('should dispatch the reposLoaded action if it requests the data successfully', () => {
const response = {
data: [{
name: 'First repo',
}, {
name: 'Second repo',
}],
};
const response = [{
name: 'First repo',
}, {
name: 'Second repo',
}];
const putDescriptor = getReposGenerator.next(response).value;
expect(putDescriptor).toEqual(put(reposLoaded(response.data, username)));
expect(putDescriptor).toEqual(put(reposLoaded(response, username)));
});

it('should call the repoLoadingError action if the response errors', () => {
const response = {
err: 'Some error',
};
const putDescriptor = getReposGenerator.next(response).value;
expect(putDescriptor).toEqual(put(repoLoadingError(response.err)));
const response = new Error('Some error');
const putDescriptor = getReposGenerator.throw(response).value;
expect(putDescriptor).toEqual(put(repoLoadingError(response)));
});
});

Expand Down
6 changes: 2 additions & 4 deletions app/utils/request.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,12 +34,10 @@ function checkStatus(response) {
* @param {string} url The URL we want to request
* @param {object} [options] The options we want to pass to "fetch"
*
* @return {object} An object containing either "data" or "err"
* @return {object} The response data
*/
export default function request(url, options) {
return fetch(url, options)
.then(checkStatus)
.then(parseJSON)
.then((data) => ({ data }))
.catch((err) => ({ err }));
.then(parseJSON);
}
8 changes: 4 additions & 4 deletions app/utils/tests/request.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ describe('request', () => {
request('/thisurliscorrect')
.catch(done)
.then((json) => {
expect(json.data.hello).toEqual('world');
expect(json.hello).toEqual('world');
done();
});
});
Expand All @@ -56,9 +56,9 @@ describe('request', () => {

it('should catch errors', (done) => {
request('/thisdoesntexist')
.then((json) => {
expect(json.err.response.status).toEqual(404);
expect(json.err.response.statusText).toEqual('Not Found');
.catch((err) => {
expect(err.response.status).toEqual(404);
expect(err.response.statusText).toEqual('Not Found');
done();
});
});
Expand Down

0 comments on commit d32282f

Please sign in to comment.