Skip to content

Commit 18b2657

Browse files
chivorotkivsergeibbb
chivorotkiv
authored and
sergeibbb
committed
Stores auth problem flag per usecase to avoid continuous refreshing
(#4324)
1 parent 04fb197 commit 18b2657

File tree

3 files changed

+124
-43
lines changed

3 files changed

+124
-43
lines changed

‎src/plus/integrations/models/gitHostIntegration.ts‎

Lines changed: 41 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -54,10 +54,10 @@ export abstract class GitHostIntegration<
5454

5555
try {
5656
const author = await this.getProviderAccountForEmail(this._session!, repo, email, options);
57-
this.resetRequestExceptionCount();
57+
this.resetRequestExceptionCount('getAccountForEmail');
5858
return author;
5959
} catch (ex) {
60-
return this.handleProviderException<Account | undefined>(ex, scope, undefined);
60+
return this.handleProviderException<Account | undefined>('getAccountForEmail', ex, scope, undefined);
6161
}
6262
}
6363

@@ -84,10 +84,10 @@ export abstract class GitHostIntegration<
8484

8585
try {
8686
const author = await this.getProviderAccountForCommit(this._session!, repo, rev, options);
87-
this.resetRequestExceptionCount();
87+
this.resetRequestExceptionCount('getAccountForCommit');
8888
return author;
8989
} catch (ex) {
90-
return this.handleProviderException<Account | undefined>(ex, scope, undefined);
90+
return this.handleProviderException<Account | undefined>('getAccountForCommit', ex, scope, undefined);
9191
}
9292
}
9393

@@ -117,10 +117,15 @@ export abstract class GitHostIntegration<
117117
value: (async () => {
118118
try {
119119
const result = await this.getProviderDefaultBranch(this._session!, repo, options?.cancellation);
120-
this.resetRequestExceptionCount();
120+
this.resetRequestExceptionCount('getDefaultBranch');
121121
return result;
122122
} catch (ex) {
123-
return this.handleProviderException<DefaultBranch | undefined>(ex, scope, undefined);
123+
return this.handleProviderException<DefaultBranch | undefined>(
124+
'getDefaultBranch',
125+
ex,
126+
scope,
127+
undefined,
128+
);
124129
}
125130
})(),
126131
}),
@@ -160,10 +165,15 @@ export abstract class GitHostIntegration<
160165
repo,
161166
options?.cancellation,
162167
);
163-
this.resetRequestExceptionCount();
168+
this.resetRequestExceptionCount('getRepositoryMetadata');
164169
return result;
165170
} catch (ex) {
166-
return this.handleProviderException<RepositoryMetadata | undefined>(ex, scope, undefined);
171+
return this.handleProviderException<RepositoryMetadata | undefined>(
172+
'getRepositoryMetadata',
173+
ex,
174+
scope,
175+
undefined,
176+
);
167177
}
168178
})(),
169179
}),
@@ -188,10 +198,10 @@ export abstract class GitHostIntegration<
188198

189199
try {
190200
const result = await this.mergeProviderPullRequest(this._session!, pr, options);
191-
this.resetRequestExceptionCount();
201+
this.resetRequestExceptionCount('mergePullRequest');
192202
return result;
193203
} catch (ex) {
194-
return this.handleProviderException<boolean>(ex, scope, false);
204+
return this.handleProviderException<boolean>('mergePullRequest', ex, scope, false);
195205
}
196206
}
197207

@@ -224,10 +234,15 @@ export abstract class GitHostIntegration<
224234
value: (async () => {
225235
try {
226236
const result = await this.getProviderPullRequestForBranch(this._session!, repo, branch, opts);
227-
this.resetRequestExceptionCount();
237+
this.resetRequestExceptionCount('getPullRequestForBranch');
228238
return result;
229239
} catch (ex) {
230-
return this.handleProviderException<PullRequest | undefined>(ex, scope, undefined);
240+
return this.handleProviderException<PullRequest | undefined>(
241+
'getPullRequestForBranch',
242+
ex,
243+
scope,
244+
undefined,
245+
);
231246
}
232247
})(),
233248
}),
@@ -264,10 +279,15 @@ export abstract class GitHostIntegration<
264279
value: (async () => {
265280
try {
266281
const result = await this.getProviderPullRequestForCommit(this._session!, repo, rev);
267-
this.resetRequestExceptionCount();
282+
this.resetRequestExceptionCount('getPullRequestForCommit');
268283
return result;
269284
} catch (ex) {
270-
return this.handleProviderException<PullRequest | undefined>(ex, scope, undefined);
285+
return this.handleProviderException<PullRequest | undefined>(
286+
'getPullRequestForCommit',
287+
ex,
288+
scope,
289+
undefined,
290+
);
271291
}
272292
})(),
273293
}),
@@ -663,9 +683,13 @@ export abstract class GitHostIntegration<
663683
cancellation,
664684
silent,
665685
);
686+
this.resetRequestExceptionCount('searchMyPullRequests');
666687
return { value: pullRequests, duration: Date.now() - start };
667688
} catch (ex) {
668-
return this.handleProviderException(ex, scope, { error: ex, duration: Date.now() - start });
689+
return this.handleProviderException('searchMyPullRequests', ex, scope, {
690+
error: ex,
691+
duration: Date.now() - start,
692+
});
669693
}
670694
}
671695

@@ -705,10 +729,10 @@ export abstract class GitHostIntegration<
705729
repos != null ? (Array.isArray(repos) ? repos : [repos]) : undefined,
706730
cancellation,
707731
);
708-
this.resetRequestExceptionCount();
732+
this.resetRequestExceptionCount('searchPullRequests');
709733
return prs;
710734
} catch (ex) {
711-
return this.handleProviderException<PullRequest[] | undefined>(ex, scope, undefined);
735+
return this.handleProviderException<PullRequest[] | undefined>('searchPullRequests', ex, scope, undefined);
712736
}
713737
}
714738

‎src/plus/integrations/models/integration.ts‎

Lines changed: 70 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,29 @@ export type IntegrationResult<T> =
5050
| { error: Error; duration?: number; value?: never }
5151
| undefined;
5252

53+
type SyncReqUsecase = Exclude<
54+
| 'getAccountForCommit'
55+
| 'getAccountForEmail'
56+
| 'getAccountForResource'
57+
| 'getCurrentAccount'
58+
| 'getDefaultBranch'
59+
| 'getIssue'
60+
| 'getIssueOrPullRequest'
61+
| 'getIssuesForProject'
62+
| 'getProjectsForResources'
63+
| 'getPullRequest'
64+
| 'getPullRequestForBranch'
65+
| 'getPullRequestForCommit'
66+
| 'getRepositoryMetadata'
67+
| 'getResourcesForUser'
68+
| 'mergePullRequest'
69+
| 'searchMyIssues'
70+
| 'searchMyPullRequests'
71+
| 'searchPullRequests',
72+
// excluding to show explicitly that we don't want to add 'all' key occasionally
73+
'all'
74+
>;
75+
5376
export abstract class IntegrationBase<
5477
ID extends IntegrationIds = IntegrationIds,
5578
T extends ResourceDescriptor = ResourceDescriptor,
@@ -174,7 +197,7 @@ export abstract class IntegrationBase<
174197
void authProvider.deleteSession(this.authProviderDescriptor);
175198
}
176199

177-
this.resetRequestExceptionCount();
200+
this.resetRequestExceptionCount('all');
178201
this._session = null;
179202

180203
if (connected) {
@@ -207,14 +230,23 @@ export abstract class IntegrationBase<
207230
void this.ensureSession({ createIfNeeded: false });
208231
}
209232

233+
private _syncRequestsPerFailedUsecase = new Set<SyncReqUsecase>();
234+
hasSessionSyncRequests(): boolean {
235+
return this._syncRequestsPerFailedUsecase.size > 0;
236+
}
237+
requestSessionSyncForUsecase(syncReqUsecase: SyncReqUsecase): void {
238+
this._syncRequestsPerFailedUsecase.add(syncReqUsecase);
239+
}
210240
private static readonly requestExceptionLimit = 5;
211-
private static readonly syncDueToRequestExceptionLimit = 1;
212-
private syncCountDueToRequestException = 0;
213241
private requestExceptionCount = 0;
214242

215-
resetRequestExceptionCount(): void {
243+
resetRequestExceptionCount(syncReqUsecase: SyncReqUsecase | 'all'): void {
216244
this.requestExceptionCount = 0;
217-
this.syncCountDueToRequestException = 0;
245+
if (syncReqUsecase === 'all') {
246+
this._syncRequestsPerFailedUsecase.clear();
247+
} else {
248+
this._syncRequestsPerFailedUsecase.delete(syncReqUsecase);
249+
}
218250
}
219251

220252
/**
@@ -277,14 +309,19 @@ export abstract class IntegrationBase<
277309
}
278310
}
279311

280-
protected handleProviderException<T>(ex: Error, scope: LogScope | undefined, defaultValue: T): T {
312+
protected handleProviderException<T>(
313+
syncReqUsecase: SyncReqUsecase,
314+
ex: Error,
315+
scope: LogScope | undefined,
316+
defaultValue: T,
317+
): T {
281318
if (ex instanceof CancellationError) return defaultValue;
282319

283320
Logger.error(ex, scope);
284321

285322
if (ex instanceof AuthenticationError && this._session?.cloud) {
286-
if (this.syncCountDueToRequestException < IntegrationBase.syncDueToRequestExceptionLimit) {
287-
this.syncCountDueToRequestException++;
323+
if (!this.hasSessionSyncRequests()) {
324+
this.requestSessionSyncForUsecase(syncReqUsecase);
288325
this._session = {
289326
...this._session,
290327
expiresAt: new Date(Date.now() - 1),
@@ -436,10 +473,10 @@ export abstract class IntegrationBase<
436473
resources != null ? (Array.isArray(resources) ? resources : [resources]) : undefined,
437474
cancellation,
438475
);
439-
this.resetRequestExceptionCount();
476+
this.resetRequestExceptionCount('searchMyIssues');
440477
return issues;
441478
} catch (ex) {
442-
return this.handleProviderException<IssueShape[] | undefined>(ex, scope, undefined);
479+
return this.handleProviderException<IssueShape[] | undefined>('searchMyIssues', ex, scope, undefined);
443480
}
444481
}
445482

@@ -476,10 +513,15 @@ export abstract class IntegrationBase<
476513
id,
477514
options?.type,
478515
);
479-
this.resetRequestExceptionCount();
516+
this.resetRequestExceptionCount('getIssueOrPullRequest');
480517
return result;
481518
} catch (ex) {
482-
return this.handleProviderException<IssueOrPullRequest | undefined>(ex, scope, undefined);
519+
return this.handleProviderException<IssueOrPullRequest | undefined>(
520+
'getIssueOrPullRequest',
521+
ex,
522+
scope,
523+
undefined,
524+
);
483525
}
484526
})(),
485527
}),
@@ -516,10 +558,10 @@ export abstract class IntegrationBase<
516558
value: (async () => {
517559
try {
518560
const result = await this.getProviderIssue(this._session!, resource, id);
519-
this.resetRequestExceptionCount();
561+
this.resetRequestExceptionCount('getIssue');
520562
return result;
521563
} catch (ex) {
522-
return this.handleProviderException<Issue | undefined>(ex, scope, undefined);
564+
return this.handleProviderException<Issue | undefined>('getIssue', ex, scope, undefined);
523565
}
524566
})(),
525567
}),
@@ -553,10 +595,15 @@ export abstract class IntegrationBase<
553595
value: (async () => {
554596
try {
555597
const account = await this.getProviderCurrentAccount?.(this._session!, opts);
556-
this.resetRequestExceptionCount();
598+
this.resetRequestExceptionCount('getCurrentAccount');
557599
return account;
558600
} catch (ex) {
559-
return this.handleProviderException<Account | undefined>(ex, scope, undefined);
601+
return this.handleProviderException<Account | undefined>(
602+
'getCurrentAccount',
603+
ex,
604+
scope,
605+
undefined,
606+
);
560607
}
561608
})(),
562609
}),
@@ -583,10 +630,15 @@ export abstract class IntegrationBase<
583630
value: (async () => {
584631
try {
585632
const result = await this.getProviderPullRequest?.(this._session!, resource, id);
586-
this.resetRequestExceptionCount();
633+
this.resetRequestExceptionCount('getPullRequest');
587634
return result;
588635
} catch (ex) {
589-
return this.handleProviderException<PullRequest | undefined>(ex, scope, undefined);
636+
return this.handleProviderException<PullRequest | undefined>(
637+
'getPullRequest',
638+
ex,
639+
scope,
640+
undefined,
641+
);
590642
}
591643
})(),
592644
}));

‎src/plus/integrations/models/issuesIntegration.ts‎

Lines changed: 13 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -32,10 +32,10 @@ export abstract class IssuesIntegration<
3232

3333
try {
3434
const account = await this.getProviderAccountForResource(this._session!, resource);
35-
this.resetRequestExceptionCount();
35+
this.resetRequestExceptionCount('getAccountForResource');
3636
return account;
3737
} catch (ex) {
38-
return this.handleProviderException<Account | undefined>(ex, undefined, undefined);
38+
return this.handleProviderException<Account | undefined>('getAccountForResource', ex, undefined, undefined);
3939
}
4040
}
4141

@@ -55,10 +55,10 @@ export abstract class IssuesIntegration<
5555

5656
try {
5757
const resources = await this.getProviderResourcesForUser(this._session!);
58-
this.resetRequestExceptionCount();
58+
this.resetRequestExceptionCount('getResourcesForUser');
5959
return resources;
6060
} catch (ex) {
61-
return this.handleProviderException<T[] | undefined>(ex, undefined, undefined);
61+
return this.handleProviderException<T[] | undefined>('getResourcesForUser', ex, undefined, undefined);
6262
}
6363
}
6464

@@ -74,10 +74,10 @@ export abstract class IssuesIntegration<
7474

7575
try {
7676
const projects = await this.getProviderProjectsForResources(this._session!, resources);
77-
this.resetRequestExceptionCount();
77+
this.resetRequestExceptionCount('getProjectsForResources');
7878
return projects;
7979
} catch (ex) {
80-
return this.handleProviderException<T[] | undefined>(ex, undefined, undefined);
80+
return this.handleProviderException<T[] | undefined>('getProjectsForResources', ex, undefined, undefined);
8181
}
8282
}
8383

@@ -106,10 +106,15 @@ export abstract class IssuesIntegration<
106106

107107
try {
108108
const issues = await this.getProviderIssuesForProject(this._session!, project, options);
109-
this.resetRequestExceptionCount();
109+
this.resetRequestExceptionCount('getIssuesForProject');
110110
return issues;
111111
} catch (ex) {
112-
return this.handleProviderException<IssueShape[] | undefined>(ex, undefined, undefined);
112+
return this.handleProviderException<IssueShape[] | undefined>(
113+
'getIssuesForProject',
114+
ex,
115+
undefined,
116+
undefined,
117+
);
113118
}
114119
}
115120

0 commit comments

Comments
 (0)