felangel / felangel/fresh

New requests sent with stale token while refresh is in-flight

Aperta
#141 0 commenti 0 reazioni 0 assegnatari Vedi su GitHub
Lingua principale
Dart
Stelle
428
Fork
60
Metriche di merge delle PR
Nessuna PR unita negli ultimi 30g

Descrizione

### Description

When a token refresh is in progress (triggered by a 401 response), new requests that enter the interceptor pipeline are still sent with the old (already invalidated) access token. This causes unnecessary 401 responses from the backend.

The single-flight refresh coordination from #130 correctly prevents multiple parallel `refreshToken` calls, but does not prevent new requests from being dispatched with a stale token during the refresh window.

### Problem Details

In `fresh_dio`, `QueuedInterceptor` uses **separate queues** for `onRequest`, `onResponse`, and `onError` ([Dio source](https://github.com/cfug/dio/blob/main/dio/lib/src/interceptor.dart#L383-L385)):

```dart
// From Dio's QueuedInterceptor:
final _requestQueue = _TaskQueue();
final _responseQueue = _TaskQueue();
final _errorQueue = _TaskQueue();
```

This means `onRequest` and `onError` run **in parallel**. While `onError` is awaiting a token refresh, new requests pass through `onRequest`, read the old `_token` value, and are dispatched to the backend with an already-invalidated access token.

In `fresh_http` and `fresh_graphql`, `send()` / `request()` are fully concurrent with no queueing, so the same issue applies.

### Example Scenario

```dart
final dio = Dio();
dio.interceptors.add(fresh);

// Request 1 → sent with old_token → gets 401 → triggers refresh
// While refresh is in-flight, backend has invalidated old_token

// Request 2 arrives → onRequest reads _token (still old_token) → sent → 401
// Request 2's onError detects token already refreshed → retries → 200

// Both succeed, but Request 2 caused an unnecessary 401 round-trip
final results = await Future.wait([
dio.get('http://example.com/1'), // 401 → refresh → retry → 200
dio.get('http://example.com/2'), // 401 (unnecessary) → retry → 200
]);
```

### Impact

* **Unnecessary 401 responses** from the backend for every new request during the refresh window
* **Extra network round-trips** — each affected request makes 2 calls instead of 1
* **Backend-side noise** — false "unauthorized" entries in logs, potential rate-limiting or security alerts
* **With refresh token rotation** — if the single-flight mechanism ever fails to coordinate, parallel refresh calls with an already-rotated refresh token could cause **deauthorization**

### Expected Behavior

New requests arriving while a token refresh is in-flight should **wait** for the refresh to complete and then be dispatched with the updated token, avoiding unnecessary 401 responses.

### Proposed Solution

Add a `tokenWaitingRefresh` getter to `FreshMixin` that awaits `_refreshFuture` (if present) before returning the current token. Use it in `onRequest` (`fresh_dio`), `send()` (`fresh_http`), and `request()` (`fresh_graphql`) instead of the plain `token` getter:

```dart
// In FreshMixin:
@protected
Future get tokenWaitingRefresh async {
final pending = _refreshFuture;
if (pending != null) {
try {
await pending;
} catch (_) {}
}
return token;
}
```

This is a minimal, non-breaking change that piggy-backs on the existing `_refreshFuture` from #130 without adding new dependencies or altering the refresh lifecycle.

Guida per i contributori

Nessuna guida per i contributori indicizzata per questo repository

Valutazione

Questa issue non è ancora stata valutata.

Ricevi le nuove issue nella tua casella

Un breve riepilogo di issue GitHub adatte ai principianti.