From c24c2b9e97da36524ae297bc6fca22e1c8f7311a Mon Sep 17 00:00:00 2001 From: awalsh128 Date: Thu, 5 Nov 2026 18:22:55 -0800 Subject: [PATCH] fix: write logs under the home cache directory and await artifact upload Instruments opened its log files at the relative path cache-apt-pkgs/ before that directory existed, so the action crashed at startup with ENOENT. It also pointed artifact uploads there while Cache uses ~/cache-apt-pkgs. Resolve both with a shared resolveCacheDir helper and create the directory before opening the logs. The artifact upload was also never awaited, so a failed upload became an unhandled rejection. Await it and report failures as a warning. Co-Authored-By: Claude Opus 5.5 --- src/cache.ts | 14 +++++++++++++- src/index.ts | 6 +++++- src/instrumentation.ts | 18 ++++++++++++------ test/instrumentation.test.ts | 26 ++++++++++++++++++++++++++ 4 files changed, 56 insertions(+), 8 deletions(-) diff --git a/src/cache.ts b/src/cache.ts index f6cde0f..2ddd22d 100644 --- a/src/cache.ts +++ b/src/cache.ts @@ -12,6 +12,18 @@ import { CommandRunner, PackageName } from "ts-apt"; export const CACHE_DEFAULT_DIRNAME = "cache-apt-pkgs"; export const CACHE_KEY_FILENAME = "cache_key.md5"; +/** + * Resolves the absolute cache directory path for a cache directory name. + * + * @param cacheDir Directory name under the user's home directory. + * @returns Absolute path to the cache directory. + */ +export function resolveCacheDir( + cacheDir: string = CACHE_DEFAULT_DIRNAME, +): string { + return path.join(os.homedir(), cacheDir); +} + export const MANIFEST_MAIN_FILENAME = "manifest_main.json"; export const MANIFEST_ALL_FILENAME = "manifest_all.json"; @@ -106,7 +118,7 @@ export class Cache { logger: winston.Logger, cacheDir: string = CACHE_DEFAULT_DIRNAME, ) { - this.path = path.join(os.homedir(), cacheDir); + this.path = resolveCacheDir(cacheDir); this.commandRunner = commandRunner; this.logger = logger; } diff --git a/src/index.ts b/src/index.ts index 49100cc..2e02257 100644 --- a/src/index.ts +++ b/src/index.ts @@ -70,7 +70,11 @@ async function main(): Promise { const message = error instanceof Error ? error.message : String(error); core.setFailed(message); } finally { - instruments.artifacts.upload(); + // Logs are diagnostics, so a failed upload shouldn't fail the action. + await instruments.artifacts.upload().catch((error: unknown) => { + const message = error instanceof Error ? error.message : String(error); + core.warning(`Failed to upload diagnostic artifacts: ${message}`); + }); } } diff --git a/src/instrumentation.ts b/src/instrumentation.ts index ee49703..2c38dcd 100644 --- a/src/instrumentation.ts +++ b/src/instrumentation.ts @@ -8,6 +8,7 @@ import { CACHE_KEY_FILENAME, MANIFEST_ALL_FILENAME, MANIFEST_MAIN_FILENAME, + resolveCacheDir, } from "./cache.js"; const APP_LOG_FILENAME = "capa_app.log"; @@ -40,9 +41,9 @@ export class Artifacts { ) {} /** Upload artifacts to GitHub Actions via [@actions/artifact]. */ - upload(): string { + async upload(): Promise { const client = new DefaultArtifactClient(); - client.uploadArtifact( + await client.uploadArtifact( "capa_artifacts-" + this.runId, this.artifactFilenames .map((filename) => path.join(this.dir, filename)) @@ -99,8 +100,10 @@ function createGitHubLogger(debug: boolean, filepath: string): winston.Logger { /** Container for all instrumentation: telemetry, logging, and artifacts generated */ export class Instruments { - /** Cache directory where all artifacts are stored, including cached packages. */ + /** Cache directory name where all artifacts are stored, including cached packages. */ readonly cacheDir: string; + /** Absolute path of the cache directory (i.e. under the home directory). */ + readonly cacheDirPath: string; /** Whether debug is enabled which affects artifacts otherwise not. */ readonly debug: boolean; /** Manages artifact creation and upload. */ @@ -112,11 +115,14 @@ export class Instruments { constructor(cacheDir: string, debug: boolean, artifacts?: Artifacts) { this.cacheDir = cacheDir; + this.cacheDirPath = resolveCacheDir(cacheDir); this.debug = debug; + // The log streams open their files right away, so the directory must exist. + fs.mkdirSync(this.cacheDirPath, { recursive: true }); this.artifacts = artifacts ?? - new Artifacts(cacheDir, [ + new Artifacts(this.cacheDirPath, [ APP_LOG_FILENAME, EXEC_LOG_FILENAME, CACHE_KEY_FILENAME, @@ -125,11 +131,11 @@ export class Instruments { ]); this.appLogger = createGitHubLogger( debug, - path.join(cacheDir, APP_LOG_FILENAME), + path.join(this.cacheDirPath, APP_LOG_FILENAME), ); this.execLogger = createExecLogger( debug, - path.join(cacheDir, EXEC_LOG_FILENAME), + path.join(this.cacheDirPath, EXEC_LOG_FILENAME), ); } } diff --git a/test/instrumentation.test.ts b/test/instrumentation.test.ts index e4ea0b4..123f169 100644 --- a/test/instrumentation.test.ts +++ b/test/instrumentation.test.ts @@ -1,3 +1,6 @@ +import fs from "node:fs"; +import os from "node:os"; +import path from "node:path"; import { Artifacts, Instruments } from "../src/instrumentation.ts"; import { afterEach, describe, expect, it, vi } from "vitest"; @@ -41,6 +44,29 @@ describe("instrumentation", () => { }); }); + describe("Instruments", () => { + const CACHE_DIRNAME = "cache-apt-pkgs"; + + it("constructor cache dir missing creates it under home with the logs", async () => { + const home = fs.mkdtempSync(path.join(os.tmpdir(), "instruments-test-")); + vi.stubEnv("HOME", home); + const expectedDir = path.join(home, CACHE_DIRNAME); + + const instruments = new Instruments(CACHE_DIRNAME, false); + instruments.appLogger.info("app message"); + await new Promise((resolve) => instruments.appLogger.end(resolve)); + + expect(instruments.cacheDirPath).toBe(expectedDir); + expect(instruments.artifacts.dir).toBe(expectedDir); + await vi.waitFor(() => + expect( + fs.readFileSync(path.join(expectedDir, "capa_app.log"), "utf8"), + ).toContain("app message"), + ); + fs.rmSync(home, { recursive: true, force: true }); + }); + }); + // describe("Instruments", () => { // it("constructor initializes properties correctly", () => { // const cacheDir = "/path/to/cache";