mirror of
https://github.com/awalsh128/cache-apt-pkgs-action.git
synced 2026-11-13 21:36:56 +00:00
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 <noreply@anthropic.com>
This commit is contained in:
parent
71288a5baf
commit
c24c2b9e97
14
src/cache.ts
14
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;
|
||||
}
|
||||
|
|
|
|||
|
|
@ -70,7 +70,11 @@ async function main(): Promise<void> {
|
|||
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}`);
|
||||
});
|
||||
}
|
||||
}
|
||||
|
||||
|
|
|
|||
|
|
@ -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<string> {
|
||||
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),
|
||||
);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -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";
|
||||
|
|
|
|||
Loading…
Reference in a new issue