Skip to content

Commit 0bbbe17

Browse files
ZibanPirateZibanPirate
authored
Merge pull request #580 from dzcode-io/485-bug-all-github-realted-api-endpoints-fails-when-github-service-fails
bugfix: all GitHub related api endpoints fails when GitHub service fails
2 parents 41f2576 + f300806 commit 0bbbe17

File tree

6 files changed

+102
-73
lines changed

6 files changed

+102
-73
lines changed

‎api/src/contribution/repository.ts

Lines changed: 46 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { Model } from "@dzcode.io/models/dist/_base";
22
import { ContributionEntity } from "@dzcode.io/models/dist/contribution";
33
import { DataService } from "src/data/service";
44
import { GithubService } from "src/github/service";
5+
import { LoggerService } from "src/logger/service";
56
import { Service } from "typedi";
67

78
import { allFilterNames, FilterDto, GetContributionsResponseDto, OptionDto } from "./types";
@@ -11,6 +12,7 @@ export class ContributionRepository {
1112
constructor(
1213
private readonly githubService: GithubService,
1314
private readonly dataService: DataService,
15+
private readonly loggerService: LoggerService,
1416
) {}
1517

1618
public async find(
@@ -26,43 +28,51 @@ export class ContributionRepository {
2628
...repositories
2729
.filter(({ provider }) => provider === "github")
2830
.map(async ({ owner, repository }) => {
29-
const issuesIncludingPRs = await this.githubService.listRepositoryIssues({
30-
owner,
31-
repository,
32-
});
31+
try {
32+
const issuesIncludingPRs = await this.githubService.listRepositoryIssues({
33+
owner,
34+
repository,
35+
});
3336

34-
const languages = await this.githubService.listRepositoryLanguages({
35-
owner,
36-
repository,
37-
});
38-
// @TODO-ZM: filter out the ones created by bots
39-
return issuesIncludingPRs.map<Model<ContributionEntity, "project">>(
40-
({
41-
number,
42-
labels: gLabels,
43-
title,
44-
html_url, // eslint-disable-line camelcase
45-
pull_request, // eslint-disable-line camelcase
46-
created_at, // eslint-disable-line camelcase
47-
updated_at, // eslint-disable-line camelcase
48-
comments,
49-
}) => ({
50-
id: `${number}`,
51-
labels: gLabels.map(({ name }) => name),
52-
languages,
53-
project: {
54-
slug,
55-
name,
56-
},
57-
title,
58-
type: pull_request ? "pullRequest" : "issue", // eslint-disable-line camelcase
59-
url: html_url, // eslint-disable-line camelcase
60-
createdAt: created_at, // eslint-disable-line camelcase
61-
updatedAt: updated_at, // eslint-disable-line camelcase
62-
commentsCount: comments,
63-
/* eslint-enable camelcase */
64-
}),
65-
);
37+
const languages = await this.githubService.listRepositoryLanguages({
38+
owner,
39+
repository,
40+
});
41+
// @TODO-ZM: filter out the ones created by bots
42+
return issuesIncludingPRs.map<Model<ContributionEntity, "project">>(
43+
({
44+
number,
45+
labels: gLabels,
46+
title,
47+
html_url, // eslint-disable-line camelcase
48+
pull_request, // eslint-disable-line camelcase
49+
created_at, // eslint-disable-line camelcase
50+
updated_at, // eslint-disable-line camelcase
51+
comments,
52+
}) => ({
53+
id: `${number}`,
54+
labels: gLabels.map(({ name }) => name),
55+
languages,
56+
project: {
57+
slug,
58+
name,
59+
},
60+
title,
61+
type: pull_request ? "pullRequest" : "issue", // eslint-disable-line camelcase
62+
url: html_url, // eslint-disable-line camelcase
63+
createdAt: created_at, // eslint-disable-line camelcase
64+
updatedAt: updated_at, // eslint-disable-line camelcase
65+
commentsCount: comments,
66+
/* eslint-enable camelcase */
67+
}),
68+
);
69+
} catch (error) {
70+
this.loggerService.warn({
71+
message: `Failed to fetch contributions for ${owner}/${repository}: ${error}`,
72+
meta: { owner, repository },
73+
});
74+
return [];
75+
}
6676
}),
6777
],
6878
[],

‎api/src/fetch/service.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ export class FetchService {
1515
});
1616
}
1717

18+
// @TODO-ZM: using DTO, validate response and DRY the types
1819
public get = async <T = unknown>(
1920
url: string,
2021
{ params = {}, headers = {} }: FetchConfig = {},

‎api/src/github/service.ts

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -66,8 +66,6 @@ export class GithubService {
6666
},
6767
);
6868

69-
// @TODO-ZM: validate responses using DTOs, for all fetchService methods
70-
if (!Array.isArray(issues)) return [];
7169
return issues;
7270
};
7371

‎api/src/project/controller.ts

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { Controller, Get } from "routing-controllers";
22
import { OpenAPI, ResponseSchema } from "routing-controllers-openapi";
33
import { DataService } from "src/data/service";
44
import { GithubService } from "src/github/service";
5+
import { LoggerService } from "src/logger/service";
56
import { Service } from "typedi";
67

78
import { GetProjectsResponseDto } from "./types";
@@ -12,6 +13,7 @@ export class ProjectController {
1213
constructor(
1314
private readonly githubService: GithubService,
1415
private readonly dataService: DataService,
16+
private readonly loggerService: LoggerService,
1517
) {}
1618

1719
@Get("/")
@@ -31,14 +33,23 @@ export class ProjectController {
3133
...project,
3234
repositories: await Promise.all(
3335
repositories.map(async ({ provider, owner, repository }) => {
36+
let contributionCount = 0;
37+
try {
38+
contributionCount = (
39+
await this.githubService.listRepositoryIssues({ owner, repository })
40+
).length;
41+
} catch (error) {
42+
this.loggerService.warn({
43+
message: `Failed to fetch contributionCount for ${owner}/${repository}: ${error}`,
44+
meta: { owner, repository },
45+
});
46+
}
3447
return {
3548
provider,
3649
owner,
3750
repository,
3851
stats: {
39-
contributionCount: (
40-
await this.githubService.listRepositoryIssues({ owner, repository })
41-
).length,
52+
contributionCount,
4253
languages: await this.githubService.listRepositoryLanguages({
4354
owner,
4455
repository,

‎api/src/team/repository.ts

Lines changed: 38 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,15 @@ import { RepositoryEntity } from "@dzcode.io/models/dist/repository";
44
import { DataService } from "src/data/service";
55
import { GithubService } from "src/github/service";
66
import { GithubRepositoryContributor } from "src/github/types";
7+
import { LoggerService } from "src/logger/service";
78
import { Service } from "typedi";
89

910
@Service()
1011
export class TeamRepository {
1112
constructor(
1213
private readonly githubService: GithubService,
1314
private readonly dataService: DataService,
15+
private readonly loggerService: LoggerService,
1416
) {}
1517

1618
public async find(): Promise<Model<AccountEntity, "repositories">[]> {
@@ -35,35 +37,42 @@ export class TeamRepository {
3537
await Promise.all(
3638
repositories.map(async ({ provider, owner, repository }) => {
3739
if (provider === "github") {
38-
const contributors = await this.githubService.listRepositoryContributors({
39-
owner,
40-
repository,
41-
});
42-
contributors.forEach((contributor) => {
43-
const uuid = this.githubService.githubUserToAccountEntity({
44-
...contributor,
45-
name: "",
46-
}).id;
47-
const { login, contributions } = contributor;
48-
// contributor first time appearing in the list
49-
if (!contributorsUsernameRankedRecord[uuid]) {
50-
contributorsUsernameRankedRecord[uuid] = {
51-
login,
52-
contributions,
53-
repositories: [{ provider, owner, repository }],
54-
};
55-
} else {
56-
// contributor already exists in the list, and is a contributor to another repository
57-
// - so we count additional contributions:
58-
contributorsUsernameRankedRecord[uuid].contributions += contributor.contributions;
59-
// - and add the other repository to the list of repositories he contributed to
60-
contributorsUsernameRankedRecord[uuid].repositories.push({
61-
provider,
62-
owner,
63-
repository,
64-
});
65-
}
66-
});
40+
try {
41+
const contributors = await this.githubService.listRepositoryContributors({
42+
owner,
43+
repository,
44+
});
45+
contributors.forEach((contributor) => {
46+
const uuid = this.githubService.githubUserToAccountEntity({
47+
...contributor,
48+
name: "",
49+
}).id;
50+
const { login, contributions } = contributor;
51+
// contributor first time appearing in the list
52+
if (!contributorsUsernameRankedRecord[uuid]) {
53+
contributorsUsernameRankedRecord[uuid] = {
54+
login,
55+
contributions,
56+
repositories: [{ provider, owner, repository }],
57+
};
58+
} else {
59+
// contributor already exists in the list, and is a contributor to another repository
60+
// - so we count additional contributions:
61+
contributorsUsernameRankedRecord[uuid].contributions += contributor.contributions;
62+
// - and add the other repository to the list of repositories he contributed to
63+
contributorsUsernameRankedRecord[uuid].repositories.push({
64+
provider,
65+
owner,
66+
repository,
67+
});
68+
}
69+
});
70+
} catch (error) {
71+
this.loggerService.warn({
72+
message: `Failed to fetch contributors for ${owner}/${repository}: ${error}`,
73+
meta: { owner, repository },
74+
});
75+
}
6776
} else throw new Error(`Provider ${provider} is not supported yet`);
6877
}),
6978
);

‎data/models/projects/Mishkal/info.json

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,10 @@
11
{
2-
"name": "List to Tree",
2+
"name": "Mishkal",
33
"repositories": [
44
{
55
"provider": "github",
6-
"owner": "ZibanPirate",
7-
"repository": "l2t"
6+
"owner": "linuxscout",
7+
"repository": "mishkal"
88
}
99
]
1010
}

0 commit comments

Comments
 (0)