mirror of
https://github.com/ZHANGTIANYAO1/teamspeak-music-bot.git
synced 2026-10-02 04:52:50 +08:00
fix(local): keep the original size when the source video can't be deleted (#149)
After extracting the audio track, uploadAudio assigned the record's `size`
from the new .mka BEFORE deleting the source video:
size = statSync(extracted).size;
rmSync(filePath, { force: true }); // can throw EBUSY/EPERM on Windows
filePath = extracted;
rmSync with force:true only swallows ENOENT — a briefly locked file (exactly
what the existing scheduleRetry machinery in this file exists to handle)
throws. The catch then discards the extract and keeps playing the original
container, which is correct, but `size` had already been overwritten with the
much smaller extracted size while the whole video stayed on disk. That makes
totalBytes() under-count and lets the upload directory grow past its quota.
Commit filePath and size together, only once the source is actually gone.
Adds a regression test that partially mocks node:fs to make rmSync throw for
the source .mp4 and asserts the persisted record (index.json — `size` is not
exposed through search()/toSong) still describes the retained file. With the
old ordering it records 27894 bytes for a 104544-byte file.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
1 parent
c79a9a6dee
commit
19ad48c4ab
2 files changed
+116
-1
No files matched your search
@@ -0,0 +1,108 @@
|
|||||||
|
import { describe, it, expect, beforeEach, afterEach, vi } from "vitest";
|
||||||
|
// unlinkSync/rmdirSync are NOT mocked below, so the test's own fixture
|
||||||
|
// teardown is unaffected by the simulated lock on *.mp4.
|
||||||
|
import { mkdtempSync, statSync, existsSync, readFileSync, unlinkSync, readdirSync, rmdirSync } from "node:fs";
|
||||||
|
import { spawnSync } from "node:child_process";
|
||||||
|
import { createRequire } from "node:module";
|
||||||
|
import { tmpdir } from "node:os";
|
||||||
|
import { join } from "node:path";
|
||||||
|
|
||||||
|
/**
|
||||||
|
* #149: when the audio track is extracted successfully but the source video
|
||||||
|
* cannot be deleted (Windows keeps files locked briefly — rmSync with
|
||||||
|
* force:true still throws EBUSY/EPERM), the record must fall back to the
|
||||||
|
* ORIGINAL container completely: both the path AND the recorded size.
|
||||||
|
*
|
||||||
|
* Committing the size before the delete succeeded would leave the record
|
||||||
|
* claiming the small extracted size while still holding the whole video, so
|
||||||
|
* totalBytes() under-counts and the upload directory grows past its quota.
|
||||||
|
*
|
||||||
|
* This lives in its own file because it partially mocks node:fs, which would
|
||||||
|
* otherwise leak into every other test in local.test.ts.
|
||||||
|
*/
|
||||||
|
vi.mock("node:fs", async (importOriginal) => {
|
||||||
|
const actual = await importOriginal<typeof import("node:fs")>();
|
||||||
|
return {
|
||||||
|
...actual,
|
||||||
|
default: actual,
|
||||||
|
rmSync: (path: string, opts?: object) => {
|
||||||
|
// Simulate the lock on the source video only; every other delete
|
||||||
|
// (the discarded .mka, temp dirs, the reject path) behaves normally.
|
||||||
|
if (typeof path === "string" && path.endsWith(".mp4")) {
|
||||||
|
const err = new Error("EBUSY: resource busy or locked") as NodeJS.ErrnoException;
|
||||||
|
err.code = "EBUSY";
|
||||||
|
throw err;
|
||||||
|
}
|
||||||
|
return actual.rmSync(path, opts as never);
|
||||||
|
},
|
||||||
|
};
|
||||||
|
});
|
||||||
|
|
||||||
|
const { LocalMusicProvider } = await import("./local.js");
|
||||||
|
|
||||||
|
const ffmpeg: string | null = (() => {
|
||||||
|
try {
|
||||||
|
return createRequire(import.meta.url)("ffmpeg-static") as string;
|
||||||
|
} catch {
|
||||||
|
return null;
|
||||||
|
}
|
||||||
|
})();
|
||||||
|
const have = !!ffmpeg && spawnSync(ffmpeg, ["-version"], { stdio: "ignore" }).status === 0;
|
||||||
|
|
||||||
|
let dir: string;
|
||||||
|
beforeEach(() => { dir = mkdtempSync(join(tmpdir(), "local-extract-fallback-")); });
|
||||||
|
afterEach(() => {
|
||||||
|
// Recursive teardown without rmSync (mocked above for *.mp4).
|
||||||
|
for (const f of readdirSync(dir)) {
|
||||||
|
try { unlinkSync(join(dir, f)); } catch { /* best effort */ }
|
||||||
|
}
|
||||||
|
try { rmdirSync(dir); } catch { /* best effort */ }
|
||||||
|
});
|
||||||
|
|
||||||
|
describe("LocalMusicProvider: source video cannot be deleted after extraction (#149)", () => {
|
||||||
|
it.runIf(have)("keeps the original container AND its real size, not the extracted size", async () => {
|
||||||
|
const src = join(dir, "fixture.mp4");
|
||||||
|
const r = spawnSync(ffmpeg!, [
|
||||||
|
"-y", "-hide_banner", "-loglevel", "error",
|
||||||
|
"-f", "lavfi", "-i", "testsrc=s=320x240:r=25:d=3",
|
||||||
|
"-f", "lavfi", "-i", "sine=f=440:d=3",
|
||||||
|
"-c:v", "libx264", "-b:v", "800k", "-c:a", "aac", "-shortest", src,
|
||||||
|
], { stdio: "ignore" });
|
||||||
|
expect(r.status).toBe(0);
|
||||||
|
|
||||||
|
const bytes = readFileSync(src);
|
||||||
|
unlinkSync(src); // uploadAudio writes its own copy under a uuid name
|
||||||
|
|
||||||
|
const p = new LocalMusicProvider(dir);
|
||||||
|
const song = await p.uploadAudio({
|
||||||
|
buffer: bytes, originalName: "fixture.mp4", mimeType: "video/mp4",
|
||||||
|
});
|
||||||
|
|
||||||
|
const resolved = await p.getSongUrl(song.id);
|
||||||
|
expect(resolved).not.toBeNull();
|
||||||
|
|
||||||
|
// Fell back to the original container — the extract was discarded.
|
||||||
|
expect(resolved!.url.endsWith(".mp4")).toBe(true);
|
||||||
|
expect(existsSync(resolved!.url)).toBe(true);
|
||||||
|
expect(existsSync(resolved!.url.replace(/\.mp4$/, ".mka"))).toBe(false);
|
||||||
|
|
||||||
|
const onDisk = statSync(resolved!.url).size;
|
||||||
|
expect(onDisk).toBe(bytes.length);
|
||||||
|
|
||||||
|
// The RECORDED size drives the quota (totalBytes()), so it must describe
|
||||||
|
// the file actually retained. It is not exposed through search()/toSong,
|
||||||
|
// but it is persisted to index.json — read it back from there.
|
||||||
|
const record = (JSON.parse(readFileSync(join(dir, "index.json"), "utf8")) as Array<{
|
||||||
|
id: string; size: number; filePath: string;
|
||||||
|
}>).find((r) => r.id === song.id);
|
||||||
|
expect(record).toBeDefined();
|
||||||
|
expect(record!.filePath.endsWith(".mp4")).toBe(true);
|
||||||
|
// Before the fix this was the (much smaller) .mka size while the whole
|
||||||
|
// .mp4 stayed on disk, so the quota under-counted the retained bytes.
|
||||||
|
expect(record!.size).toBe(bytes.length);
|
||||||
|
|
||||||
|
// Sanity: the extract really is much smaller, so a wrong commit order
|
||||||
|
// would have been clearly observable rather than a rounding error.
|
||||||
|
expect(onDisk).toBeGreaterThan(50_000);
|
||||||
|
}, 60000);
|
||||||
|
});
|
||||||
+8
-1
@@ -330,9 +330,16 @@ export class LocalMusicProvider implements MusicProvider {
|
|||||||
const extracted = path.join(this.uploadDir, `${id}${EXTRACTED_AUDIO_EXT}`);
|
const extracted = path.join(this.uploadDir, `${id}${EXTRACTED_AUDIO_EXT}`);
|
||||||
if (await extractAudioTrack(filePath, extracted)) {
|
if (await extractAudioTrack(filePath, extracted)) {
|
||||||
try {
|
try {
|
||||||
size = statSync(extracted).size;
|
// Commit filePath and size TOGETHER, and only after the source is
|
||||||
|
// actually gone. rmSync(force) still throws EBUSY/EPERM on Windows,
|
||||||
|
// and assigning size first would leave the record claiming the
|
||||||
|
// small extracted size while still pointing at the whole video —
|
||||||
|
// which makes totalBytes() under-count and lets the upload
|
||||||
|
// directory grow past its quota.
|
||||||
|
const extractedSize = statSync(extracted).size;
|
||||||
rmSync(filePath, { force: true });
|
rmSync(filePath, { force: true });
|
||||||
filePath = extracted;
|
filePath = extracted;
|
||||||
|
size = extractedSize;
|
||||||
} catch {
|
} catch {
|
||||||
// Could not stat/remove (Windows lock) — keep playing the original
|
// Could not stat/remove (Windows lock) — keep playing the original
|
||||||
// container and drop the half-finished extract.
|
// container and drop the half-finished extract.
|
||||||
|
|||||||
Reference in new issue
Block a user