Skip to content

Commit 2e7fc05

Browse files
committed
Refactors exception handling for integrations
- The `handleProviderException` method in `IntegrationBase` is updated to accept an options object instead of individual parameters. - The method now only returns void, requiring the calling functions to return the default value themselves. (#4324)
1 parent 6043403 commit 2e7fc05

File tree

3 files changed

+44
-66
lines changed

3 files changed

+44
-66
lines changed

src/plus/integrations/models/gitHostIntegration.ts

Lines changed: 21 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,8 @@ export abstract class GitHostIntegration<
5757
this.resetRequestExceptionCount('getAccountForEmail');
5858
return author;
5959
} catch (ex) {
60-
return this.handleProviderException<Account | undefined>('getAccountForEmail', ex, scope, undefined);
60+
this.handleProviderException('getAccountForEmail', ex, { scope: scope });
61+
return undefined;
6162
}
6263
}
6364

@@ -87,7 +88,8 @@ export abstract class GitHostIntegration<
8788
this.resetRequestExceptionCount('getAccountForCommit');
8889
return author;
8990
} catch (ex) {
90-
return this.handleProviderException<Account | undefined>('getAccountForCommit', ex, scope, undefined);
91+
this.handleProviderException('getAccountForCommit', ex, { scope: scope });
92+
return undefined;
9193
}
9294
}
9395

@@ -120,12 +122,8 @@ export abstract class GitHostIntegration<
120122
this.resetRequestExceptionCount('getDefaultBranch');
121123
return result;
122124
} catch (ex) {
123-
return this.handleProviderException<DefaultBranch | undefined>(
124-
'getDefaultBranch',
125-
ex,
126-
scope,
127-
undefined,
128-
);
125+
this.handleProviderException('getDefaultBranch', ex, { scope: scope });
126+
return undefined;
129127
}
130128
})(),
131129
}),
@@ -168,12 +166,8 @@ export abstract class GitHostIntegration<
168166
this.resetRequestExceptionCount('getRepositoryMetadata');
169167
return result;
170168
} catch (ex) {
171-
return this.handleProviderException<RepositoryMetadata | undefined>(
172-
'getRepositoryMetadata',
173-
ex,
174-
scope,
175-
undefined,
176-
);
169+
this.handleProviderException('getRepositoryMetadata', ex, { scope: scope });
170+
return undefined;
177171
}
178172
})(),
179173
}),
@@ -201,7 +195,8 @@ export abstract class GitHostIntegration<
201195
this.resetRequestExceptionCount('mergePullRequest');
202196
return result;
203197
} catch (ex) {
204-
return this.handleProviderException<boolean>('mergePullRequest', ex, scope, false);
198+
this.handleProviderException('mergePullRequest', ex, { scope: scope });
199+
return false;
205200
}
206201
}
207202

@@ -237,12 +232,8 @@ export abstract class GitHostIntegration<
237232
this.resetRequestExceptionCount('getPullRequestForBranch');
238233
return result;
239234
} catch (ex) {
240-
return this.handleProviderException<PullRequest | undefined>(
241-
'getPullRequestForBranch',
242-
ex,
243-
scope,
244-
undefined,
245-
);
235+
this.handleProviderException('getPullRequestForBranch', ex, { scope: scope });
236+
return undefined;
246237
}
247238
})(),
248239
}),
@@ -282,12 +273,8 @@ export abstract class GitHostIntegration<
282273
this.resetRequestExceptionCount('getPullRequestForCommit');
283274
return result;
284275
} catch (ex) {
285-
return this.handleProviderException<PullRequest | undefined>(
286-
'getPullRequestForCommit',
287-
ex,
288-
scope,
289-
undefined,
290-
);
276+
this.handleProviderException('getPullRequestForCommit', ex, { scope: scope });
277+
return undefined;
291278
}
292279
})(),
293280
}),
@@ -686,10 +673,13 @@ export abstract class GitHostIntegration<
686673
this.resetRequestExceptionCount('searchMyPullRequests');
687674
return { value: pullRequests, duration: Date.now() - start };
688675
} catch (ex) {
689-
return this.handleProviderException('searchMyPullRequests', ex, scope, {
676+
this.handleProviderException('searchMyPullRequests', ex, {
677+
scope: scope,
678+
});
679+
return {
690680
error: ex,
691681
duration: Date.now() - start,
692-
});
682+
};
693683
}
694684
}
695685

@@ -732,7 +722,8 @@ export abstract class GitHostIntegration<
732722
this.resetRequestExceptionCount('searchPullRequests');
733723
return prs;
734724
} catch (ex) {
735-
return this.handleProviderException<PullRequest[] | undefined>('searchPullRequests', ex, scope, undefined);
725+
this.handleProviderException('searchPullRequests', ex, { scope: scope });
726+
return undefined;
736727
}
737728
}
738729

src/plus/integrations/models/integration.ts

Lines changed: 15 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -309,15 +309,14 @@ export abstract class IntegrationBase<
309309
}
310310
}
311311

312-
protected handleProviderException<T>(
312+
protected handleProviderException(
313313
syncReqUsecase: SyncReqUsecase,
314314
ex: Error,
315-
scope: LogScope | undefined,
316-
defaultValue: T,
317-
): T {
318-
if (ex instanceof CancellationError) return defaultValue;
315+
options?: { scope?: LogScope | undefined },
316+
): void {
317+
if (ex instanceof CancellationError) return;
319318

320-
Logger.error(ex, scope);
319+
Logger.error(ex, options?.scope);
321320

322321
if (ex instanceof AuthenticationError && this._session?.cloud) {
323322
if (!this.hasSessionSyncRequests()) {
@@ -332,7 +331,6 @@ export abstract class IntegrationBase<
332331
} else if (ex instanceof AuthenticationError || ex instanceof RequestClientError) {
333332
this.trackRequestException();
334333
}
335-
return defaultValue;
336334
}
337335

338336
private missingExpirityReported = false;
@@ -480,7 +478,8 @@ export abstract class IntegrationBase<
480478
this.resetRequestExceptionCount('searchMyIssues');
481479
return issues;
482480
} catch (ex) {
483-
return this.handleProviderException<IssueShape[] | undefined>('searchMyIssues', ex, scope, undefined);
481+
this.handleProviderException('searchMyIssues', ex, { scope: scope });
482+
return undefined;
484483
}
485484
}
486485

@@ -520,12 +519,8 @@ export abstract class IntegrationBase<
520519
this.resetRequestExceptionCount('getIssueOrPullRequest');
521520
return result;
522521
} catch (ex) {
523-
return this.handleProviderException<IssueOrPullRequest | undefined>(
524-
'getIssueOrPullRequest',
525-
ex,
526-
scope,
527-
undefined,
528-
);
522+
this.handleProviderException('getIssueOrPullRequest', ex, { scope: scope });
523+
return undefined;
529524
}
530525
})(),
531526
}),
@@ -565,7 +560,8 @@ export abstract class IntegrationBase<
565560
this.resetRequestExceptionCount('getIssue');
566561
return result;
567562
} catch (ex) {
568-
return this.handleProviderException<Issue | undefined>('getIssue', ex, scope, undefined);
563+
this.handleProviderException('getIssue', ex, { scope: scope });
564+
return undefined;
569565
}
570566
})(),
571567
}),
@@ -602,12 +598,8 @@ export abstract class IntegrationBase<
602598
this.resetRequestExceptionCount('getCurrentAccount');
603599
return account;
604600
} catch (ex) {
605-
return this.handleProviderException<Account | undefined>(
606-
'getCurrentAccount',
607-
ex,
608-
scope,
609-
undefined,
610-
);
601+
this.handleProviderException('getCurrentAccount', ex, { scope: scope });
602+
return undefined;
611603
}
612604
})(),
613605
}),
@@ -637,12 +629,8 @@ export abstract class IntegrationBase<
637629
this.resetRequestExceptionCount('getPullRequest');
638630
return result;
639631
} catch (ex) {
640-
return this.handleProviderException<PullRequest | undefined>(
641-
'getPullRequest',
642-
ex,
643-
scope,
644-
undefined,
645-
);
632+
this.handleProviderException('getPullRequest', ex, { scope: scope });
633+
return undefined;
646634
}
647635
})(),
648636
}));

src/plus/integrations/models/issuesIntegration.ts

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,8 @@ export abstract class IssuesIntegration<
3535
this.resetRequestExceptionCount('getAccountForResource');
3636
return account;
3737
} catch (ex) {
38-
return this.handleProviderException<Account | undefined>('getAccountForResource', ex, undefined, undefined);
38+
this.handleProviderException('getAccountForResource', ex);
39+
return undefined;
3940
}
4041
}
4142

@@ -58,7 +59,8 @@ export abstract class IssuesIntegration<
5859
this.resetRequestExceptionCount('getResourcesForUser');
5960
return resources;
6061
} catch (ex) {
61-
return this.handleProviderException<T[] | undefined>('getResourcesForUser', ex, undefined, undefined);
62+
this.handleProviderException('getResourcesForUser', ex);
63+
return undefined;
6264
}
6365
}
6466

@@ -77,7 +79,8 @@ export abstract class IssuesIntegration<
7779
this.resetRequestExceptionCount('getProjectsForResources');
7880
return projects;
7981
} catch (ex) {
80-
return this.handleProviderException<T[] | undefined>('getProjectsForResources', ex, undefined, undefined);
82+
this.handleProviderException('getProjectsForResources', ex);
83+
return undefined;
8184
}
8285
}
8386

@@ -109,12 +112,8 @@ export abstract class IssuesIntegration<
109112
this.resetRequestExceptionCount('getIssuesForProject');
110113
return issues;
111114
} catch (ex) {
112-
return this.handleProviderException<IssueShape[] | undefined>(
113-
'getIssuesForProject',
114-
ex,
115-
undefined,
116-
undefined,
117-
);
115+
this.handleProviderException('getIssuesForProject', ex);
116+
return undefined;
118117
}
119118
}
120119

0 commit comments

Comments
 (0)