From 46aac867e2bc9f12c53eae1897e79c051831d015 Mon Sep 17 00:00:00 2001 From: Quentin Date: Mon, 17 Jul 2023 16:28:48 +0100 Subject: [PATCH 1/9] Better errors --- src/utils/constants.ts | 2 ++ src/utils/workflow.utils.ts | 41 +++++++++++++++++++++++++++++++------ 2 files changed, 37 insertions(+), 6 deletions(-) diff --git a/src/utils/constants.ts b/src/utils/constants.ts index a6b5fd01..71d2b444 100644 --- a/src/utils/constants.ts +++ b/src/utils/constants.ts @@ -14,3 +14,5 @@ export const EXPECTED_PERMISSIONS_BLOCK = [ // the environment changes in a way that would cause a replay to behave differently, e.g. upgrading to a newer // replay-orchestrator-launcher version, or changing the version of puppeteer. export const LOGICAL_ENVIRONMENT_VERSION = 2; + +export const DOCS_URL = "https://app.meticulous.ai/docs/github-actions-v2"; diff --git a/src/utils/workflow.utils.ts b/src/utils/workflow.utils.ts index 0ac632a7..91690088 100644 --- a/src/utils/workflow.utils.ts +++ b/src/utils/workflow.utils.ts @@ -3,6 +3,7 @@ import { GitHub } from "@actions/github/lib/utils"; import { METICULOUS_LOGGER_NAME } from "@alwaysmeticulous/common"; import log from "loglevel"; import { DateTime, Duration } from "luxon"; +import { DOCS_URL } from "./constants"; // The GitHub REST API will not list a workflow run immediately after it has been dispatched const LISTING_AFTER_DISPATCH_DELAY = Duration.fromObject({ seconds: 10 }); @@ -42,12 +43,40 @@ export const startNewWorkflowRun = async ({ commitSha: string; octokit: InstanceType; }): Promise<{ workflowRunId: number; [key: string]: unknown } | undefined> => { - await octokit.rest.actions.createWorkflowDispatch({ - owner, - repo, - workflow_id: workflowId, - ref, - }); + try { + await octokit.rest.actions.createWorkflowDispatch({ + owner, + repo, + workflow_id: workflowId, + ref, + }); + } catch (err: unknown) { + const logger = log.getLogger(METICULOUS_LOGGER_NAME); + if ( + (err as { message?: string } | null)?.message?.includes( + "Workflow does not have 'workflow_dispatch' trigger" + ) + ) { + logger.error( + `Could not trigger a workflow run on ${commitSha} of the base branch (${ref}) to compare against, because there was no Meticulous workflow with the 'workflow_dispatch' trigger on the ${ref} branch.` + + ` Screenshots of the new flows will be taken, but no comparisons will be made.` + + ` If you haven't merged the PR to setup Meticulous in Github Actions to the ${ref} branch yet then this is expected.` + + ` Otherwise please check that Meticulous is running on the ${ref} branch, that it has a 'workflow_dispatch' trigger, and has the appropiate permissions.` + + ` See ${DOCS_URL} for the correct setup.` + ); + logger.debug(err); + return undefined; + } + logger.error( + `Could not trigger a workflow run on ${commitSha} of the base branch (${ref}) to compare against.` + + ` Screenshots of the new flows will be taken, but no comparisons will be made.` + + ` Please check that Meticulous is running on the ${ref} branch, that it has a 'workflow_dispatch' trigger, and has the appropiate permissions.` + + ` See ${DOCS_URL} for the correct setup.`, + err + ); + return undefined; + } + // Wait before listing again await delay(LISTING_AFTER_DISPATCH_DELAY); From d488e6ecbbe4b7ccbd89582db8257fb204070293 Mon Sep 17 00:00:00 2001 From: Quentin Date: Mon, 17 Jul 2023 19:48:47 +0100 Subject: [PATCH 2/9] Fix --- src/utils/workflow.utils.ts | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/src/utils/workflow.utils.ts b/src/utils/workflow.utils.ts index 91690088..69c9bfd8 100644 --- a/src/utils/workflow.utils.ts +++ b/src/utils/workflow.utils.ts @@ -52,10 +52,9 @@ export const startNewWorkflowRun = async ({ }); } catch (err: unknown) { const logger = log.getLogger(METICULOUS_LOGGER_NAME); + const message = (err as { message?: string } | null)?.message ?? ""; if ( - (err as { message?: string } | null)?.message?.includes( - "Workflow does not have 'workflow_dispatch' trigger" - ) + message.includes("Workflow does not have 'workflow_dispatch' trigger") ) { logger.error( `Could not trigger a workflow run on ${commitSha} of the base branch (${ref}) to compare against, because there was no Meticulous workflow with the 'workflow_dispatch' trigger on the ${ref} branch.` + @@ -67,6 +66,16 @@ export const startNewWorkflowRun = async ({ logger.debug(err); return undefined; } + if (message.includes("Resource not accessible by integration")) { + logger.error( + `Does not have permission to trigger a workflow run on ${commitSha} of the base branch (${ref}) to compare against, because the 'actions: write' permission is missing in your workflow YAML file.` + + ` Screenshots of the new flows will be taken, but no comparisons will be made.` + + ` Please check that the Meticulous workflow has the correct permissions: see ${DOCS_URL} for the correct setup.` + ); + logger.debug(err); + return undefined; + } + logger.error( `Could not trigger a workflow run on ${commitSha} of the base branch (${ref}) to compare against.` + ` Screenshots of the new flows will be taken, but no comparisons will be made.` + From 7ccce6224cda438959e9c31c72f8b3b05a26f9dc Mon Sep 17 00:00:00 2001 From: Quentin Date: Mon, 17 Jul 2023 19:49:47 +0100 Subject: [PATCH 3/9] Add actions write permission --- .github/workflows/test-meticulous-against-deployment.yaml | 1 + .github/workflows/test-meticulous.yaml | 1 + 2 files changed, 2 insertions(+) diff --git a/.github/workflows/test-meticulous-against-deployment.yaml b/.github/workflows/test-meticulous-against-deployment.yaml index c677afb0..bf2060af 100644 --- a/.github/workflows/test-meticulous-against-deployment.yaml +++ b/.github/workflows/test-meticulous-against-deployment.yaml @@ -9,6 +9,7 @@ on: pull_request: {} permissions: + actions: write contents: read statuses: write pull-requests: write diff --git a/.github/workflows/test-meticulous.yaml b/.github/workflows/test-meticulous.yaml index 5249ba88..411b707d 100644 --- a/.github/workflows/test-meticulous.yaml +++ b/.github/workflows/test-meticulous.yaml @@ -9,6 +9,7 @@ on: pull_request: {} permissions: + actions: write contents: read statuses: write pull-requests: write From 52f870590ef685e6f7a6afab5c20289e2bddbad2 Mon Sep 17 00:00:00 2001 From: Quentin Date: Mon, 17 Jul 2023 19:58:17 +0100 Subject: [PATCH 4/9] Improvements --- src/action.ts | 4 +--- src/utils/logger.utils.ts | 2 ++ src/utils/workflow.utils.ts | 13 +++++++++---- 3 files changed, 12 insertions(+), 7 deletions(-) diff --git a/src/action.ts b/src/action.ts index 8bd467ac..af87359b 100644 --- a/src/action.ts +++ b/src/action.ts @@ -17,7 +17,7 @@ import { getEnvironment } from "./utils/environment.utils"; import { getBaseAndHeadCommitShas } from "./utils/get-base-and-head-commit-shas"; import { getCodeChangeEvent } from "./utils/get-code-change-event"; import { getInputs } from "./utils/get-inputs"; -import { initLogger, setLogLevel } from "./utils/logger.utils"; +import { initLogger, setLogLevel, shortSha } from "./utils/logger.utils"; import { ResultsReporter } from "./utils/results-reporter"; import { waitForDeploymentUrl } from "./utils/wait-for-deployment-url"; @@ -224,5 +224,3 @@ const getOctokitOrFail = (githubToken: string | null) => { ); } }; - -const shortSha = (sha: string) => sha.slice(0, 7); diff --git a/src/utils/logger.utils.ts b/src/utils/logger.utils.ts index c02b57ec..c1b76d28 100644 --- a/src/utils/logger.utils.ts +++ b/src/utils/logger.utils.ts @@ -33,3 +33,5 @@ export const setLogLevel: (logLevel: string | undefined) => void = ( break; } }; + +export const shortSha = (sha: string) => sha.slice(0, 7); diff --git a/src/utils/workflow.utils.ts b/src/utils/workflow.utils.ts index 69c9bfd8..3925ab0a 100644 --- a/src/utils/workflow.utils.ts +++ b/src/utils/workflow.utils.ts @@ -4,6 +4,7 @@ import { METICULOUS_LOGGER_NAME } from "@alwaysmeticulous/common"; import log from "loglevel"; import { DateTime, Duration } from "luxon"; import { DOCS_URL } from "./constants"; +import { shortSha } from "./logger.utils"; // The GitHub REST API will not list a workflow run immediately after it has been dispatched const LISTING_AFTER_DISPATCH_DELAY = Duration.fromObject({ seconds: 10 }); @@ -57,7 +58,9 @@ export const startNewWorkflowRun = async ({ message.includes("Workflow does not have 'workflow_dispatch' trigger") ) { logger.error( - `Could not trigger a workflow run on ${commitSha} of the base branch (${ref}) to compare against, because there was no Meticulous workflow with the 'workflow_dispatch' trigger on the ${ref} branch.` + + `Could not trigger a workflow run on commit ${shortSha( + commitSha + )} of the base branch (${ref}) to compare against, because there was no Meticulous workflow with the 'workflow_dispatch' trigger on the ${ref} branch.` + ` Screenshots of the new flows will be taken, but no comparisons will be made.` + ` If you haven't merged the PR to setup Meticulous in Github Actions to the ${ref} branch yet then this is expected.` + ` Otherwise please check that Meticulous is running on the ${ref} branch, that it has a 'workflow_dispatch' trigger, and has the appropiate permissions.` + @@ -68,16 +71,18 @@ export const startNewWorkflowRun = async ({ } if (message.includes("Resource not accessible by integration")) { logger.error( - `Does not have permission to trigger a workflow run on ${commitSha} of the base branch (${ref}) to compare against, because the 'actions: write' permission is missing in your workflow YAML file.` + + `Missing permission to trigger a workflow run on the base branch (${ref}).` + ` Screenshots of the new flows will be taken, but no comparisons will be made.` + - ` Please check that the Meticulous workflow has the correct permissions: see ${DOCS_URL} for the correct setup.` + ` Please add the 'actions: write' permission to your workflow YAML file: see ${DOCS_URL} for the correct setup.` ); logger.debug(err); return undefined; } logger.error( - `Could not trigger a workflow run on ${commitSha} of the base branch (${ref}) to compare against.` + + `Could not trigger a workflow run on commit ${shortSha( + commitSha + )} of the base branch (${ref}) to compare against.` + ` Screenshots of the new flows will be taken, but no comparisons will be made.` + ` Please check that Meticulous is running on the ${ref} branch, that it has a 'workflow_dispatch' trigger, and has the appropiate permissions.` + ` See ${DOCS_URL} for the correct setup.`, From a487d6c75477f2501d6ec7c8bd28fa70d64435e9 Mon Sep 17 00:00:00 2001 From: Quentin Date: Tue, 18 Jul 2023 11:46:24 +0100 Subject: [PATCH 5/9] Improvements to more errors --- src/utils/ensure-base-exists.utils.ts | 32 +++++++++--- src/utils/error.utils.ts | 25 ++++++++++ src/utils/results-reporter.ts | 46 ++++++++++++----- src/utils/update-status-comment.ts | 71 +++++++++++++++++---------- src/utils/wait-for-deployment-url.ts | 15 ++++-- src/utils/workflow.utils.ts | 4 +- 6 files changed, 143 insertions(+), 50 deletions(-) create mode 100644 src/utils/error.utils.ts diff --git a/src/utils/ensure-base-exists.utils.ts b/src/utils/ensure-base-exists.utils.ts index b324fffd..f2d8448d 100644 --- a/src/utils/ensure-base-exists.utils.ts +++ b/src/utils/ensure-base-exists.utils.ts @@ -9,7 +9,8 @@ import { METICULOUS_LOGGER_NAME } from "@alwaysmeticulous/common"; import log from "loglevel"; import { Duration } from "luxon"; import { CodeChangeEvent } from "../types"; -import { LOGICAL_ENVIRONMENT_VERSION } from "./constants"; +import { DOCS_URL, LOGICAL_ENVIRONMENT_VERSION } from "./constants"; +import { isGithubPermissionsError } from "./error.utils"; import { getCurrentWorkflowId, getPendingWorkflowRun, @@ -251,11 +252,26 @@ const getHeadCommitForRef = async ({ ref: string; octokit: InstanceType; }): Promise => { - const result = await octokit.rest.repos.getBranch({ - owner, - repo, - branch: ref, - }); - const commitSha = result.data.commit.sha; - return commitSha; + try { + const result = await octokit.rest.repos.getBranch({ + owner, + repo, + branch: ref, + }); + const commitSha = result.data.commit.sha; + return commitSha; + } catch (err: unknown) { + if (isGithubPermissionsError(err)) { + // https://docs.github.com/en/rest/overview/permissions-required-for-github-apps?apiVersion=2022-11-28#repository-permissions-for-contents + throw new Error( + `Missing permission to get the head commit of the branch '${ref}'. This is required in order to correctly calculate the two commits to compare.` + + ` Please add the 'contents: read' permission to your workflow YAML file: see ${DOCS_URL} for the correct setup.` + ); + } + const logger = log.getLogger(METICULOUS_LOGGER_NAME); + logger.error( + `Unable to get head commit of branch '${ref}'. This is required in order to correctly calculate the two commits to compare. Please check that you have setup the correct permissions: see ${DOCS_URL} for the correct setup.` + ); + throw err; + } }; diff --git a/src/utils/error.utils.ts b/src/utils/error.utils.ts new file mode 100644 index 00000000..3682d58c --- /dev/null +++ b/src/utils/error.utils.ts @@ -0,0 +1,25 @@ +export const isGithubPermissionsError = ( + error: MaybeOctokitRequestError | unknown +): boolean => { + const message = getErrorMessage(error); + return ( + message.toLowerCase().includes("resource not accessible by integration") || + (error as MaybeOctokitRequestError)?.status === 403 + ); +}; + +export const getErrorMessage = (error: unknown) => { + const message = (error as MaybeOctokitRequestError)?.message ?? ""; + return typeof message === "string" ? message : ""; +}; + +// Octokit returns RequestErrors, which have a message and a code +// But we can't make any strong assumptions, so we type it on the safe side +// https://github.com/octokit/request-error.js/blob/372097e9b16f70d4ad75089003dc9154e304faa7/src/index.ts#L16 +type MaybeOctokitRequestError = + | { + message?: string | unknown; + status?: number | unknown; + } + | null + | undefined; diff --git a/src/utils/results-reporter.ts b/src/utils/results-reporter.ts index d68edf2c..b9beae6f 100644 --- a/src/utils/results-reporter.ts +++ b/src/utils/results-reporter.ts @@ -1,10 +1,15 @@ import { getOctokit } from "@actions/github"; import { Project } from "@alwaysmeticulous/api"; +import { METICULOUS_LOGGER_NAME } from "@alwaysmeticulous/common"; import { ExecuteTestRunResult, RunningTestRunExecution, } from "@alwaysmeticulous/sdk-bundles-api"; +import log from "loglevel"; import { CodeChangeEvent } from "../types"; +import { DOCS_URL } from "./constants"; +import { isGithubPermissionsError } from "./error.utils"; +import { shortSha } from "./logger.utils"; import { updateStatusComment } from "./update-status-comment"; const SHORT_SHA_LENGTH = 7; @@ -160,18 +165,35 @@ export class ResultsReporter { targetUrl?: string; }) { const { octokit, owner, repo, headSha } = this.options; - return octokit.rest.repos.createCommitStatus({ - owner, - repo, - context: - this.options.testSuiteId != null - ? `Meticulous (${this.options.testSuiteId})` - : "Meticulous", - description, - state, - sha: headSha, - ...(targetUrl ? { target_url: targetUrl } : {}), - }); + try { + return octokit.rest.repos.createCommitStatus({ + owner, + repo, + context: + this.options.testSuiteId != null + ? `Meticulous (${this.options.testSuiteId})` + : "Meticulous", + description, + state, + sha: headSha, + ...(targetUrl ? { target_url: targetUrl } : {}), + }); + } catch (err: unknown) { + if (isGithubPermissionsError(err)) { + // https://docs.github.com/en/actions/using-jobs/assigning-permissions-to-jobs + throw new Error( + `Missing permission to create and update commit statuses.` + + ` Please add the 'statuses: write' permission to your workflow YAML file: see ${DOCS_URL} for the correct setup.` + ); + } + const logger = log.getLogger(METICULOUS_LOGGER_NAME); + logger.error( + `Unable to create commit status for commit '${shortSha( + headSha + )}'. Please check that you have setup the correct permissions: see ${DOCS_URL} for the correct setup.` + ); + throw err; + } } private setStatusComment({ diff --git a/src/utils/update-status-comment.ts b/src/utils/update-status-comment.ts index 4b88f7e8..93f9c6d7 100644 --- a/src/utils/update-status-comment.ts +++ b/src/utils/update-status-comment.ts @@ -1,5 +1,9 @@ import { getOctokit } from "@actions/github"; +import { METICULOUS_LOGGER_NAME } from "@alwaysmeticulous/common"; +import log from "loglevel"; import { CodeChangeEvent } from "../types"; +import { DOCS_URL } from "./constants"; +import { isGithubPermissionsError } from "./error.utils"; const getMeticulousCommentIdentifier = (testSuiteId: string | null) => `