Skip to content

Commit

Permalink
ARC-2759 skip the error "Commit not found by sha..." if on last try (#…
Browse files Browse the repository at this point in the history 
…2618)
  • Loading branch information
@gxueatlassian
gxueatlassian authored Dec 18, 2023
1 parent 38acece commit 223264a
Show file tree
Hide file tree
Showing 7 changed files with 205 additions and 77 deletions.
3 changes: 2 additions & 1 deletion src/config/feature-flags.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,8 @@ export enum BooleanFlags {
USE_CUSTOM_ROOT_CA_BUNDLE = "use-custom-root-ca-bundle",
GENERATE_CORE_HEAP_DUMPS_ON_LOW_MEM = "generate-core-heap-dumps-on-low-mem",
USE_RATELIMIT_ON_JIRA_CLIENT = "use-ratelimit-on-jira-client",
SKIP_PROCESS_QUEUE_IF_ISSUE_NOT_FOUND = "skip-process-queue-when-issue-not-exists"
SKIP_PROCESS_QUEUE_IF_ISSUE_NOT_FOUND = "skip-process-queue-when-issue-not-exists",
SKIP_COMMIT_IF_SHA_NOT_FOUND_ON_LAST_TRY = "skip-commit-if-sha-not-found-on-last-try"
}

export enum StringFlags {
Expand Down
7 changes: 7 additions & 0 deletions src/github/client/github-client-errors.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,13 @@ export class GithubClientNotFoundError extends GithubClientError {
}
}

export class GithubClientCommitNotFoundBySHAError extends GithubClientError {
constructor(cause: AxiosError) {
super("Commit not found by sha", cause);
this.uiErrorCode = "RESOURCE_NOT_FOUND";
}
}


/**
* Type for errors section in GraphQL response
Expand Down
20 changes: 19 additions & 1 deletion src/github/client/github-client-interceptors.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@ import { getLogger } from "config/logger";
import {
GithubClientBlockedIpError,
GithubClientInvalidPermissionsError,
GithubClientNotFoundError, GithubClientSSOLoginError
GithubClientNotFoundError,
GithubClientSSOLoginError,
GithubClientCommitNotFoundBySHAError
} from "~/src/github/client/github-client-errors";

describe("github-client-interceptors", () => {
Expand Down Expand Up @@ -66,4 +68,20 @@ describe("github-client-interceptors", () => {
}
expect(error!).toBeInstanceOf(GithubClientSSOLoginError);
});

it("correctly maps commits not found by sha error", async () => {
gheNock.get("/").reply(422, {
"message": "No commit found for SHA: whatever the sha is",
"documentation_url": "https://docs.github.com"
});

let error: Error;
const client = await createAnonymousClient(gheUrl, jiraHost, { trigger: "test" }, getLogger("test"));
try {
await client.getPage(1000);
} catch (err: unknown) {
error = err as Error;
}
expect(error!).toBeInstanceOf(GithubClientCommitNotFoundBySHAError);
});
});
14 changes: 13 additions & 1 deletion src/github/client/github-client-interceptors.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,8 @@ import {
GithubClientInvalidPermissionsError,
GithubClientRateLimitingError,
GithubClientNotFoundError,
GithubClientSSOLoginError
GithubClientSSOLoginError,
GithubClientCommitNotFoundBySHAError
} from "./github-client-errors";
import Logger from "bunyan";
import { statsd } from "config/statsd";
Expand Down Expand Up @@ -166,6 +167,17 @@ export const handleFailedRequest = (rootLogger: Logger) =>
return Promise.reject(mappedError);
}

if (status === 422) {
if ((String(err.response?.data?.message) || "").toLocaleLowerCase().includes("no commit found for sha")) {
const mappedError = new GithubClientCommitNotFoundBySHAError(err);
logger.warn({
err: mappedError,
remote: response.data.message
}, "commit not found by sha");
return Promise.reject(mappedError);
}
}

const isWarning = status && (status >= 300 && status < 500 && status !== 400);

if (isWarning) {
Expand Down
2 changes: 1 addition & 1 deletion src/sqs/push.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,5 +30,5 @@ export const pushQueueMessageHandler: MessageHandler<PushQueueMessagePayload> =
subTrigger: "push"
};
const gitHubInstallationClient = await createInstallationClient(installationId, jiraHost, metrics, context.log, payload.gitHubAppConfig?.gitHubAppId);
await processPush(gitHubInstallationClient, payload, context.log);
await processPush(gitHubInstallationClient, context, payload, context.log);
};
222 changes: 152 additions & 70 deletions src/transforms/push.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,29 @@ import { GitHubCommit, GitHubRepository } from "interfaces/github";
import { shouldSendAll, booleanFlag, BooleanFlags, numberFlag, NumberFlags } from "config/feature-flags";
import { getLogger } from "config/logger";
import { GitHubInstallationClient } from "../github/client/github-installation-client";
import { GithubClientCommitNotFoundBySHAError } from "../github/client/github-client-errors";
import { DatabaseStateCreator, CreatorResult } from "test/utils/database-state-creator";

jest.mock("../sqs/queues");
jest.mock("config/feature-flags");
const logger = getLogger("test");

describe("Enqueue push", () => {

let db: CreatorResult;
let issueKey: string;
let sha: string;
beforeEach(async () => {
db = await new DatabaseStateCreator()
.forCloud()
.create();
when(shouldSendAll).calledWith("commits", expect.anything(), expect.anything()).mockResolvedValue(false);
when(numberFlag).calledWith(NumberFlags.SKIP_PROCESS_QUEUE_IF_ISSUE_NOT_FOUND_TIMEOUT, expect.anything(), expect.anything())
.mockResolvedValue(10000);
issueKey = `KEY-${new Date().getTime()}`;
sha = `sha-${issueKey}`;
});

it("should push GitHubAppConfig to payload", async () => {
await enqueuePush({
installation: { id: 123, node_id: 456 },
Expand Down Expand Up @@ -105,21 +121,72 @@ describe("Enqueue push", () => {
}), 0, expect.anything());
});

describe("Skipping msg when issue not exist", () => {
describe("Maybe skipp commit when ff is on and commimt sha on not found", () => {
it("when ff is OFF, should throw error for the problematic commits if 422 and even on last try", async () => {

when(booleanFlag).calledWith(BooleanFlags.SKIP_COMMIT_IF_SHA_NOT_FOUND_ON_LAST_TRY, expect.anything())
.mockResolvedValue(false);

githubUserTokenNock(db.subscription.gitHubInstallationId);
githubUserTokenNock(db.subscription.gitHubInstallationId);
mockGitHubCommitRestApi();
mockGitHubCommitRestApi422("invalid-sha");

const pushPayload = getPushPayload();
pushPayload.shas.push({
id: "invalid-sha",
issueKeys: [ issueKey ]
});

await expect(async () => {
await processPush(getGitHubClient(), getContext({ lastAttempt: true }), pushPayload, logger);
}).rejects.toThrow(GithubClientCommitNotFoundBySHAError);
});

it("should throw error for the problematic commits if 422 but NOT on last try", async () => {

when(booleanFlag).calledWith(BooleanFlags.SKIP_COMMIT_IF_SHA_NOT_FOUND_ON_LAST_TRY, expect.anything())
.mockResolvedValue(true);

githubUserTokenNock(db.subscription.gitHubInstallationId);
githubUserTokenNock(db.subscription.gitHubInstallationId);
mockGitHubCommitRestApi();
mockGitHubCommitRestApi422("invalid-sha");

const pushPayload = getPushPayload();
pushPayload.shas.push({
id: "invalid-sha",
issueKeys: [ issueKey ]
});

await expect(async () => {
await processPush(getGitHubClient(), getContext({ lastAttempt: false }), pushPayload, logger);
}).rejects.toThrow(GithubClientCommitNotFoundBySHAError);
});

let db: CreatorResult;
let issueKey;
let sha;
beforeEach(async () => {
db = await new DatabaseStateCreator()
.forCloud()
.create();
when(shouldSendAll).calledWith("commits", expect.anything(), expect.anything()).mockResolvedValue(false);
when(numberFlag).calledWith(NumberFlags.SKIP_PROCESS_QUEUE_IF_ISSUE_NOT_FOUND_TIMEOUT, expect.anything(), expect.anything())
.mockResolvedValue(10000);
issueKey = `KEY-${new Date().getTime()}`;
sha = `sha-${issueKey}`;
it("should success skip the problematic commits if 422 on last try", async () => {

when(booleanFlag).calledWith(BooleanFlags.SKIP_COMMIT_IF_SHA_NOT_FOUND_ON_LAST_TRY, expect.anything())
.mockResolvedValue(true);

githubUserTokenNock(db.subscription.gitHubInstallationId);
githubUserTokenNock(db.subscription.gitHubInstallationId);
mockGitHubCommitRestApi();
mockGitHubCommitRestApi422("invalid-sha");

mockJiraDevInfoAcceptUpdate();

const pushPayload = getPushPayload();
pushPayload.shas.push({
id: "invalid-sha",
issueKeys: [ issueKey ]
});

await processPush(getGitHubClient(), getContext({ lastAttempt: true }), pushPayload, logger);
});
});

describe("Skipping msg when issue not exist", () => {

describe("Use redis to avoid overload jira", () => {
it("should reuse status from redis and only call jira once for same issue-key", async () => {
Expand All @@ -128,8 +195,8 @@ describe("Enqueue push", () => {

mockIssueNotExists();

await processPush(getGitHubClient(), getPushPayload(), logger);
await processPush(getGitHubClient(), getPushPayload(), logger);
await processPush(getGitHubClient(), getContext(), getPushPayload(), logger);
await processPush(getGitHubClient(), getContext(), getPushPayload(), logger);
});
});

Expand All @@ -140,7 +207,7 @@ describe("Enqueue push", () => {

mockIssueNotExists();

await processPush(getGitHubClient(), getPushPayload(), logger);
await processPush(getGitHubClient(), getContext(), getPushPayload(), logger);

});

Expand All @@ -156,7 +223,7 @@ describe("Enqueue push", () => {

mockJiraDevInfoAcceptUpdate();

await processPush(getGitHubClient(), getPushPayload(), logger);
await processPush(getGitHubClient(), getContext(), getPushPayload(), logger);

});

Expand All @@ -170,66 +237,81 @@ describe("Enqueue push", () => {

mockJiraDevInfoAcceptUpdate();

await processPush(getGitHubClient(), getPushPayload(), logger);
await processPush(getGitHubClient(), getContext(), getPushPayload(), logger);

});
});

const mockJiraDevInfoAcceptUpdate = () => {
jiraNock.post("/rest/devinfo/0.10/bulk", (reqBody) => {
return reqBody.repositories[0].commits.flatMap(c => c.issueKeys).some(ck => ck === issueKey);
}).reply(202, "");
};

const mockGitHubCommitRestApi = () => {
githubNock
.get("/repos/org1/repo1/commits/" + sha)
.reply(200, {
files: [],
sha
});
};

const getPushPayload = () => {
return {
jiraHost,
installationId: db.subscription.gitHubInstallationId,
gitHubAppConfig: undefined,
webhookId: "aaa",
repository: {
owner: { login: "org1" },
name: "repo1"
} as GitHubRepository,
shas: [{
id: sha,
issueKeys: [issueKey]
}]
};
const mockJiraDevInfoAcceptUpdate = () => {
jiraNock.post("/rest/devinfo/0.10/bulk", (reqBody) => {
return reqBody.repositories[0].commits.flatMap(c => c.issueKeys).some(ck => ck === issueKey);
}).reply(202, "");
};

const mockGitHubCommitRestApi = () => {
githubNock
.get("/repos/org1/repo1/commits/" + sha)
.reply(200, {
files: [],
sha
});
};

const mockGitHubCommitRestApi422 = (sha: string) => {
githubNock
.get("/repos/org1/repo1/commits/" + sha)
.reply(422, {
message: `No commit found for SHA: ${sha}`,
documentation_url: "https://docs.github.com"
});
};

const getPushPayload = () => {
return {
jiraHost,
installationId: db.subscription.gitHubInstallationId,
gitHubAppConfig: undefined,
webhookId: "aaa",
repository: {
owner: { login: "org1" },
name: "repo1"
} as GitHubRepository,
shas: [{
id: sha,
issueKeys: [issueKey]
}]
};
};

const mockIssueExists = () => {
jiraNock.get(`/rest/api/latest/issue/${issueKey}`)
.query({ fields: "summary" }).reply(200, {});
};
const mockIssueExists = () => {
jiraNock.get(`/rest/api/latest/issue/${issueKey}`)
.query({ fields: "summary" }).reply(200, {});
};

const mockIssueNotExists = () => {
jiraNock.get(`/rest/api/latest/issue/${issueKey}`)
.query({ fields: "summary" }).reply(404, "");
};
const mockIssueNotExists = () => {
jiraNock.get(`/rest/api/latest/issue/${issueKey}`)
.query({ fields: "summary" }).reply(404, "");
};

const getGitHubClient = () => {
return new GitHubInstallationClient({
appId: 2,
githubBaseUrl: "https://api.github.com",
installationId: db.subscription.gitHubInstallationId
}, {
apiUrl: "https://api.github.com",
baseUrl: "https://github.com",
graphqlUrl: "https://api.github.com/graphql",
hostname: "https://github.com",
apiKeyConfig: undefined,
proxyBaseUrl: undefined
}, jiraHost, { trigger: "test" }, logger, undefined);
const getGitHubClient = () => {
return new GitHubInstallationClient({
appId: 2,
githubBaseUrl: "https://api.github.com",
installationId: db.subscription.gitHubInstallationId
}, {
apiUrl: "https://api.github.com",
baseUrl: "https://github.com",
graphqlUrl: "https://api.github.com/graphql",
hostname: "https://github.com",
apiKeyConfig: undefined,
proxyBaseUrl: undefined
}, jiraHost, { trigger: "test" }, logger, undefined);
};

const getContext = (override?: any): any => {
return {
...override
};
});
};

});
Loading

0 comments on commit 223264a

Please sign in to comment.