Mirror: The highly customizable and versatile GraphQL client with which you add on features like normalized caching as you grow.
Filter results that are being retried from retryExchange's output (#592)
* retryExchange now only returns an error if the max number of attempts has been exceeded
* remove unnecessary typings
*make tests more robust
* update MAX_ATTEMPTS default
authored by
Will Golledge
and committed by
GitHub
e66c1962
981b6465
+72
-59
+47
-14
exchanges/retry/src/retryExchange.test.ts
+47
-14
exchanges/retry/src/retryExchange.test.ts
···
8
8
OperationResult,
9
9
ExchangeIO,
10
10
} from '@urql/core';
11
-
import { retryExchange, OperationWithRetry } from './retryExchange';
11
+
import { retryExchange } from './retryExchange';
12
12
13
13
beforeEach(() => {
14
14
jest.useFakeTimers();
···
57
57
query: queryOne,
58
58
});
59
59
60
-
({ source: ops$, next } = makeSubject<OperationWithRetry>());
60
+
({ source: ops$, next } = makeSubject<Operation>());
61
61
});
62
62
63
-
it('retries if it hits an error', () => {
63
+
it(`retries if it hits an error and works for multiple concurrent operations`, () => {
64
+
const queryTwo = gql`
65
+
{
66
+
films {
67
+
id
68
+
name
69
+
}
70
+
}
71
+
`;
72
+
const queryTwoError = {
73
+
name: 'error2',
74
+
message: 'scary error2',
75
+
};
76
+
const opTwo = client.createRequestOperation('query', {
77
+
key: 2,
78
+
query: queryTwo,
79
+
});
80
+
64
81
const response = jest.fn(
65
82
(forwardOp: Operation): OperationResult => {
66
-
expect(forwardOp.key).toBe(op.key);
83
+
expect(
84
+
forwardOp.key === op.key || forwardOp.key === opTwo.key
85
+
).toBeTruthy();
86
+
67
87
return {
68
88
operation: forwardOp,
69
89
// @ts-ignore
70
-
error: queryOneError,
90
+
error: forwardOp.key === 2 ? queryTwoError : queryOneError,
71
91
};
72
92
}
73
93
);
···
91
111
);
92
112
93
113
next(op);
94
-
// Once for failed results, once for successful results
95
-
expect(mockRetryIf).toHaveBeenCalledTimes(2);
114
+
115
+
expect(mockRetryIf).toHaveBeenCalledTimes(1);
96
116
expect(mockRetryIf).toHaveBeenCalledWith(queryOneError);
97
117
98
118
jest.runAllTimers();
99
119
100
-
// max number of retries, plus original call
120
+
expect(mockRetryIf).toHaveBeenCalledTimes(mockOptions.maxNumberAttempts);
121
+
101
122
expect(response).toHaveBeenCalledTimes(mockOptions.maxNumberAttempts);
102
-
expect(result).toHaveBeenCalledTimes(mockOptions.maxNumberAttempts);
123
+
124
+
// result should only ever be called once per operation
125
+
expect(result).toHaveBeenCalledTimes(1);
126
+
127
+
next(opTwo);
128
+
129
+
jest.runAllTimers();
130
+
131
+
expect(mockRetryIf).toHaveBeenCalledWith(queryTwoError);
132
+
133
+
// max number of retries for each op
134
+
expect(response).toHaveBeenCalledTimes(mockOptions.maxNumberAttempts * 2);
135
+
expect(result).toHaveBeenCalledTimes(2);
103
136
});
104
137
105
138
it('should retry x number of times and then return the successful result', () => {
106
139
const numberRetriesBeforeSuccess = 3;
107
140
const response = jest.fn(
108
-
(forwardOp: OperationWithRetry): OperationResult => {
141
+
(forwardOp: Operation): OperationResult => {
109
142
expect(forwardOp.key).toBe(op.key);
110
143
// @ts-ignore
111
144
return {
112
145
operation: forwardOp,
113
-
...(forwardOp.retryCount! >= numberRetriesBeforeSuccess
146
+
...(forwardOp.context.retryCount! >= numberRetriesBeforeSuccess
114
147
? { data: queryOneData }
115
148
: { error: queryOneError }),
116
149
};
···
138
171
next(op);
139
172
jest.runAllTimers();
140
173
141
-
expect(mockRetryIf).toHaveBeenCalledTimes(numberRetriesBeforeSuccess * 2);
174
+
expect(mockRetryIf).toHaveBeenCalledTimes(numberRetriesBeforeSuccess);
142
175
expect(mockRetryIf).toHaveBeenCalledWith(queryOneError);
143
176
144
177
// one for original source, one for retry
145
178
expect(response).toHaveBeenCalledTimes(1 + numberRetriesBeforeSuccess);
146
-
expect(result).toHaveBeenCalledTimes(1 + numberRetriesBeforeSuccess);
179
+
expect(result).toHaveBeenCalledTimes(1);
147
180
});
148
181
149
182
it(`should still retry if retryIf undefined but there is a networkError`, () => {
···
185
218
186
219
// max number of retries, plus original call
187
220
expect(response).toHaveBeenCalledTimes(mockOptions.maxNumberAttempts);
188
-
expect(result).toHaveBeenCalledTimes(mockOptions.maxNumberAttempts);
221
+
expect(result).toHaveBeenCalledTimes(1);
189
222
});
+25
-45
exchanges/retry/src/retryExchange.ts
+25
-45
exchanges/retry/src/retryExchange.ts
···
4
4
pipe,
5
5
merge,
6
6
filter,
7
-
tap,
8
7
fromValue,
9
8
delay,
10
9
mergeMap,
···
25
24
maxNumberAttempts?: number;
26
25
/** Conditionally determine whether an error should be retried */
27
26
retryIf?: (e: CombinedError) => boolean;
28
-
}
29
-
30
-
export interface OperationWithRetry extends Operation {
31
-
retryCount?: number;
32
-
}
33
-
34
-
export interface OperationResultWithRetry extends OperationResult {
35
-
operation: OperationWithRetry;
36
27
}
37
28
38
29
export const retryExchange = ({
···
40
31
maxDelayMs,
41
32
randomDelay,
42
33
maxNumberAttempts,
43
-
retryIf,
34
+
retryIf: retryIfOption,
44
35
}: RetryExchangeOptions): Exchange => {
45
36
const MIN_DELAY = initialDelayMs || 1000;
46
37
const MAX_DELAY = maxDelayMs || 15000;
47
-
const MAX_ATTEMPTS = maxNumberAttempts || Infinity;
38
+
const MAX_ATTEMPTS = maxNumberAttempts || 2;
48
39
const RANDOM_DELAY = randomDelay || true;
49
40
50
-
const networkErrorOrRetryIf = err =>
51
-
(retryIf && retryIf(err)) || err.networkError;
41
+
const retryIf =
42
+
retryIfOption || ((err: CombinedError) => err && err.networkError);
52
43
53
44
return ({ forward }) => ops$ => {
54
45
const sharedOps$ = pipe(ops$, share);
55
46
const { source: retry$, next: nextRetryOperation } = makeSubject<
56
-
OperationWithRetry
47
+
Operation
57
48
>();
58
-
let maxNumberAttemptsExceeded = false;
59
49
60
50
const retryWithBackoff$ = pipe(
61
51
retry$,
62
-
mergeMap((op: OperationWithRetry) => {
63
-
const { key, context, retryCount } = op;
64
-
if (retryCount && retryCount > MAX_ATTEMPTS) {
65
-
maxNumberAttemptsExceeded = true;
66
-
}
52
+
mergeMap((op: Operation) => {
53
+
const { key, context } = op;
54
+
const retryCount = context.retryCount || 0;
67
55
let delayAmount = context.retryDelay || MIN_DELAY;
68
56
69
57
const backoffFactor = Math.random() + 1.5;
···
75
63
76
64
// We stop the retries if a teardown event for this operation comes in
77
65
// But if this event comes through regularly we also stop the retries, since it's
78
-
// basically the query retrying itself, so no backoff should be added!
66
+
// basically the query retrying itself, no backoff should be added!
79
67
const teardown$ = pipe(
80
68
sharedOps$,
81
69
filter(op => {
···
94
82
context: {
95
83
...op.context,
96
84
retryDelay: delayAmount,
85
+
retryCount: retryCount + 1,
97
86
},
98
-
retryCount: retryCount != null ? retryCount + 1 : 1,
99
87
}),
100
-
filter(op => op.retryCount < MAX_ATTEMPTS),
101
-
// Here's the actual delay
102
88
delay(delayAmount),
103
89
// Stop retry if a teardown comes in
104
90
takeUntil(teardown$)
···
109
95
const result$ = pipe(
110
96
merge([sharedOps$, retryWithBackoff$]),
111
97
forward,
112
-
share
113
-
) as sourceT<OperationResultWithRetry>;
114
-
115
-
const successResult$ = pipe(
116
-
result$,
117
-
// We let through all results that don't fit the criteria
118
-
filter(res => !res.error || !networkErrorOrRetryIf(res.error))
119
-
);
120
-
121
-
const failedResult$ = pipe(
122
-
result$,
123
-
// Only retry if the error passes the conditional retryIf function (if passed)
124
-
// or if the error contains a networkError
125
-
filter(res => !!(res.error && networkErrorOrRetryIf(res.error))),
126
-
// Send failed responses to be retried by calling next on the retry$ subject
127
-
// Exclude operations that have been retried more than the specified max
128
-
tap(op => {
129
-
if (!maxNumberAttemptsExceeded) {
130
-
nextRetryOperation(op.operation);
98
+
share,
99
+
filter(res => {
100
+
const maxNumberAttemptsExceeded =
101
+
(res.operation.context.retryCount || 0) >= MAX_ATTEMPTS - 1;
102
+
// Only retry if the error passes the conditional retryIf function (if passed)
103
+
// or if the error contains a networkError
104
+
if (res.error && retryIf(res.error) && !maxNumberAttemptsExceeded) {
105
+
// Send failed responses to be retried by calling next on the retry$ subject
106
+
// Exclude operations that have been retried more than the specified max
107
+
nextRetryOperation(res.operation);
108
+
return false;
109
+
} else {
110
+
return true;
131
111
}
132
112
})
133
-
);
113
+
) as sourceT<OperationResult>;
134
114
135
-
return merge([successResult$, failedResult$]);
115
+
return result$;
136
116
};
137
117
};