fix(retry): Reset delay/count state if an operation succeeds (#3229)

+5

.changeset/curvy-poems-compare.md

··· 1 + --- 2 + '@urql/exchange-retry': minor 3 + --- 4 + 5 + Reset `retryExchange`’s previous attempts and delay if an operation succeeds. This prevents the exchange from keeping its old retry count and delay if the operation delivered a result in the meantime. This is important for it to help recover from failing subscriptions.

+75 -3

exchanges/retry/src/retryExchange.test.ts

··· 1 - import { pipe, map, makeSubject, publish, tap } from 'wonka'; 1 + import { 2 + Source, 3 + pipe, 4 + map, 5 + makeSubject, 6 + mergeMap, 7 + fromValue, 8 + fromArray, 9 + publish, 10 + tap, 11 + } from 'wonka'; 2 12 import { vi, expect, it, beforeEach, afterEach } from 'vitest'; 3 13 4 14 import { ··· 67 77 ({ source: ops$, next } = makeSubject<Operation>()); 68 78 }); 69 79 70 - it(`retries if it hits an error and works for multiple concurrent operations`, () => { 80 + it('retries if it hits an error and works for multiple concurrent operations', () => { 71 81 const queryTwo = gql` 72 82 { 73 83 films { ··· 149 159 // @ts-ignore 150 160 return { 151 161 operation: forwardOp, 152 - ...(forwardOp.context.retryCount! >= numberRetriesBeforeSuccess 162 + ...(forwardOp.context.retry?.count >= numberRetriesBeforeSuccess 153 163 ? { data: queryOneData } 154 164 : { error: queryOneError }), 155 165 }; ··· 184 194 // one for original source, one for retry 185 195 expect(response).toHaveBeenCalledTimes(1 + numberRetriesBeforeSuccess); 186 196 expect(result).toHaveBeenCalledTimes(1); 197 + }); 198 + 199 + it('should reset the retry counter if an operation succeeded first', () => { 200 + let call = 0; 201 + const response = vi.fn((forwardOp: Operation): Source<any> => { 202 + expect(forwardOp.key).toBe(op.key); 203 + if (call === 0) { 204 + call++; 205 + return fromValue({ 206 + operation: forwardOp, 207 + error: queryOneError, 208 + } as any); 209 + } else if (call === 1) { 210 + call++; 211 + return fromArray([ 212 + { 213 + operation: forwardOp, 214 + data: queryOneData, 215 + } as any, 216 + { 217 + operation: forwardOp, 218 + error: queryOneError, 219 + } as any, 220 + ]); 221 + } else { 222 + expect(forwardOp.context.retry).toEqual({ count: 1, delay: null }); 223 + 224 + return fromValue({ 225 + operation: forwardOp, 226 + data: queryOneData, 227 + } as any); 228 + } 229 + }); 230 + 231 + const result = vi.fn(); 232 + const forward: ExchangeIO = ops$ => { 233 + return pipe(ops$, mergeMap(response)); 234 + }; 235 + 236 + const mockRetryIf = vi.fn(() => true); 237 + 238 + pipe( 239 + retryExchange({ 240 + ...mockOptions, 241 + retryIf: mockRetryIf, 242 + })({ 243 + forward, 244 + client, 245 + dispatchDebug, 246 + })(ops$), 247 + tap(result), 248 + publish 249 + ); 250 + 251 + next(op); 252 + vi.runAllTimers(); 253 + 254 + expect(mockRetryIf).toHaveBeenCalledTimes(2); 255 + expect(mockRetryIf).toHaveBeenCalledWith(queryOneError as any, op); 256 + 257 + expect(response).toHaveBeenCalledTimes(3); 258 + expect(result).toHaveBeenCalledTimes(2); 187 259 }); 188 260 189 261 it(`should still retry if retryIf undefined but there is a networkError`, () => {

+25 -12

exchanges/retry/src/retryExchange.ts

··· 78 78 ): Operation | null | undefined; 79 79 } 80 80 81 + interface RetryState { 82 + count: number; 83 + delay: number | null; 84 + } 85 + 81 86 /** Exchange factory that retries failed operations. 82 87 * 83 88 * @param options - A {@link RetriesExchangeOptions} configuration object. ··· 117 122 118 123 const retryWithBackoff$ = pipe( 119 124 retry$, 120 - mergeMap((op: Operation) => { 121 - const { key, context } = op; 122 - const retryCount = (context.retryCount || 0) + 1; 123 - let delayAmount = context.retryDelay || MIN_DELAY; 125 + mergeMap((operation: Operation) => { 126 + const retry: RetryState = operation.context.retry || { 127 + count: 0, 128 + delay: null, 129 + }; 130 + 131 + const retryCount = ++retry.count; 132 + let delayAmount = retry.delay || MIN_DELAY; 124 133 125 134 const backoffFactor = Math.random() + 1.5; 126 135 // if randomDelay is enabled and it won't exceed the max delay, apply a random ··· 137 146 filter(op => { 138 147 return ( 139 148 (op.kind === 'query' || op.kind === 'teardown') && 140 - op.key === key 149 + op.key === operation.key 141 150 ); 142 151 }) 143 152 ); ··· 145 154 dispatchDebug({ 146 155 type: 'retryAttempt', 147 156 message: `The operation has failed and a retry has been triggered (${retryCount} / ${MAX_ATTEMPTS})`, 148 - operation: op, 157 + operation, 149 158 data: { 150 159 retryCount, 151 160 }, ··· 154 163 // Add new retryDelay and retryCount to operation 155 164 return pipe( 156 165 fromValue( 157 - makeOperation(op.kind, op, { 158 - ...op.context, 159 - retryDelay: delayAmount, 160 - retryCount, 166 + makeOperation(operation.kind, operation, { 167 + ...operation.context, 168 + retry, 161 169 }) 162 170 ), 163 171 debounce(() => delayAmount), ··· 171 179 merge([operations$, retryWithBackoff$]), 172 180 forward, 173 181 filter(res => { 182 + const retry = res.operation.context.retry as RetryState | undefined; 174 183 // Only retry if the error passes the conditional retryIf function (if passed) 175 184 // or if the error contains a networkError 176 185 if ( ··· 179 188 ? !retryIf(res.error, res.operation) 180 189 : !retryWith && !res.error.networkError) 181 190 ) { 191 + // Reset the delay state for a successful operation 192 + if (retry) { 193 + retry.count = 0; 194 + retry.delay = null; 195 + } 182 196 return true; 183 197 } 184 198 185 199 const maxNumberAttemptsExceeded = 186 - (res.operation.context.retryCount || 0) >= MAX_ATTEMPTS - 1; 187 - 200 + ((retry && retry.count) || 0) >= MAX_ATTEMPTS - 1; 188 201 if (!maxNumberAttemptsExceeded) { 189 202 const operation = retryWith 190 203 ? retryWith(res.error, res.operation)

Configure Feed

Configure Feed