Skip to content

Commit

Permalink
fetchActions tests: Test retry logic with a more representative error.
Browse files Browse the repository at this point in the history
The fact that this test (before this commit) was passing isn't good
-- it means a basically arbitrary, unexpected kind of error will let
the retry loop continue without propagating to `tryFetch`'s caller.
Instead, if something unexpected like that happens, we should fail
early (and stop wasting the user's time) by breaking out of the
retry loop and having the error propagate to the caller.

We plan to fix that in a later series of commits, and add a test
case with an error like that. But for now, test that we get the
right behavior with representative inputs.
  • Loading branch information
chrisbobbe committed May 21, 2021
1 parent c7c696e commit 9f54560
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 3 deletions.
15 changes: 12 additions & 3 deletions src/message/__tests__/fetchActions-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -116,13 +116,22 @@ describe('fetchActions', () => {
jest.runAllTimers();
});

test('retries a call, if there is an exception', async () => {
// TODO: test more errors, like regular `new Error()`s. Unexpected
// errors should actually cause the retry loop to break; we'll fix
// that soon.
test('retries a call if there is a non-client error', async () => {
const serverError = new ApiError(500, {
code: 'SOME_ERROR_CODE',
msg: 'Internal Server Error',
result: 'error',
});

// fail on first call, succeed second time
let callCount = 0;
const thrower = jest.fn(() => {
callCount++;
if (callCount === 1) {
throw new Error('First run exception');
throw serverError;
}
return 'hello';
});
Expand All @@ -135,7 +144,7 @@ describe('fetchActions', () => {
await expect(tryFetch(tryFetchFunc)).resolves.toBe('hello');

expect(tryFetchFunc).toHaveBeenCalledTimes(2);
await expect(tryFetchFunc.mock.results[0].value).rejects.toThrow('First run exception');
await expect(tryFetchFunc.mock.results[0].value).rejects.toThrow(serverError);
await expect(tryFetchFunc.mock.results[1].value).resolves.toBe('hello');

jest.runAllTimers();
Expand Down
3 changes: 3 additions & 0 deletions src/message/fetchActions.js
Original file line number Diff line number Diff line change
Expand Up @@ -265,6 +265,9 @@ export async function tryFetch<T>(func: () => Promise<T>): Promise<T> {
try {
return await func();
} catch (e) {
// TODO: This should be narrowed to `!isServerError(e)`; we
// should fail early if we encounter unrecognized / unexpected
// errors.
if (isClientError(e)) {
throw e;
}
Expand Down

0 comments on commit 9f54560

Please sign in to comment.