Merge pull request #102 from github/joshmgross/action-errors

Ensure errors are handled when fetching action metadata

actions-languageserver/src/description-providers/action-input.test.ts

@@ -111,8 +111,7 @@ describe("action descriptions", () => {
     expect(await getDescription("typo", mock)).toBeUndefined();
   });
 
-  // TODO: https://github.com/github/c2c-actions-experience/issues/7056
-  it.failing("action does not exist", async () => {
+  it("action does not exist", async () => {
     const mock = fetchMock
       .sandbox()
       .getOnce("https://api.github.com/repos/actions/checkout/contents/action.yml?ref=v3", 404)
@@ -121,8 +120,7 @@ describe("action descriptions", () => {
     expect(await getDescription("repository", mock)).toBeUndefined();
   });
 
-  // TODO: https://github.com/github/c2c-actions-experience/issues/7056
-  it.failing("invalid permissions", async () => {
+  it("invalid permissions", async () => {
     const mock = fetchMock
       .sandbox()
       .getOnce("https://api.github.com/repos/actions/checkout/contents/action.yml?ref=v3", 403);

actions-languageserver/src/utils/action-metadata.test.ts

@@ -0,0 +1,251 @@
+import {Octokit} from "@octokit/rest";
+import fetchMock from "fetch-mock";
+import {fetchActionMetadata} from "./action-metadata";
+import {TTLCache} from "./cache";
+
+// A simplified version of the action.yml file from actions/checkout
+const actionMetadataContent = `
+name: 'Checkout'
+description: 'Checkout a Git repository at a particular version'
+inputs:
+  repository:
+    description: Repository name with owner. For example, actions/checkout
+    default: \${{ github.repository }}
+runs:
+  using: node16
+  main: dist/index.js
+  post: dist/index.js
+`;
+
+// Based on https://api.github.com/repos/actions/checkout/contents/action.yml?ref=v3
+const actionMetadata = {
+  name: "action.yml",
+  path: "action.yml",
+  sha: "cab09ebd3a964aba67b57f9727f5f6fff1372b04",
+  size: 3649,
+  url: "https://api.github.com/repos/actions/checkout/contents/action.yml?ref=v3",
+  html_url: "https://github.com/actions/checkout/blob/v3/action.yml",
+  git_url: "https://api.github.com/repos/actions/checkout/git/blobs/cab09ebd3a964aba67b57f9727f5f6fff1372b04",
+  download_url: "https://raw.githubusercontent.com/actions/checkout/v3/action.yml",
+  type: "file",
+  content: Buffer.from(actionMetadataContent).toString("base64"),
+  encoding: "base64",
+  _links: {
+    self: "https://api.github.com/repos/actions/checkout/contents/action.yml?ref=v3",
+    git: "https://api.github.com/repos/actions/checkout/git/blobs/cab09ebd3a964aba67b57f9727f5f6fff1372b04",
+    html: "https://github.com/actions/checkout/blob/v3/action.yml"
+  }
+};
+
+async function fetchActionWithMock(mock: fetchMock.FetchMockSandbox, cache?: TTLCache) {
+  return await fetchActionMetadata(
+    new Octokit({
+      request: {
+        fetch: mock
+      }
+    }),
+    cache || new TTLCache(),
+    {
+      owner: "actions",
+      name: "checkout",
+      ref: "v3"
+    }
+  );
+}
+
+describe("fetchActionMetadata", () => {
+  it("fetches action metadata", async () => {
+    const mock = fetchMock
+      .sandbox()
+      .getOnce("https://api.github.com/repos/actions/checkout/contents/action.yml?ref=v3", actionMetadata);
+
+    const metadata = await fetchActionWithMock(mock);
+
+    expect(metadata?.inputs?.repository?.description).toEqual(
+      "Repository name with owner. For example, actions/checkout"
+    );
+  });
+
+  it("fetches action metadata at a path", async () => {
+    const mock = fetchMock
+      .sandbox()
+      .getOnce("https://api.github.com/repos/actions/checkout/contents/some-path%2Faction.yml?ref=v3", actionMetadata);
+
+    const metadata = await fetchActionMetadata(
+      new Octokit({
+        request: {
+          fetch: mock
+        }
+      }),
+      new TTLCache(),
+      {
+        owner: "actions",
+        name: "checkout",
+        ref: "v3",
+        path: "some-path"
+      }
+    );
+
+    expect(metadata?.inputs?.repository?.description).toEqual(
+      "Repository name with owner. For example, actions/checkout"
+    );
+  });
+
+  it("falls back to .yaml extension on 404", async () => {
+    const mock = fetchMock
+      .sandbox()
+      .getOnce("https://api.github.com/repos/actions/checkout/contents/action.yml?ref=v3", 404)
+      .getOnce("https://api.github.com/repos/actions/checkout/contents/action.yaml?ref=v3", actionMetadata);
+
+    const metadata = await fetchActionWithMock(mock);
+
+    expect(metadata?.inputs?.repository?.description).toEqual(
+      "Repository name with owner. For example, actions/checkout"
+    );
+  });
+
+  it("fetches action metadata at a path with a .yaml extension", async () => {
+    const mock = fetchMock
+      .sandbox()
+      .getOnce("https://api.github.com/repos/actions/checkout/contents/some-path%2Faction.yml?ref=v3", 404)
+      .getOnce("https://api.github.com/repos/actions/checkout/contents/some-path%2Faction.yaml?ref=v3", actionMetadata);
+
+    const metadata = await fetchActionMetadata(
+      new Octokit({
+        request: {
+          fetch: mock
+        }
+      }),
+      new TTLCache(),
+      {
+        owner: "actions",
+        name: "checkout",
+        ref: "v3",
+        path: "some-path"
+      }
+    );
+
+    expect(metadata?.inputs?.repository?.description).toEqual(
+      "Repository name with owner. For example, actions/checkout"
+    );
+  });
+
+  it("does not fall back for other errors", async () => {
+    const mock = fetchMock
+      .sandbox()
+      .getOnce("https://api.github.com/repos/actions/checkout/contents/action.yml?ref=v3", 403);
+
+    const metadata = await fetchActionWithMock(mock);
+
+    expect(metadata).toBeUndefined();
+  });
+
+  it("handles invalid actions", async () => {
+    const mock = fetchMock
+      .sandbox()
+      .getOnce("https://api.github.com/repos/actions/checkout/contents/action.yml?ref=v3", 404)
+      .getOnce("https://api.github.com/repos/actions/checkout/contents/action.yaml?ref=v3", 404);
+
+    const metadata = await fetchActionWithMock(mock);
+
+    expect(metadata).toBeUndefined();
+  });
+
+  it("caches action metadata", async () => {
+    const mock = fetchMock
+      .sandbox()
+      .getOnce("https://api.github.com/repos/actions/checkout/contents/action.yml?ref=v3", actionMetadata);
+
+    const cache = new TTLCache();
+
+    const metadata = await fetchActionWithMock(mock, cache);
+    expect(metadata?.inputs?.repository?.description).toEqual(
+      "Repository name with owner. For example, actions/checkout"
+    );
+
+    const cachedMetadata = await fetchActionWithMock(mock, cache);
+    expect(cachedMetadata?.inputs?.repository?.description).toEqual(
+      "Repository name with owner. For example, actions/checkout"
+    );
+  });
+
+  it("caches action metadata", async () => {
+    const mock = fetchMock
+      .sandbox()
+      .getOnce("https://api.github.com/repos/actions/checkout/contents/action.yml?ref=v3", actionMetadata);
+
+    const cache = new TTLCache();
+
+    const metadata = await fetchActionWithMock(mock, cache);
+    expect(metadata?.inputs?.repository?.description).toEqual(
+      "Repository name with owner. For example, actions/checkout"
+    );
+
+    const cachedMetadata = await fetchActionWithMock(mock, cache);
+    expect(cachedMetadata?.inputs?.repository?.description).toEqual(
+      "Repository name with owner. For example, actions/checkout"
+    );
+  });
+
+  it("ignores directories", async () => {
+    const mock = fetchMock
+      .sandbox()
+      .getOnce("https://api.github.com/repos/actions/checkout/contents/action.yml?ref=v3", [actionMetadata]);
+
+    const metadata = await fetchActionWithMock(mock);
+
+    expect(metadata).toBeUndefined();
+  });
+
+  it("ignores non-files", async () => {
+    const mock = fetchMock
+      .sandbox()
+      .getOnce("https://api.github.com/repos/actions/checkout/contents/action.yml?ref=v3", {
+        type: "not-a-file",
+        content: Buffer.from(actionMetadataContent).toString("base64")
+      });
+
+    const metadata = await fetchActionWithMock(mock);
+
+    expect(metadata).toBeUndefined();
+  });
+
+  it("ignores responses without content", async () => {
+    const mock = fetchMock
+      .sandbox()
+      .getOnce("https://api.github.com/repos/actions/checkout/contents/action.yml?ref=v3", {
+        type: "file"
+      });
+
+    const metadata = await fetchActionWithMock(mock);
+
+    expect(metadata).toBeUndefined();
+  });
+
+  it("handles emojis in action descriptions", async () => {
+    const actionMetadataContent = `
+name: 'Checkout'
+description: 'Checkout a Git repository at a particular version'
+inputs:
+  repository:
+    description: 📦 Repository 📦 name with owner. For example, actions/checkout
+    default: \${{ github.repository }}
+runs:
+  using: node16
+  main: dist/index.js
+  post: dist/index.js
+`;
+    const mock = fetchMock
+      .sandbox()
+      .getOnce("https://api.github.com/repos/actions/checkout/contents/action.yml?ref=v3", {
+        type: "file",
+        content: Buffer.from(actionMetadataContent).toString("base64")
+      });
+
+    const metadata = await fetchActionWithMock(mock);
+
+    expect(metadata?.inputs?.repository?.description).toEqual(
+      "📦 Repository 📦 name with owner. For example, actions/checkout"
+    );
+  });
+});

actions-languageserver/src/utils/action-metadata.ts

@@ -1,4 +1,5 @@
 import {ActionReference, ActionInputs, ActionOutputs, actionIdentifier} from "@github/actions-languageservice/action";
+import {error} from "@github/actions-languageservice/log";
 import {Octokit, RestEndpointMethodTypes} from "@octokit/rest";
 import {parse} from "yaml";
 import {TTLCache} from "./cache";
@@ -20,33 +21,20 @@ export async function fetchActionMetadata(
     return undefined;
   }
 
-  return parseActionMetadata(metadata);
+  // https://docs.github.com/actions/creating-actions/metadata-syntax-for-github-actions
+  return parse(metadata);
 }
 
 async function getActionMetadata(client: Octokit, action: ActionReference): Promise<string | undefined> {
   let resp: RestEndpointMethodTypes["repos"]["getContent"]["response"];
   try {
-    resp = await client.repos.getContent({
-      owner: action.owner,
-      repo: action.name,
-      ref: action.ref,
-      path: action.path ? `${action.path}/action.yml` : "action.yml"
-    });
+    resp = await fetchAction(client, action);
   } catch (e: any) {
-    // If action.yml doesn't exist, try action.yaml
-    if (e.status === 404) {
-      resp = await client.repos.getContent({
-        owner: action.owner,
-        repo: action.name,
-        ref: action.ref,
-        path: action.path ? `${action.path}/action.yaml` : "action.yaml"
-      });
-    } else {
-      throw e;
-    }
+    error(`Failed to fetch action metadata for ${actionIdentifier(action)}: '${e?.message || "<no details>"}'`);
+    return;
   }
 
-  // https://docs.github.com/en/rest/repos/contents?apiVersion=2022-11-28
+  // https://docs.github.com/rest/repos/contents?apiVersion=2022-11-28
   // Ignore directories (array of files) and non-file content
   if (resp.data === undefined || Array.isArray(resp.data) || resp.data.type !== "file") {
     return undefined;
@@ -56,13 +44,28 @@ async function getActionMetadata(client: Octokit, action: ActionReference): Prom
     return undefined;
   }
 
-  const text = Buffer.from(resp.data.content, "base64").toString("utf8");
-  // Remove any null bytes
-  return text.replace(/\0/g, "");
+  return Buffer.from(resp.data.content, "base64").toString("utf8");
 }
 
-// https://docs.github.com/en/actions/creating-actions/metadata-syntax-for-github-actions
-async function parseActionMetadata(content: string): Promise<ActionMetadata> {
-  const metadata: ActionMetadata = parse(content);
-  return metadata;
+async function fetchAction(client: Octokit, action: ActionReference) {
+  try {
+    return await client.repos.getContent({
+      owner: action.owner,
+      repo: action.name,
+      ref: action.ref,
+      path: action.path ? `${action.path}/action.yml` : "action.yml"
+    });
+  } catch (e: any) {
+    // If action.yml doesn't exist, try action.yaml
+    if (e.status === 404) {
+      return await client.repos.getContent({
+        owner: action.owner,
+        repo: action.name,
+        ref: action.ref,
+        path: action.path ? `${action.path}/action.yaml` : "action.yaml"
+      });
+    } else {
+      throw e;
+    }
+  }
 }