review-comments: refactor getActionsStatus to accept an extra parameter designating if the analysis is third-party

This commit is contained in:
Fotis Koutoulakis (@NlightNFotis)
2025-04-01 14:58:59 +01:00
parent 01f1a1f2c9
commit 55ee663d5f
30 changed files with 121 additions and 56 deletions

View File

@@ -34,6 +34,7 @@ import {
createStatusReportBase,
DatabaseCreationTimings,
getActionsStatus,
isFirstPartyAnalysis,
StatusReportBase,
} from "./status-report";
import {
@@ -72,7 +73,12 @@ async function sendStatusReport(
trapCacheCleanup: TrapCacheCleanupStatusReport | undefined,
logger: Logger,
) {
const status = getActionsStatus(error, stats?.analyze_failure_language);
const isThirdPartyAnalysis = !isFirstPartyAnalysis(ActionName.Analyze);
const status = getActionsStatus(
isThirdPartyAnalysis,
error,
stats?.analyze_failure_language,
);
const statusReportBase = await createStatusReportBase(
ActionName.Analyze,
status,

View File

@@ -150,13 +150,15 @@ test("wrapApiConfigurationError correctly wraps specific configuration errors",
new util.ConfigurationError("API rate limit exceeded for installation"),
);
const tokenSuggestionMessage =
"Please check that your token is valid and has the required permissions: contents: read, security-events: write";
const badCredentialsError = new util.HTTPError("Bad credentials", 401);
res = api.wrapApiConfigurationError(badCredentialsError);
t.deepEqual(res, new util.ConfigurationError("Bad credentials"));
t.deepEqual(res, new util.ConfigurationError(tokenSuggestionMessage));
const notFoundError = new util.HTTPError("Not Found", 404);
res = api.wrapApiConfigurationError(notFoundError);
t.deepEqual(res, new util.ConfigurationError("Not Found"));
t.deepEqual(res, new util.ConfigurationError(tokenSuggestionMessage));
const resourceNotAccessibleError = new util.HTTPError(
"Resource not accessible by integration",

View File

@@ -245,12 +245,17 @@ export function wrapApiConfigurationError(e: unknown) {
if (
e.message.includes("API rate limit exceeded for installation") ||
e.message.includes("commit not found") ||
e.message.includes("Bad credentials") ||
e.message.includes("Not Found") ||
e.message.includes("Resource not accessible by integration") ||
/ref .* not found in this repository/.test(e.message)
) {
return new ConfigurationError(e.message);
} else if (
e.message.includes("Bad credentials") ||
e.message.includes("Not Found")
) {
return new ConfigurationError(
"Please check that your token is valid and has the required permissions: contents: read, security-events: write",
);
}
}
return e;

View File

@@ -18,6 +18,7 @@ import {
createStatusReportBase,
sendStatusReport,
ActionName,
isFirstPartyAnalysis,
} from "./status-report";
import { endTracingForCluster } from "./tracer-config";
import {
@@ -46,7 +47,8 @@ async function sendCompletedStatusReport(
) {
initializeEnvironment(getActionVersion());
const status = getActionsStatus(cause, failingLanguage);
const isThirdPartyAnalysis = !isFirstPartyAnalysis(ActionName.Autobuild);
const status = getActionsStatus(isThirdPartyAnalysis, cause, failingLanguage);
const statusReportBase = await createStatusReportBase(
ActionName.Autobuild,
status,

View File

@@ -25,6 +25,7 @@ import {
getActionsStatus,
ActionName,
getJobStatusDisplayName,
isFirstPartyAnalysis,
} from "./status-report";
import { checkDiskUsage, checkGitHubVersionInRange, wrapError } from "./util";
@@ -74,9 +75,11 @@ async function runWrapper() {
const error = wrapError(unwrappedError);
core.setFailed(error.message);
const isThirdPartyAnalysis = !isFirstPartyAnalysis(ActionName.InitPost);
const statusReportBase = await createStatusReportBase(
ActionName.InitPost,
getActionsStatus(error),
getActionsStatus(isThirdPartyAnalysis, error),
startedAt,
config,
await checkDiskUsage(logger),

View File

@@ -51,6 +51,7 @@ import {
StatusReportBase,
createStatusReportBase,
getActionsStatus,
isFirstPartyAnalysis,
sendStatusReport,
} from "./status-report";
import { ZstdAvailability } from "./tar";
@@ -136,9 +137,10 @@ async function sendCompletedStatusReport(
logger: Logger,
error?: Error,
) {
const isThirdPartyAnalysis = !isFirstPartyAnalysis(ActionName.Init);
const statusReportBase = await createStatusReportBase(
ActionName.Init,
getActionsStatus(error),
getActionsStatus(isThirdPartyAnalysis, error),
startedAt,
config,
await checkDiskUsage(logger),

View File

@@ -16,6 +16,7 @@ import {
createStatusReportBase,
getActionsStatus,
ActionName,
isFirstPartyAnalysis,
} from "./status-report";
import {
checkActionVersion,
@@ -82,9 +83,12 @@ async function run() {
`Failed to resolve a build environment suitable for automatically building your code. ${error.message}`,
);
const isThirdPartyAnalysis = !isFirstPartyAnalysis(
ActionName.ResolveEnvironment,
);
const statusReportBase = await createStatusReportBase(
ActionName.ResolveEnvironment,
getActionsStatus(error),
getActionsStatus(isThirdPartyAnalysis, error),
startedAt,
config,
await checkDiskUsage(logger),

View File

@@ -193,26 +193,37 @@ test("createStatusReportBase_firstParty", async (t) => {
});
test("getActionStatus handling correctly various types of errors", (t) => {
const isThirdPartyAnalysis = true;
const isFirstPartyAnalysis = !isThirdPartyAnalysis;
t.is(
getActionsStatus(new Error("arbitrary error")),
getActionsStatus(isFirstPartyAnalysis, new Error("arbitrary error")),
"failure",
"We categorise an arbitrary error as a failure",
);
t.is(
getActionsStatus(new ConfigurationError("arbitrary error")),
getActionsStatus(
isFirstPartyAnalysis,
new ConfigurationError("arbitrary error"),
),
"user-error",
"We categorise a ConfigurationError as a user error",
);
t.is(
getActionsStatus(new Error("exit code 1"), "multiple things went wrong"),
getActionsStatus(
isFirstPartyAnalysis,
new Error("exit code 1"),
"multiple things went wrong",
),
"failure",
"getActionsStatus should return failure if passed an arbitrary error and an additional failure cause",
);
t.is(
getActionsStatus(
isFirstPartyAnalysis,
new ConfigurationError("exit code 1"),
"multiple things went wrong",
),
@@ -221,34 +232,47 @@ test("getActionStatus handling correctly various types of errors", (t) => {
);
t.is(
getActionsStatus(),
getActionsStatus(isFirstPartyAnalysis),
"success",
"getActionsStatus should return success if no error is passed",
);
t.is(
getActionsStatus(new Object()),
getActionsStatus(isFirstPartyAnalysis, new Object()),
"failure",
"getActionsStatus should return failure if passed an arbitrary object",
);
t.is(
getActionsStatus(null, "an error occurred"),
getActionsStatus(isFirstPartyAnalysis, null, "an error occurred"),
"failure",
"getActionsStatus should return failure if passed null and an additional failure cause",
);
t.is(
getActionsStatus(wrapError(new ConfigurationError("arbitrary error"))),
getActionsStatus(
isFirstPartyAnalysis,
wrapError(new ConfigurationError("arbitrary error")),
),
"user-error",
"We still recognise a wrapped ConfigurationError as a user error",
);
t.is(
getActionsStatus(
isThirdPartyAnalysis,
new InvalidSarifUploadError("SyntaxError: Unexpected end of JSON input"),
),
"user-error",
"We recognise an InvalidSarifUploadError as a user error",
"We recognise an InvalidSarifUploadError as a user error if the tool that generated the SARIF file is not ours",
);
t.is(
getActionsStatus(
isFirstPartyAnalysis,
new InvalidSarifUploadError("arbitrary error"),
),
"failure",
"We recognise an InvalidSarifUploadError as a failure if the tool that generated the SARIF file is ours",
);
});

View File

@@ -170,14 +170,16 @@ export interface DatabaseCreationTimings {
}
export function getActionsStatus(
isThirdPartyAnalysis: boolean,
error?: unknown,
otherFailureCause?: string,
): ActionStatus {
if (error || otherFailureCause) {
if (
error instanceof ConfigurationError ||
error instanceof InvalidSarifUploadError
) {
if (error instanceof ConfigurationError) {
return "user-error";
}
if (error instanceof InvalidSarifUploadError && isThirdPartyAnalysis) {
return "user-error";
}

View File

@@ -104,8 +104,9 @@ async function run() {
}
await sendSuccessStatusReport(startedAt, uploadResult.statusReport, logger);
} catch (unwrappedError) {
const isThirdPartyAnalysis = !isFirstPartyAnalysis(ActionName.UploadSarif);
const error =
!isFirstPartyAnalysis(ActionName.UploadSarif) &&
isThirdPartyAnalysis &&
unwrappedError instanceof upload_lib.InvalidSarifUploadError
? new ConfigurationError(unwrappedError.message)
: wrapError(unwrappedError);
@@ -114,7 +115,7 @@ async function run() {
const errorStatusReportBase = await createStatusReportBase(
ActionName.UploadSarif,
getActionsStatus(error),
getActionsStatus(isThirdPartyAnalysis, error),
startedAt,
undefined,
await checkDiskUsage(logger),