feat: enhance error handling in LaTeX compilation and improve file saving logic; add regression tests for Tauri IPC errors

This commit is contained in:
delibae 2026-03-02 22:51:37 +09:00
parent 84e0ee2fd9
commit f03424dacc
9 changed files with 290 additions and 29 deletions

View file

@ -358,6 +358,10 @@ pub async fn compile_latex(
);
}
// Remove stale PDF so a failed compile doesn't return the previous result.
let pdf_path = work_dir.join(format!("{}.pdf", main_file_name));
let _ = std::fs::remove_file(&pdf_path);
// Run Tectonic in a blocking task (it uses an internal global mutex)
let work_dir_clone = work_dir.clone();
let main_file_clone = main_file.clone();
@ -373,7 +377,6 @@ pub async fn compile_latex(
compile_result.is_ok()
);
let pdf_path = work_dir.join(format!("{}.pdf", main_file_name));
let log_path = work_dir.join(format!("{}.log", main_file_name));
// Handle "No pages of output" — retry with \AtEndDocument{\null} injection
@ -893,4 +896,86 @@ Postamble:
assert!(!dst.path().join("chapters").join("ch1.aux").exists());
assert!(!dst.path().join(".claudeprism").exists());
}
// --- sync_source_files preserves stale PDF (compile_latex deletes it separately) ---
#[test]
fn test_sync_source_files_does_not_copy_pdf_from_source() {
// sync_source_files treats .pdf as an artifact and does not copy it.
// This means a stale PDF in the build dir survives a sync — the
// compile_latex command must delete it explicitly before compiling.
let src = tempfile::tempdir().unwrap();
let dst = tempfile::tempdir().unwrap();
std::fs::write(src.path().join("main.tex"), "new content").unwrap();
// Simulate a stale PDF already sitting in the build dir
std::fs::write(dst.path().join("main.pdf"), "old pdf bytes").unwrap();
sync_source_files(src.path(), dst.path()).unwrap();
// Source .tex was synced
assert_eq!(
std::fs::read_to_string(dst.path().join("main.tex")).unwrap(),
"new content"
);
// Stale PDF in dst was NOT overwritten (sync skips .pdf)
// This confirms that compile_latex must delete the PDF itself
assert!(dst.path().join("main.pdf").exists());
assert_eq!(
std::fs::read_to_string(dst.path().join("main.pdf")).unwrap(),
"old pdf bytes"
);
}
#[test]
fn test_sync_source_files_overwrites_changed_tex_content() {
// Regression: when a user empties a file, sync must overwrite
// the old content in the build dir with the empty content.
let src = tempfile::tempdir().unwrap();
let dst = tempfile::tempdir().unwrap();
// Old content in build dir
std::fs::write(dst.path().join("main.tex"), "old content").unwrap();
// User emptied the file
std::fs::write(src.path().join("main.tex"), "").unwrap();
sync_source_files(src.path(), dst.path()).unwrap();
assert_eq!(
std::fs::read_to_string(dst.path().join("main.tex")).unwrap(),
""
);
}
// --- persistent_build_dir ---
#[test]
fn test_stale_pdf_removal_pattern() {
// Simulates the pattern used in compile_latex: remove stale PDF
// before compilation so a failed compile doesn't return old results.
let build_dir = tempfile::tempdir().unwrap();
let pdf_path = build_dir.path().join("document.pdf");
// Simulate previous successful build left a PDF
std::fs::write(&pdf_path, "old pdf data").unwrap();
assert!(pdf_path.exists());
// This is what compile_latex does before running tectonic
let _ = std::fs::remove_file(&pdf_path);
assert!(!pdf_path.exists());
// If compilation fails, pdf_path.exists() is false → error returned
}
#[test]
fn test_stale_pdf_removal_no_existing_file() {
// remove_file on a non-existent path should not panic (we use let _ =)
let build_dir = tempfile::tempdir().unwrap();
let pdf_path = build_dir.path().join("document.pdf");
assert!(!pdf_path.exists());
let result = std::fs::remove_file(&pdf_path);
// It's an error but we ignore it with let _ =
assert!(result.is_err());
}
}

View file

@ -0,0 +1,56 @@
import { describe, it, expect } from "vitest";
/**
* Regression tests: Tauri IPC error handling.
*
* When a Rust #[tauri::command] returns Err(String), the frontend's
* `invoke()` rejects with a **plain string**, NOT an Error object.
* Catch blocks that only check `error instanceof Error` will miss these
* and fall through to a generic message, losing the actual error details.
*
* The correct pattern is:
* error instanceof Error ? error.message
* : typeof error === "string" ? error
* : "Compilation failed"
*/
/** Helper that mirrors the pattern used in catch blocks */
function extractErrorMessage(error: unknown): string {
return error instanceof Error
? error.message
: typeof error === "string"
? error
: "Compilation failed";
}
describe("Tauri IPC error message extraction", () => {
it("extracts message from a standard Error object", () => {
const error = new Error("Something went wrong");
expect(extractErrorMessage(error)).toBe("Something went wrong");
});
it("extracts message from a plain string (Tauri Err(String) pattern)", () => {
// This is what Tauri invoke() rejects with for Rust Err(String)
const error = "Compilation failed\n\n! Undefined control sequence.";
expect(extractErrorMessage(error)).toBe(
"Compilation failed\n\n! Undefined control sequence."
);
});
it("extracts message from an empty string", () => {
expect(extractErrorMessage("")).toBe("");
});
it("falls back to generic message for non-string, non-Error values", () => {
expect(extractErrorMessage(42)).toBe("Compilation failed");
expect(extractErrorMessage(null)).toBe("Compilation failed");
expect(extractErrorMessage(undefined)).toBe("Compilation failed");
expect(extractErrorMessage({ code: 1 })).toBe("Compilation failed");
});
it("a plain string is NOT instanceof Error (the root cause of the bug)", () => {
const tauriError: unknown = "Compilation failed\n\n! Missing $ inserted.";
expect(tauriError instanceof Error).toBe(false);
expect(typeof tauriError === "string").toBe(true);
});
});

View file

@ -1,4 +1,5 @@
import { describe, it, expect, beforeEach, vi } from "vitest";
import { writeTextFile } from "@tauri-apps/plugin-fs";
import { useDocumentStore, type ProjectFile } from "@/stores/document-store";
// Mock history store
@ -252,4 +253,107 @@ describe("useDocumentStore", () => {
expect(state.selectionRange).toBeNull();
});
});
describe("saveFile", () => {
beforeEach(() => {
vi.mocked(writeTextFile).mockClear();
vi.mocked(writeTextFile).mockResolvedValue(undefined);
});
it("saves a dirty file with content to disk", async () => {
useDocumentStore.setState({
files: [makeFile({ isDirty: true, content: "saved content" })],
});
await useDocumentStore.getState().saveFile("main.tex");
expect(writeTextFile).toHaveBeenCalledWith("/project/main.tex", "saved content");
expect(useDocumentStore.getState().files[0].isDirty).toBe(false);
});
it("saves a dirty file with empty string content (regression: empty content is not falsy-skipped)", async () => {
useDocumentStore.setState({
files: [makeFile({ isDirty: true, content: "" })],
});
await useDocumentStore.getState().saveFile("main.tex");
expect(writeTextFile).toHaveBeenCalledWith("/project/main.tex", "");
expect(useDocumentStore.getState().files[0].isDirty).toBe(false);
});
it("skips saving when content is null", async () => {
useDocumentStore.setState({
files: [makeFile({ isDirty: true, content: null as unknown as string })],
});
await useDocumentStore.getState().saveFile("main.tex");
expect(writeTextFile).not.toHaveBeenCalled();
});
it("skips saving when file is not dirty", async () => {
useDocumentStore.setState({
files: [makeFile({ isDirty: false, content: "clean" })],
});
await useDocumentStore.getState().saveFile("main.tex");
expect(writeTextFile).not.toHaveBeenCalled();
});
});
describe("saveAllFiles", () => {
beforeEach(() => {
vi.mocked(writeTextFile).mockClear();
vi.mocked(writeTextFile).mockResolvedValue(undefined);
});
it("saves all dirty files", async () => {
useDocumentStore.setState({
files: [
makeFile({ isDirty: true, content: "dirty content" }),
makeFile({ id: "clean.tex", name: "clean.tex", absolutePath: "/project/clean.tex", relativePath: "clean.tex", isDirty: false, content: "clean" }),
],
});
await useDocumentStore.getState().saveAllFiles();
expect(writeTextFile).toHaveBeenCalledTimes(1);
expect(writeTextFile).toHaveBeenCalledWith("/project/main.tex", "dirty content");
});
it("saves dirty files with empty string content (regression: empty string is not falsy-skipped)", async () => {
useDocumentStore.setState({
files: [
makeFile({ isDirty: true, content: "" }),
makeFile({ id: "slide.tex", name: "slide.tex", absolutePath: "/project/slide.tex", relativePath: "slide.tex", isDirty: true, content: "" }),
],
});
await useDocumentStore.getState().saveAllFiles();
expect(writeTextFile).toHaveBeenCalledTimes(2);
expect(writeTextFile).toHaveBeenCalledWith("/project/main.tex", "");
expect(writeTextFile).toHaveBeenCalledWith("/project/slide.tex", "");
// Both should be marked clean
const files = useDocumentStore.getState().files;
expect(files.every((f) => !f.isDirty)).toBe(true);
});
it("skips files with null content even if dirty", async () => {
useDocumentStore.setState({
files: [
makeFile({ isDirty: true, content: null as unknown as string }),
],
});
await useDocumentStore.getState().saveAllFiles();
expect(writeTextFile).not.toHaveBeenCalled();
});
});
describe("setPdfData / setCompileError", () => {
it("setPdfData clears compile error", () => {
useDocumentStore.setState({ compileError: "some error" });
useDocumentStore.getState().setPdfData(new Uint8Array([1, 2, 3]));
const state = useDocumentStore.getState();
expect(state.pdfData).toEqual(new Uint8Array([1, 2, 3]));
expect(state.compileError).toBeNull();
});
it("setCompileError stores the error string (regression: Tauri string errors must be preserved)", () => {
useDocumentStore.getState().setCompileError("Compilation failed\n\n! Undefined control sequence.");
expect(useDocumentStore.getState().compileError).toBe(
"Compilation failed\n\n! Undefined control sequence."
);
});
});
});

View file

@ -30,7 +30,7 @@ function getFileIcon(file: ProjectFile) {
return <FileTextIcon className="size-3.5 shrink-0 text-muted-foreground" />;
}
export const ChatComposer: FC = () => {
export const ChatComposer: FC<{ isOpen?: boolean }> = ({ isOpen }) => {
const sendPrompt = useClaudeChatStore((s) => s.sendPrompt);
const cancelExecution = useClaudeChatStore((s) => s.cancelExecution);
const isStreaming = useClaudeChatStore((s) => s.isStreaming);
@ -86,6 +86,15 @@ export const ChatComposer: FC = () => {
const pendingAttachments = useClaudeChatStore((s) => s.pendingAttachments);
const consumePendingAttachments = useClaudeChatStore((s) => s.consumePendingAttachments);
// Focus textarea when the drawer opens
const prevOpenRef = useRef(false);
useEffect(() => {
if (isOpen && !prevOpenRef.current) {
setTimeout(() => textareaRef.current?.focus(), 0);
}
prevOpenRef.current = !!isOpen;
}, [isOpen]);
useEffect(() => {
if (pendingAttachments.length === 0) return;
const attachments = consumePendingAttachments();
@ -697,7 +706,6 @@ export const ChatComposer: FC = () => {
onPaste={handlePaste}
placeholder="Ask me anything (/ for commands, @ to mention)"
className="max-h-40 min-h-10 w-full resize-none bg-transparent px-4 py-2 text-sm outline-none placeholder:text-muted-foreground"
autoFocus
rows={1}
/>
)}

View file

@ -145,7 +145,7 @@ export function ClaudeChatDrawer() {
</div>
{/* Composer */}
<ChatComposer />
<ChatComposer isOpen={isOpen} />
</div>
</div>
);

View file

@ -278,7 +278,7 @@ export function LatexEditor() {
const data = await compileLatex(projectRoot, targetFile);
setPdfData(data);
} catch (error) {
setCompileError(error instanceof Error ? error.message : "Compilation failed");
setCompileError(error instanceof Error ? error.message : typeof error === "string" ? error : "Compilation failed");
} finally {
setIsCompiling(false);
}
@ -782,7 +782,7 @@ export function LatexEditor() {
)}
<ClaudeChatDrawer />
{/* Selection toolbar */}
{toolbarPosition && selectionLabel && !isMergeActiveRef.current && (
{toolbarPosition && selectionLabel && !isMergeActiveRef.current && !isSearchOpen && (
<SelectionToolbar
position={toolbarPosition}
contextLabel={selectionLabel}

View file

@ -60,10 +60,10 @@ export function PdfPreview() {
const files = useDocumentStore((s) => s.files);
const saveAllFiles = useDocumentStore((s) => s.saveAllFiles);
const setActiveFile = useDocumentStore((s) => s.setActiveFile);
const activeFileType = useDocumentStore((s) => {
const active = s.files.find((f) => f.id === s.activeFileId);
return active?.type ?? "tex";
const activeFile = useDocumentStore((s) => {
return s.files.find((f) => f.id === s.activeFileId) ?? null;
});
const activeFileType = activeFile?.type ?? "tex";
const isTexActive = activeFileType === "tex";
const requestJumpToPosition = useDocumentStore(
(s) => s.requestJumpToPosition,
@ -277,20 +277,21 @@ export function PdfPreview() {
setIsCompiling(true);
try {
await saveAllFiles();
const mainFile = files.find((f) => f.name === "document.tex" || f.name === "main.tex");
const mainFileName = mainFile?.relativePath || "document.tex";
const data = await compileLatex(projectRoot, mainFileName);
const targetFile = activeFile?.type === "tex"
? activeFile.relativePath
: (files.find((f) => f.name === "document.tex" || f.name === "main.tex")?.relativePath || "document.tex");
const data = await compileLatex(projectRoot, targetFile);
setPdfData(data);
} catch (error) {
setCompileError(
error instanceof Error ? error.message : "Compilation failed",
error instanceof Error ? error.message : typeof error === "string" ? error : "Compilation failed",
);
} finally {
setIsCompiling(false);
}
};
compile();
}, [initialized, projectRoot, pdfData, isCompiling, compileError, setIsCompiling, setPdfData, setCompileError, saveAllFiles, files]);
}, [initialized, projectRoot, pdfData, isCompiling, compileError, setIsCompiling, setPdfData, setCompileError, saveAllFiles, files, activeFile]);
const zoomIn = () => setScale((s) => Math.min(4, s + 0.1));
const zoomOut = () => setScale((s) => Math.max(0.25, s - 0.1));
@ -320,12 +321,13 @@ export function PdfPreview() {
setPdfError(null);
try {
await saveAllFiles();
const mainFile = files.find((f) => f.name === "document.tex" || f.name === "main.tex");
const mainFileName = mainFile?.relativePath || "document.tex";
const data = await compileLatex(projectRoot, mainFileName);
const targetFile = activeFile?.type === "tex"
? activeFile.relativePath
: (files.find((f) => f.name === "document.tex" || f.name === "main.tex")?.relativePath || "document.tex");
const data = await compileLatex(projectRoot, targetFile);
setPdfData(data);
} catch (error) {
setCompileError(error instanceof Error ? error.message : "Compilation failed");
setCompileError(error instanceof Error ? error.message : typeof error === "string" ? error : "Compilation failed");
} finally {
setIsCompiling(false);
}

View file

@ -244,22 +244,28 @@ export function useClaudeEvents() {
const docStore = useDocumentStore.getState();
await docStore.refreshFiles();
// Auto-recompile after Claude finishes — always attempt if a tex file exists.
// Auto-recompile after Claude finishes.
// Prefer the active file if it's a .tex file; fall back to document.tex / main.tex.
// Skip if another compilation is already in progress (e.g. initial compile).
const { projectRoot, files, isCompiling: alreadyCompiling } = useDocumentStore.getState();
const { projectRoot, files, activeFileId, isCompiling: alreadyCompiling } = useDocumentStore.getState();
if (projectRoot && !alreadyCompiling) {
const mainFile = files.find(
(f) => f.name === "document.tex" || f.name === "main.tex",
);
if (mainFile) {
const mainFileName = mainFile.relativePath;
const activeFile = files.find((f) => f.id === activeFileId);
const targetFile =
activeFile?.type === "tex"
? activeFile
: files.find((f) => f.name === "document.tex" || f.name === "main.tex");
if (targetFile) {
const mainFileName = targetFile.relativePath;
useDocumentStore.getState().setIsCompiling(true);
try {
// Flush any dirty files to disk before compiling so the
// compiler (which reads from disk) sees the latest content.
await useDocumentStore.getState().saveAllFiles();
const pdfData = await compileLatex(projectRoot, mainFileName);
useDocumentStore.getState().setPdfData(pdfData);
} catch (err) {
useDocumentStore.getState().setCompileError(
err instanceof Error ? err.message : "Compilation failed",
err instanceof Error ? err.message : typeof err === "string" ? err : "Compilation failed",
);
} finally {
useDocumentStore.getState().setIsCompiling(false);

View file

@ -95,7 +95,7 @@ function scheduleAutoSave() {
const store = storeRef;
if (!store) return;
const state = store.getState();
const dirtyFiles = state.files.filter((f) => f.isDirty && f.content);
const dirtyFiles = state.files.filter((f) => f.isDirty && f.content != null);
if (dirtyFiles.length > 0) {
await state.saveAllFiles();
}
@ -347,7 +347,7 @@ export const useDocumentStore = create<DocumentState>()((set, get) => ({
saveFile: async (id) => {
const state = get();
const file = state.files.find((f) => f.id === id);
if (!file || !file.isDirty || !file.content) return;
if (!file || !file.isDirty || file.content == null) return;
await writeTexFileContent(file.absolutePath, file.content);
set((s) => ({
@ -359,7 +359,7 @@ export const useDocumentStore = create<DocumentState>()((set, get) => ({
saveAllFiles: async () => {
const state = get();
const dirtyFiles = state.files.filter((f) => f.isDirty && f.content);
const dirtyFiles = state.files.filter((f) => f.isDirty && f.content != null);
const results = await Promise.allSettled(
dirtyFiles.map((f) => writeTexFileContent(f.absolutePath, f.content!)),
);