Skip to content

Commit b1295cc

Browse files
committed
1 parent 7725bf1 commit b1295cc

7 files changed

Lines changed: 156 additions & 9 deletions

File tree

src/vs/platform/userDataSync/common/userDataAutoSyncService.ts

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ import { IStorageService, StorageScope, IWorkspaceStorageChangeEvent } from 'vs/
1515
import { IEnvironmentService } from 'vs/platform/environment/common/environment';
1616
import { IUserDataSyncMachinesService } from 'vs/platform/userDataSync/common/userDataSyncMachines';
1717
import { localize } from 'vs/nls';
18+
import { toLocalISOString } from 'vs/base/common/date';
1819

1920
type AutoSyncClassification = {
2021
sources: { classification: 'SystemMetaData', purpose: 'FeatureInsight', isMeasurement: true };
@@ -101,13 +102,14 @@ export class UserDataAutoSyncService extends UserDataAutoSyncEnablementService i
101102
this.disableMachineEventually();
102103
}
103104
this._register(userDataSyncAccountService.onDidChangeAccount(() => this.updateAutoSync()));
105+
this._register(userDataSyncStoreService.onDidChangeDonotMakeRequestsUntil(() => this.updateAutoSync()));
104106
this._register(Event.debounce<string, string[]>(userDataSyncService.onDidChangeLocal, (last, source) => last ? [...last, source] : [source], 1000)(sources => this.triggerSync(sources, false)));
105107
this._register(Event.filter(this.userDataSyncResourceEnablementService.onDidChangeResourceEnablement, ([, enabled]) => enabled)(() => this.triggerSync(['resourceEnablement'], false)));
106108
}
107109
}
108110

109111
private updateAutoSync(): void {
110-
const { enabled, reason } = this.isAutoSyncEnabled();
112+
const { enabled, message } = this.isAutoSyncEnabled();
111113
if (enabled) {
112114
if (this.autoSync.value === undefined) {
113115
this.autoSync.value = new AutoSync(1000 * 60 * 5 /* 5 miutes */, this.userDataSyncStoreService, this.userDataSyncService, this.userDataSyncMachinesService, this.logService, this.storageService);
@@ -120,7 +122,9 @@ export class UserDataAutoSyncService extends UserDataAutoSyncEnablementService i
120122
} else {
121123
this.syncTriggerDelayer.cancel();
122124
if (this.autoSync.value !== undefined) {
123-
this.logService.info('Auto Sync: Disabled because', reason);
125+
if (message) {
126+
this.logService.info(message);
127+
}
124128
this.autoSync.clear();
125129
}
126130
}
@@ -129,12 +133,15 @@ export class UserDataAutoSyncService extends UserDataAutoSyncEnablementService i
129133
// For tests purpose only
130134
protected startAutoSync(): boolean { return true; }
131135

132-
private isAutoSyncEnabled(): { enabled: boolean, reason?: string } {
136+
private isAutoSyncEnabled(): { enabled: boolean, message?: string } {
133137
if (!this.isEnabled()) {
134-
return { enabled: false, reason: 'sync is disabled' };
138+
return { enabled: false, message: 'Auto Sync: Disabled.' };
135139
}
136140
if (!this.userDataSyncAccountService.account) {
137-
return { enabled: false, reason: 'token is not avaialable' };
141+
return { enabled: false, message: 'Auto Sync: Suspended until auth token is available.' };
142+
}
143+
if (this.userDataSyncStoreService.donotMakeRequestsUntil) {
144+
return { enabled: false, message: `Auto Sync: Suspended until ${toLocalISOString(this.userDataSyncStoreService.donotMakeRequestsUntil)} because server is not accepting requests until then.` };
138145
}
139146
return { enabled: true };
140147
}

src/vs/platform/userDataSync/common/userDataSync.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -164,6 +164,9 @@ export interface IUserDataSyncStoreService {
164164
readonly _serviceBrand: undefined;
165165
readonly userDataSyncStore: IUserDataSyncStore | undefined;
166166

167+
readonly onDidChangeDonotMakeRequestsUntil: Event<void>;
168+
readonly donotMakeRequestsUntil: Date | undefined;
169+
167170
readonly onTokenFailed: Event<void>;
168171
readonly onTokenSucceed: Event<void>;
169172
setAuthToken(token: string, type: string): void;
@@ -207,6 +210,7 @@ export enum UserDataSyncErrorCode {
207210
UpgradeRequired = 'UpgradeRequired', /* 426 */
208211
PreconditionRequired = 'PreconditionRequired', /* 428 */
209212
TooManyRequests = 'RemoteTooManyRequests', /* 429 */
213+
TooManyRequestsAndRetryAfter = 'TooManyRequestsAndRetryAfter', /* 429 + Retry-After */
210214

211215
// Local Errors
212216
ConnectionRefused = 'ConnectionRefused',

src/vs/platform/userDataSync/common/userDataSyncService.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -394,6 +394,7 @@ export class UserDataSyncService extends Disposable implements IUserDataSyncServ
394394
throw new UserDataSyncError(e.message, e.code, source);
395395

396396
case UserDataSyncErrorCode.TooManyRequests:
397+
case UserDataSyncErrorCode.TooManyRequestsAndRetryAfter:
397398
case UserDataSyncErrorCode.LocalTooManyRequests:
398399
case UserDataSyncErrorCode.Gone:
399400
case UserDataSyncErrorCode.UpgradeRequired:

src/vs/platform/userDataSync/common/userDataSyncStoreService.ts

Lines changed: 48 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,9 @@ import { assign } from 'vs/base/common/objects';
1919
import { generateUuid } from 'vs/base/common/uuid';
2020
import { isWeb } from 'vs/base/common/platform';
2121
import { Emitter, Event } from 'vs/base/common/event';
22+
import { createCancelablePromise, timeout, CancelablePromise } from 'vs/base/common/async';
2223

24+
const DONOT_MAKE_REQUESTS_UNTIL_KEY = 'sync.donot-make-requests-until';
2325
const USER_SESSION_ID_KEY = 'sync.user-session-id';
2426
const MACHINE_SESSION_ID_KEY = 'sync.machine-session-id';
2527
const REQUEST_SESSION_LIMIT = 100;
@@ -40,6 +42,11 @@ export class UserDataSyncStoreService extends Disposable implements IUserDataSyn
4042
private _onTokenSucceed: Emitter<void> = this._register(new Emitter<void>());
4143
readonly onTokenSucceed: Event<void> = this._onTokenSucceed.event;
4244

45+
private _donotMakeRequestsUntil: Date | undefined = undefined;
46+
get donotMakeRequestsUntil() { return this._donotMakeRequestsUntil; }
47+
private _onDidChangeDonotMakeRequestsUntil = this._register(new Emitter<void>());
48+
readonly onDidChangeDonotMakeRequestsUntil = this._onDidChangeDonotMakeRequestsUntil.event;
49+
4350
constructor(
4451
@IProductService productService: IProductService,
4552
@IConfigurationService configurationService: IConfigurationService,
@@ -66,12 +73,41 @@ export class UserDataSyncStoreService extends Disposable implements IUserDataSyn
6673

6774
/* A requests session that limits requests per sessions */
6875
this.session = new RequestsSession(REQUEST_SESSION_LIMIT, REQUEST_SESSION_INTERVAL, this.requestService, this.logService);
76+
this.initDonotMakeRequestsUntil();
6977
}
7078

7179
setAuthToken(token: string, type: string): void {
7280
this.authToken = { token, type };
7381
}
7482

83+
private initDonotMakeRequestsUntil(): void {
84+
const donotMakeRequestsUntil = this.storageService.getNumber(DONOT_MAKE_REQUESTS_UNTIL_KEY, StorageScope.GLOBAL);
85+
if (donotMakeRequestsUntil && Date.now() < donotMakeRequestsUntil) {
86+
this.setDonotMakeRequestsUntil(new Date(donotMakeRequestsUntil));
87+
}
88+
}
89+
90+
private resetDonotMakeRequestsUntilPromise: CancelablePromise<void> | undefined = undefined;
91+
private setDonotMakeRequestsUntil(donotMakeRequestsUntil: Date | undefined): void {
92+
if (this._donotMakeRequestsUntil?.getTime() !== donotMakeRequestsUntil?.getTime()) {
93+
this._donotMakeRequestsUntil = donotMakeRequestsUntil;
94+
95+
if (this.resetDonotMakeRequestsUntilPromise) {
96+
this.resetDonotMakeRequestsUntilPromise.cancel();
97+
this.resetDonotMakeRequestsUntilPromise = undefined;
98+
}
99+
100+
if (this._donotMakeRequestsUntil) {
101+
this.storageService.store(DONOT_MAKE_REQUESTS_UNTIL_KEY, this._donotMakeRequestsUntil.getTime(), StorageScope.GLOBAL);
102+
this.resetDonotMakeRequestsUntilPromise = createCancelablePromise(token => timeout(this._donotMakeRequestsUntil!.getTime() - Date.now(), token).then(() => this.setDonotMakeRequestsUntil(undefined)));
103+
} else {
104+
this.storageService.remove(DONOT_MAKE_REQUESTS_UNTIL_KEY, StorageScope.GLOBAL);
105+
}
106+
107+
this._onDidChangeDonotMakeRequestsUntil.fire();
108+
}
109+
}
110+
75111
async getAllRefs(resource: ServerResource): Promise<IResourceRefHandle[]> {
76112
if (!this.userDataSyncStore) {
77113
throw new Error('No settings sync store url configured.');
@@ -244,6 +280,11 @@ export class UserDataSyncStoreService extends Disposable implements IUserDataSyn
244280
throw new UserDataSyncStoreError('No Auth Token Available', UserDataSyncErrorCode.Unauthorized, undefined);
245281
}
246282

283+
if (this._donotMakeRequestsUntil && Date.now() < this._donotMakeRequestsUntil.getTime()) {
284+
throw new UserDataSyncStoreError(`${options.type} request '${options.url?.toString()}' failed because of too many requests (429).`, UserDataSyncErrorCode.TooManyRequestsAndRetryAfter, undefined);
285+
}
286+
this.setDonotMakeRequestsUntil(undefined);
287+
247288
const commonHeaders = await this.commonHeadersPromise;
248289
options.headers = assign(options.headers || {}, commonHeaders, {
249290
'X-Account-Type': this.authToken.type,
@@ -299,7 +340,13 @@ export class UserDataSyncStoreService extends Disposable implements IUserDataSyn
299340
}
300341

301342
if (context.res.statusCode === 429) {
302-
throw new UserDataSyncStoreError(`${options.type} request '${options.url?.toString()}' failed because of too many requests (429).`, UserDataSyncErrorCode.TooManyRequests, operationId);
343+
const retryAfter = context.res.headers['retry-after'];
344+
if (retryAfter) {
345+
this.setDonotMakeRequestsUntil(new Date(Date.now() + (parseInt(retryAfter) * 1000)));
346+
throw new UserDataSyncStoreError(`${options.type} request '${options.url?.toString()}' failed because of too many requests (429).`, UserDataSyncErrorCode.TooManyRequestsAndRetryAfter, operationId);
347+
} else {
348+
throw new UserDataSyncStoreError(`${options.type} request '${options.url?.toString()}' failed because of too many requests (429).`, UserDataSyncErrorCode.TooManyRequests, operationId);
349+
}
303350
}
304351

305352
return context;

src/vs/platform/userDataSync/test/common/userDataAutoSyncService.test.ts

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -383,4 +383,26 @@ suite('UserDataAutoSyncService', () => {
383383
assert.deepEqual((<UserDataSyncStoreError>e).code, UserDataSyncErrorCode.TooManyRequests);
384384
});
385385

386+
test('test auto sync is suspended when server donot accepts requests', async () => {
387+
const target = new UserDataSyncTestServer(5, 1);
388+
389+
// Set up and sync from the test client
390+
const testClient = disposableStore.add(new UserDataSyncClient(target));
391+
await testClient.setUp();
392+
const testObject: TestUserDataAutoSyncService = testClient.instantiationService.createInstance(TestUserDataAutoSyncService);
393+
394+
const errorPromise = Event.toPromise(testObject.onError);
395+
while (target.requests.length < 5) {
396+
await testObject.sync();
397+
}
398+
const e = await errorPromise;
399+
assert.ok(e instanceof UserDataSyncStoreError);
400+
assert.deepEqual((<UserDataSyncStoreError>e).code, UserDataSyncErrorCode.TooManyRequestsAndRetryAfter);
401+
402+
target.reset();
403+
await testObject.sync();
404+
405+
assert.deepEqual(target.requests, []);
406+
});
407+
386408
});

src/vs/platform/userDataSync/test/common/userDataSyncClient.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -154,13 +154,13 @@ export class UserDataSyncTestServer implements IRequestService {
154154
get responses(): { status: number }[] { return this._responses; }
155155
reset(): void { this._requests = []; this._responses = []; this._requestsWithAllHeaders = []; }
156156

157-
constructor(private readonly rateLimit = Number.MAX_SAFE_INTEGER) { }
157+
constructor(private readonly rateLimit = Number.MAX_SAFE_INTEGER, private readonly retryAfter?: number) { }
158158

159159
async resolveProxy(url: string): Promise<string | undefined> { return url; }
160160

161161
async request(options: IRequestOptions, token: CancellationToken): Promise<IRequestContext> {
162162
if (this._requests.length === this.rateLimit) {
163-
return this.toResponse(429);
163+
return this.toResponse(429, this.retryAfter ? { 'retry-after': `${this.retryAfter}` } : undefined);
164164
}
165165
const headers: IHeaders = {};
166166
if (options.headers) {

src/vs/platform/userDataSync/test/common/userDataSyncStoreService.test.ts

Lines changed: 67 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,13 @@ import { UserDataSyncClient, UserDataSyncTestServer } from 'vs/platform/userData
99
import { DisposableStore } from 'vs/base/common/lifecycle';
1010
import { IProductService } from 'vs/platform/product/common/productService';
1111
import { isWeb } from 'vs/base/common/platform';
12-
import { RequestsSession } from 'vs/platform/userDataSync/common/userDataSyncStoreService';
12+
import { RequestsSession, UserDataSyncStoreService } from 'vs/platform/userDataSync/common/userDataSyncStoreService';
1313
import { CancellationToken } from 'vs/base/common/cancellation';
1414
import { IRequestService } from 'vs/platform/request/common/request';
1515
import { newWriteableBufferStream } from 'vs/base/common/buffer';
1616
import { timeout } from 'vs/base/common/async';
1717
import { NullLogService } from 'vs/platform/log/common/log';
18+
import { Event } from 'vs/base/common/event';
1819

1920
suite('UserDataSyncStoreService', () => {
2021

@@ -322,6 +323,71 @@ suite('UserDataSyncStoreService', () => {
322323
assert.notEqual(target.requestsWithAllHeaders[0].headers!['X-User-Session-Id'], undefined);
323324
});
324325

326+
test('test rate limit on server with retry after', async () => {
327+
const target = new UserDataSyncTestServer(1, 1);
328+
const client = disposableStore.add(new UserDataSyncClient(target));
329+
await client.setUp();
330+
const testObject = client.instantiationService.get(IUserDataSyncStoreService);
331+
332+
await testObject.manifest();
333+
334+
const promise = Event.toPromise(testObject.onDidChangeDonotMakeRequestsUntil);
335+
try {
336+
await testObject.manifest();
337+
assert.fail('should fail');
338+
} catch (e) {
339+
assert.ok(e instanceof UserDataSyncStoreError);
340+
assert.deepEqual((<UserDataSyncStoreError>e).code, UserDataSyncErrorCode.TooManyRequestsAndRetryAfter);
341+
await promise;
342+
assert.ok(!!testObject.donotMakeRequestsUntil);
343+
}
344+
});
345+
346+
test('test donotMakeRequestsUntil is reset after retry time is finished', async () => {
347+
const client = disposableStore.add(new UserDataSyncClient(new UserDataSyncTestServer(1, 0.25)));
348+
await client.setUp();
349+
const testObject = client.instantiationService.get(IUserDataSyncStoreService);
350+
351+
await testObject.manifest();
352+
try {
353+
await testObject.manifest();
354+
} catch (e) { }
355+
356+
const promise = Event.toPromise(testObject.onDidChangeDonotMakeRequestsUntil);
357+
await timeout(300);
358+
await promise;
359+
assert.ok(!testObject.donotMakeRequestsUntil);
360+
});
361+
362+
test('test donotMakeRequestsUntil is retrieved', async () => {
363+
const client = disposableStore.add(new UserDataSyncClient(new UserDataSyncTestServer(1, 1)));
364+
await client.setUp();
365+
const testObject = client.instantiationService.get(IUserDataSyncStoreService);
366+
367+
await testObject.manifest();
368+
try {
369+
await testObject.manifest();
370+
} catch (e) { }
371+
372+
const target = client.instantiationService.createInstance(UserDataSyncStoreService);
373+
assert.equal(target.donotMakeRequestsUntil?.getTime(), testObject.donotMakeRequestsUntil?.getTime());
374+
});
375+
376+
test('test donotMakeRequestsUntil is checked and reset after retreived', async () => {
377+
const client = disposableStore.add(new UserDataSyncClient(new UserDataSyncTestServer(1, 0.25)));
378+
await client.setUp();
379+
const testObject = client.instantiationService.get(IUserDataSyncStoreService);
380+
381+
await testObject.manifest();
382+
try {
383+
await testObject.manifest();
384+
} catch (e) { }
385+
386+
await timeout(300);
387+
const target = client.instantiationService.createInstance(UserDataSyncStoreService);
388+
assert.ok(!target.donotMakeRequestsUntil);
389+
});
390+
325391
});
326392

327393
suite('UserDataSyncRequestsSession', () => {

0 commit comments

Comments
 (0)