Skip to content

Commit

Permalink
Fix possible undefined error thrown when loading pr
Browse files Browse the repository at this point in the history
- if pr is merged then mergeable state is unknown
  • Loading branch information
mtsgrd committed Nov 15, 2024
1 parent 01186a2 commit 7abc56f
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 19 deletions.
39 changes: 20 additions & 19 deletions apps/desktop/src/lib/forge/github/githubPrMonitor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,8 +47,7 @@ export class GitHubPrMonitor implements ForgePrMonitor {
this.error.set(undefined);
this.loading.set(true);
try {
this.pr.set(await this.getPrWithRetries(this.prNumber));
this.lastFetch.set(new Date());
await this.loadPrWithRetries(this.prNumber);
} catch (err: any) {
this.error.set(err);
console.error(err);
Expand All @@ -58,34 +57,36 @@ export class GitHubPrMonitor implements ForgePrMonitor {
}

/**
* Get the PR information.
* Loads pull request details.
*
* Right after pushing GitHub will respond with status 422, necessatating retries.
* After that, the mergeable state is not always available immediately.
* The function will set the initial PR information immediately and then poll for the mergeable state.
* Right after pushing GitHub will respond with status 422, necessatating
* retries. After that, it can take a few seconds for the mergeable state
* to be known.
*/
private async getPrWithRetries(prNumber: number): Promise<DetailedPullRequest> {
private async loadPrWithRetries(prNumber: number): Promise<void> {
const request = async () => await this.prService.get(prNumber);
let lastError: any;
let attempt = 0;
while (attempt < MAX_POLL_ATTEMPTS) {
attempt++;
while (attempt++ < MAX_POLL_ATTEMPTS) {
try {
const detailedPR = await request();
// Set the PR info immediately to show
// the user all preliminary information.
this.pr.set(detailedPR);
if (detailedPR.mergeableState === 'unknown') {
await sleep(1000);
continue;
const pr = await request();
this.pr.set(pr);
this.lastFetch.set(new Date());

// Stop polling polling if merged or mergeable state known.
if (pr.mergeableState !== 'unknown' || pr.merged) {
return;
}
return detailedPR;
} catch (err: any) {
if (err.status !== 422) throw err;
lastError = err;
await sleep(1000);
}
await sleep(1000);
}
if (lastError) {
throw lastError;
}
throw lastError;
// The end of the function is reached if the pull request has an
// unknown mergeable state after retries.
}
}
1 change: 1 addition & 0 deletions apps/desktop/src/lib/forge/github/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ export function parseGitHubDetailedPullRequest(
createdAt: new Date(data.created_at),
mergedAt: data.merged_at ? new Date(data.merged_at) : undefined,
closedAt: data.closed_at ? new Date(data.closed_at) : undefined,
merged: data.merged,
mergeable: !!data.mergeable,
mergeableState: data.mergeable_state,
rebaseable: !!data.rebaseable,
Expand Down
1 change: 1 addition & 0 deletions apps/desktop/src/lib/forge/interface/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ export interface DetailedPullRequest {
mergedAt?: Date;
closedAt?: Date;
htmlUrl: string;
merged: boolean;
mergeable: boolean;
mergeableState: string;
rebaseable: boolean;
Expand Down

0 comments on commit 7abc56f

Please sign in to comment.