Skip to content

Commit a34fb57

Browse files
nicholas-codecovnicholas-codecov
authored
feat: Add in better debugging logs (#132)
Add in more rigorous debug logging when it comes to defining the commit sha so we can better diagnose the issues with merge commits. GH codecov/engineering-team#1751
1 parent 3ed9e97 commit a34fb57

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

52 files changed

+1495
-308
lines changed

‎.changeset/shaggy-snakes-sip.md

+9
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
---
2+
"@codecov/bundler-plugin-core": patch
3+
"@codecov/nuxt-plugin": patch
4+
"@codecov/rollup-plugin": patch
5+
"@codecov/vite-plugin": patch
6+
"@codecov/webpack-plugin": patch
7+
---
8+
9+
Add in better debug logging around choosing the commit sha

‎packages/bundler-plugin-core/src/types.ts

+3-2
Original file line numberDiff line numberDiff line change
@@ -138,10 +138,11 @@ export interface ProviderUtilInputs {
138138
}
139139

140140
export interface ProviderUtil {
141-
detect: (arg0: ProviderEnvs) => boolean;
141+
detect: (envs: ProviderEnvs) => boolean;
142142
getServiceName: () => string;
143143
getServiceParams: (
144-
arg0: ProviderUtilInputs,
144+
inputs: ProviderUtilInputs,
145+
output: Output,
145146
) => Promise<ProviderServiceParams>;
146147
getEnvVarNames: () => string[];
147148
}

‎packages/bundler-plugin-core/src/utils/Output.ts

+1-1
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,7 @@ class Output {
135135
};
136136
const envs = process.env;
137137
const inputs: ProviderUtilInputs = { envs, args };
138-
const provider = await detectProvider(inputs);
138+
const provider = await detectProvider(inputs, this);
139139

140140
let url = "";
141141
try {

‎packages/bundler-plugin-core/src/utils/__tests__/provider.test.ts

+20-3
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ import {
1111
import { type ProviderUtilInputs } from "src/types.ts";
1212
import { detectProvider, setSlug } from "../provider.ts";
1313
import { isProgramInstalled } from "../isProgramInstalled";
14-
14+
import { Output } from "../Output.ts";
1515
vi.mock("../isProgramInstalled");
1616
const mockedIsProgramInstalled = isProgramInstalled as Mock;
1717

@@ -40,8 +40,16 @@ describe("detectProvider", () => {
4040
},
4141
};
4242

43-
const result = await detectProvider(inputs);
43+
const output = new Output({
44+
apiUrl: "http://localhost",
45+
bundleName: "provider-test",
46+
debug: false,
47+
dryRun: true,
48+
enableBundleAnalysis: true,
49+
retryCount: 0,
50+
});
4451

52+
const result = await detectProvider(inputs, output);
4553
expect(result.service).toEqual("appveyor");
4654
});
4755
});
@@ -50,13 +58,22 @@ describe("detectProvider", () => {
5058
it("throws an error", async () => {
5159
let error;
5260

61+
const output = new Output({
62+
apiUrl: "http://localhost",
63+
bundleName: "provider-test",
64+
debug: false,
65+
dryRun: true,
66+
enableBundleAnalysis: true,
67+
retryCount: 0,
68+
});
69+
5370
try {
5471
const inputs: ProviderUtilInputs = {
5572
args: {},
5673
envs: {},
5774
};
5875

59-
await detectProvider(inputs);
76+
await detectProvider(inputs, output);
6077
} catch (e) {
6178
error = e;
6279
}

‎packages/bundler-plugin-core/src/utils/fetchWithRetry.ts

+2-2
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,7 @@ export const fetchWithRetry = async ({
2020

2121
for (let i = 0; i < retryCount + 1; i++) {
2222
try {
23-
debug(`Attempting to fetch ${name}, attempt: ${i}`);
23+
debug(`Attempting to fetch ${name}, attempt: ${i + 1}`);
2424
await delay(DEFAULT_RETRY_DELAY * i);
2525
response = await fetch(url, requestData);
2626

@@ -29,7 +29,7 @@ export const fetchWithRetry = async ({
2929
}
3030
break;
3131
} catch (err) {
32-
debug(`${name} fetch attempt ${i} failed`);
32+
debug(`${name} fetch attempt ${i + 1} failed`);
3333
const isLastAttempt = i + 1 === retryCount;
3434

3535
if (isLastAttempt) {

‎packages/bundler-plugin-core/src/utils/logging.ts

+13-4
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,17 @@ export function cyan(msg: string): void {
4040
return l(Chalk.cyan(prepareMessage(msg)));
4141
}
4242

43-
// Disable eslint because we want to allow any type of message when debugging
44-
// eslint-disable-next-line @typescript-eslint/no-explicit-any
45-
export function debug(msg: any): void {
46-
return l(Chalk.italic.yellow(prepareMessage(msg)));
43+
interface DebugOptions {
44+
enabled?: boolean;
45+
}
46+
47+
export function debug(
48+
// Disable eslint because we want to allow any type of message when debugging
49+
// eslint-disable-next-line @typescript-eslint/no-explicit-any
50+
msg: any,
51+
{ enabled = true }: DebugOptions = { enabled: true },
52+
): void {
53+
if (enabled) {
54+
return l(Chalk.italic.yellow(prepareMessage(msg)));
55+
}
4756
}

‎packages/bundler-plugin-core/src/utils/provider.ts

+3-1
Original file line numberDiff line numberDiff line change
@@ -2,17 +2,19 @@ import {
22
type ProviderServiceParams,
33
type ProviderUtilInputs,
44
} from "../types.ts";
5+
import { type Output } from "./Output.ts";
56
import { cyan, red } from "./logging.ts";
67
import { providerList } from "./providers";
78

89
export async function detectProvider(
910
inputs: ProviderUtilInputs,
11+
output: Output,
1012
): Promise<ProviderServiceParams> {
1113
cyan("Detecting CI provider");
1214
for (const provider of providerList) {
1315
if (provider.detect(inputs.envs)) {
1416
cyan(`Detected CI provider: ${provider.getServiceName()}`);
15-
return await provider.getServiceParams(inputs);
17+
return await provider.getServiceParams(inputs, output);
1618
}
1719
}
1820
red("Could not detect CI provider");

‎packages/bundler-plugin-core/src/utils/providers/AppVeyorCI.ts

+32-14
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@ import {
33
type ProviderServiceParams,
44
type ProviderUtilInputs,
55
} from "../../types.ts";
6+
import { type Output } from "../Output.ts";
7+
import { debug } from "../logging.ts";
68

79
export function detect(envs: ProviderEnvs): boolean {
810
return (
@@ -13,7 +15,10 @@ export function detect(envs: ProviderEnvs): boolean {
1315

1416
function _getBuild(inputs: ProviderUtilInputs) {
1517
const { args, envs } = inputs;
16-
return args?.build ?? envs?.APPVEYOR_JOB_ID ?? "";
18+
if (args?.build && args.build !== "") {
19+
return args.build;
20+
}
21+
return envs?.APPVEYOR_JOB_ID ?? "";
1722
}
1823

1924
function _getBuildURL(inputs: ProviderUtilInputs) {
@@ -31,10 +36,9 @@ function _getBuildURL(inputs: ProviderUtilInputs) {
3136

3237
function _getBranch(inputs: ProviderUtilInputs) {
3338
const { args, envs } = inputs;
34-
if (args?.branch && args?.branch !== "") {
35-
return args?.branch;
39+
if (args?.branch && args.branch !== "") {
40+
return args.branch;
3641
}
37-
3842
return envs?.APPVEYOR_REPO_BRANCH ?? "";
3943
}
4044

@@ -51,7 +55,10 @@ function _getJob(envs: ProviderEnvs) {
5155

5256
function _getPR(inputs: ProviderUtilInputs): string {
5357
const { args, envs } = inputs;
54-
return args?.pr ?? envs?.APPVEYOR_PULL_REQUEST_NUMBER ?? "";
58+
if (args?.pr && args.pr !== "") {
59+
return args.pr;
60+
}
61+
return envs?.APPVEYOR_PULL_REQUEST_NUMBER ?? "";
5562
}
5663

5764
function _getService() {
@@ -62,30 +69,41 @@ export function getServiceName(): string {
6269
return "Appveyor CI";
6370
}
6471

65-
function _getSHA(inputs: ProviderUtilInputs) {
72+
function _getSHA(inputs: ProviderUtilInputs, output: Output) {
6673
const { args, envs } = inputs;
67-
return (
68-
args?.sha ??
69-
envs?.APPVEYOR_PULL_REQUEST_HEAD_COMMIT ??
70-
envs?.APPVEYOR_REPO_COMMIT ??
71-
""
72-
);
74+
if (args?.sha && args.sha !== "") {
75+
debug(`Using commit: ${args.sha}`, { enabled: output.debug });
76+
return args.sha;
77+
}
78+
79+
const commitSha =
80+
envs?.APPVEYOR_PULL_REQUEST_HEAD_COMMIT ?? envs?.APPVEYOR_REPO_COMMIT ?? "";
81+
82+
debug(`Using commit: ${commitSha ?? ""}`, {
83+
enabled: output.debug,
84+
});
85+
86+
return commitSha;
7387
}
7488

7589
function _getSlug(inputs: ProviderUtilInputs) {
7690
const { args, envs } = inputs;
77-
return args?.slug ?? envs?.APPVEYOR_REPO_NAME ?? "";
91+
if (args?.slug && args.slug !== "") {
92+
return args.slug;
93+
}
94+
return envs?.APPVEYOR_REPO_NAME ?? "";
7895
}
7996

8097
// eslint-disable-next-line @typescript-eslint/require-await
8198
export async function getServiceParams(
8299
inputs: ProviderUtilInputs,
100+
output: Output,
83101
): Promise<ProviderServiceParams> {
84102
return {
85103
branch: _getBranch(inputs),
86104
build: _getBuild(inputs),
87105
buildURL: _getBuildURL(inputs),
88-
commit: _getSHA(inputs),
106+
commit: _getSHA(inputs, output),
89107
job: _getJob(inputs.envs),
90108
pr: _getPR(inputs),
91109
service: _getService(),

‎packages/bundler-plugin-core/src/utils/providers/AzurePipelines.ts

+43-21
Original file line numberDiff line numberDiff line change
@@ -5,14 +5,19 @@ import {
55
type ProviderUtilInputs,
66
} from "../../types.ts";
77
import { parseSlugFromRemoteAddr } from "../git.ts";
8+
import { type Output } from "../Output.ts";
9+
import { debug } from "../logging.ts";
810

911
export function detect(envs: ProviderEnvs): boolean {
1012
return Boolean(envs?.SYSTEM_TEAMFOUNDATIONSERVERURI);
1113
}
1214

1315
function _getBuild(inputs: ProviderUtilInputs): string {
1416
const { args, envs } = inputs;
15-
return args?.build ?? envs?.BUILD_BUILDNUMBER ?? "";
17+
if (args?.build && args.build !== "") {
18+
return args.build;
19+
}
20+
return envs?.BUILD_BUILDNUMBER ?? "";
1621
}
1722

1823
function _getBuildURL(inputs: ProviderUtilInputs): string {
@@ -25,8 +30,8 @@ function _getBuildURL(inputs: ProviderUtilInputs): string {
2530

2631
function _getBranch(inputs: ProviderUtilInputs): string {
2732
const { args, envs } = inputs;
28-
if (args?.branch && args?.branch !== "") {
29-
return args?.branch;
33+
if (args?.branch && args.branch !== "") {
34+
return args.branch;
3035
}
3136

3237
if (envs?.BUILD_SOURCEBRANCH) {
@@ -42,12 +47,15 @@ function _getJob(envs: ProviderEnvs): string {
4247

4348
function _getPR(inputs: ProviderUtilInputs): string {
4449
const { args, envs } = inputs;
45-
return (
46-
args?.pr ??
50+
if (args?.pr && args.pr !== "") {
51+
return args.pr;
52+
}
53+
54+
const pr =
4755
envs?.SYSTEM_PULLREQUEST_PULLREQUESTNUMBER ??
4856
envs?.SYSTEM_PULLREQUEST_PULLREQUESTID ??
49-
""
50-
);
57+
"";
58+
return pr;
5159
}
5260

5361
function _getService(): string {
@@ -58,11 +66,13 @@ export function getServiceName(): string {
5866
return "Azure Pipelines";
5967
}
6068

61-
function _getSHA(inputs: ProviderUtilInputs): string {
69+
function _getSHA(inputs: ProviderUtilInputs, output: Output): string {
6270
const { args, envs } = inputs;
63-
64-
if (args?.sha && args?.sha !== "") {
65-
return args?.sha;
71+
if (args?.sha && args.sha !== "") {
72+
debug(`Using commit: ${args?.sha}`, {
73+
enabled: output.debug,
74+
});
75+
return args.sha;
6676
}
6777

6878
let commit = envs?.BUILD_SOURCEVERSION ?? "";
@@ -73,35 +83,47 @@ function _getSHA(inputs: ProviderUtilInputs): string {
7383
.execFileSync("git", ["show", "--no-patch", "--format=%P"])
7484
.toString()
7585
.trimEnd();
86+
87+
debug(`Merge commit message: ${mergeCommitMessage}`, {
88+
enabled: output.debug,
89+
});
90+
7691
if (mergeCommitRegex.exec(mergeCommitMessage)) {
77-
const mergeCommit = mergeCommitMessage.split(" ")[1];
78-
commit = mergeCommit ?? "";
92+
const splitMergeCommit = mergeCommitMessage.split(" ");
93+
debug(`Split merge commit: ${splitMergeCommit}`, {
94+
enabled: output.debug,
95+
});
96+
97+
commit = splitMergeCommit?.[1] ?? "";
7998
}
8099
}
81100

82-
return commit ?? "";
101+
debug(`Using commit: ${commit}`, {
102+
enabled: output.debug,
103+
});
104+
105+
return commit;
83106
}
84107

85108
function _getSlug(inputs: ProviderUtilInputs): string {
86109
const { args, envs } = inputs;
110+
if (args?.slug && args.slug !== "") {
111+
return args.slug;
112+
}
87113

88-
return (
89-
args?.slug ??
90-
envs?.BUILD_REPOSITORY_NAME ??
91-
parseSlugFromRemoteAddr("") ??
92-
""
93-
);
114+
return envs?.BUILD_REPOSITORY_NAME ?? parseSlugFromRemoteAddr("") ?? "";
94115
}
95116

96117
// eslint-disable-next-line @typescript-eslint/require-await
97118
export async function getServiceParams(
98119
inputs: ProviderUtilInputs,
120+
output: Output,
99121
): Promise<ProviderServiceParams> {
100122
return {
101123
branch: _getBranch(inputs),
102124
build: _getBuild(inputs),
103125
buildURL: _getBuildURL(inputs),
104-
commit: _getSHA(inputs),
126+
commit: _getSHA(inputs, output),
105127
job: _getJob(inputs.envs),
106128
pr: _getPR(inputs),
107129
service: _getService(),

0 commit comments

Comments
 (0)