forked from akai/readest
fix(sync): retry thrown transport errors in Google Drive sync (#4827)
Google Drive library sync failed on Android: after the first few requests every files.list threw `error sending request for url (...)` and the sync stuck at "Syncing 0 / N". The provider's backoff only retried 429/5xx responses; a thrown fetch propagated immediately. On mobile a long multi-request sync hits transient transport failures (a pooled keep-alive connection to googleapis.com going bad), so without a retry every request after the first batch failed. - withBackoff now retries a thrown fetch with the same bounded exponential backoff as 429/5xx, letting reqwest re-establish a fresh connection. - mapDriveError classifies a thrown transport error (TypeError, or the Tauri HTTP plugin's plain "error sending request" Error) as NETWORK instead of UNKNOWN, so the engine's head-probe short-circuit treats it as transient. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This commit is contained in:
+31
@@ -140,6 +140,37 @@ describe('GoogleDriveProvider — Drive transport', () => {
|
||||
expect(h.fetchMock).toHaveBeenCalledTimes(4);
|
||||
});
|
||||
|
||||
test('retries a thrown transport error (mobile connection-reuse) and then succeeds', async () => {
|
||||
const h = makeDrive();
|
||||
h.fetchMock
|
||||
.mockResolvedValueOnce(json({ files: [folder('RID')] })) // findChild('Readest') ok
|
||||
// childrenQuery throws like the Tauri HTTP plugin does on Android when a
|
||||
// pooled connection goes bad: a plain Error (not a TypeError).
|
||||
.mockRejectedValueOnce(
|
||||
new Error('error sending request for url (https://www.googleapis.com/...)'),
|
||||
)
|
||||
.mockResolvedValueOnce(json({ files: [{ id: 'XID', name: 'x.json' }] })); // retry opens a fresh connection
|
||||
const entries = await h.provider.list('/Readest');
|
||||
expect(entries.map((e) => e.name)).toEqual(['x.json']);
|
||||
expect(h.sleep).toHaveBeenCalledTimes(1);
|
||||
expect(h.fetchMock).toHaveBeenCalledTimes(3);
|
||||
});
|
||||
|
||||
test('maps an exhausted thrown transport error to a NETWORK FileSyncError', async () => {
|
||||
const h = makeDrive();
|
||||
// Every attempt throws the reqwest transport error; after the bounded
|
||||
// retries it must surface as NETWORK (not UNKNOWN) so the engine treats it
|
||||
// as transient.
|
||||
h.fetchMock.mockRejectedValue(
|
||||
new Error('error sending request for url (https://www.googleapis.com/...)'),
|
||||
);
|
||||
const err = await h.provider.list('/Readest').catch((e: unknown) => e);
|
||||
expect(err).toBeInstanceOf(FileSyncError);
|
||||
expect((err as FileSyncError).code).toBe('NETWORK');
|
||||
// 1 initial attempt + MAX_BACKOFF_RETRIES (4) = 5 calls on the first segment.
|
||||
expect(h.fetchMock).toHaveBeenCalledTimes(5);
|
||||
});
|
||||
|
||||
test('classifies a 403 rate-limit as NETWORK and a 403 permission error as AUTH_FAILED', async () => {
|
||||
const rate = makeDrive();
|
||||
rate.fetchMock.mockResolvedValueOnce(
|
||||
|
||||
@@ -200,12 +200,17 @@ const mapDriveError = (e: unknown): FileSyncError => {
|
||||
}
|
||||
return new FileSyncError(e.message, code, e.status);
|
||||
}
|
||||
// A thrown fetch (offline / DNS / reset) surfaces as a TypeError.
|
||||
const networkLike = e instanceof TypeError;
|
||||
return new FileSyncError(
|
||||
e instanceof Error ? e.message : String(e),
|
||||
networkLike ? 'NETWORK' : 'UNKNOWN',
|
||||
);
|
||||
// A thrown fetch is a transport failure: a TypeError on the web `fetch`
|
||||
// (offline / DNS / reset), or a plain Error from the Tauri HTTP plugin whose
|
||||
// reqwest message reads `error sending request for url (...)` (seen on Android
|
||||
// when a pooled connection to googleapis.com goes bad mid-sync). Classify both
|
||||
// as NETWORK so the engine's head-probe short-circuit and retry logic treat
|
||||
// them as transient rather than a hard UNKNOWN failure.
|
||||
const message = e instanceof Error ? e.message : String(e);
|
||||
const networkLike =
|
||||
e instanceof TypeError ||
|
||||
/sending request|network|connection|timed out|timeout|dns/i.test(message);
|
||||
return new FileSyncError(message, networkLike ? 'NETWORK' : 'UNKNOWN');
|
||||
};
|
||||
|
||||
/** Run a provider operation, mapping any Drive failure to a {@link FileSyncError}. */
|
||||
@@ -694,15 +699,29 @@ class DriveProviderImpl {
|
||||
}
|
||||
|
||||
/**
|
||||
* Retry `fn` on a 429 / 5xx response with `Retry-After`-aware exponential
|
||||
* backoff. A first full-sync of a large library at concurrency 4 multiplies
|
||||
* Drive calls (per-segment resolution + 2-write create-then-name), so a 429 is
|
||||
* expected, not exceptional. A thrown fetch is *not* retried here — it
|
||||
* propagates and is mapped to NETWORK by {@link mapDriveError}.
|
||||
* Retry `fn` on a 429 / 5xx response — OR a thrown transport error — with
|
||||
* `Retry-After`-aware exponential backoff. A first full-sync of a large library
|
||||
* at concurrency 4 multiplies Drive calls (per-segment resolution + 2-write
|
||||
* create-then-name), so a 429 is expected, not exceptional.
|
||||
*
|
||||
* Thrown fetches are retried too: on mobile (observed on Android) a long
|
||||
* multi-request sync hits transient transport failures — reqwest's
|
||||
* `error sending request for url (...)` when a pooled keep-alive connection to
|
||||
* googleapis.com goes bad — and the retry lets it re-establish a fresh
|
||||
* connection. Without this every request after the first batch failed. The
|
||||
* final attempt's error propagates (mapped to NETWORK by {@link mapDriveError}).
|
||||
*/
|
||||
private async withBackoff(fn: () => Promise<Response>): Promise<Response> {
|
||||
for (let attempt = 0; ; attempt++) {
|
||||
const res = await fn();
|
||||
let res: Response;
|
||||
try {
|
||||
res = await fn();
|
||||
} catch (e) {
|
||||
// Out of retries: let the transport error propagate to mapDriveError.
|
||||
if (attempt >= MAX_BACKOFF_RETRIES) throw e;
|
||||
await this.sleep(BASE_BACKOFF_MS * 2 ** attempt);
|
||||
continue;
|
||||
}
|
||||
const transient =
|
||||
res.status === HTTP_TOO_MANY_REQUESTS || res.status >= HTTP_SERVER_ERROR_FLOOR;
|
||||
if (!transient || attempt >= MAX_BACKOFF_RETRIES) return res;
|
||||
|
||||
Reference in New Issue
Block a user