From 167a2b13a5fd73a14546f4be9c115e1a3dfe6535 Mon Sep 17 00:00:00 2001 From: Andrew Beal Date: Mon, 17 Nov 2025 19:02:15 +0000 Subject: [PATCH] refactor: standardize error handling with Exception helper and improve return types Add Exception helper class for consistent error handling and logging. Replace throw statements and console.error calls with Exception methods. Update service methods to return Error | T instead of mixed success/failure objects. Improve type safety in Claude.extractContents with explicit return type. Add WikiLinks helper to VaultCacheService for managing wiki link references. Update unit tests. --- AIClasses/Claude/Claude.ts | 2 +- .../Claude/ClaudeConversationNamingService.ts | 5 +- .../Gemini/GeminiConversationNamingService.ts | 5 +- AIClasses/IPrompt.ts | 12 +- AIClasses/OpenAI/OpenAI.ts | 29 ++-- .../OpenAI/OpenAIConversationNamingService.ts | 5 +- Components/TopBar.svelte | 9 +- Enums/AIFunction.ts | 4 +- Enums/ApiProvider.ts | 4 +- Enums/SearchTrigger.ts | 3 +- Helpers/Exception.ts | 32 ++++ Helpers/Semaphore.ts | 2 +- Helpers/WikiLinks.ts | 39 +++++ Modals/ConversationHistoryModal.ts | 15 +- Modals/HelpModal.ts | 2 +- Services/AIFunctionService.ts | 33 +++-- Services/ChatService.ts | 20 ++- Services/ConversationFileSystemService.ts | 58 +++++--- Services/ConversationNamingService.ts | 26 +++- Services/DependencyService.ts | 10 +- Services/FileSystemService.ts | 56 +++---- Services/HTMLService.ts | 4 +- Services/InputService.ts | 12 +- Services/SanitiserService.ts | 5 - Services/SettingsService.ts | 2 +- Services/StreamingMarkdownService.ts | 21 +-- Services/StreamingService.ts | 20 ++- Services/VaultCacheService.ts | 9 ++ Services/VaultService.ts | 107 ++++++++------ VaultkeeperAISettingTab.ts | 4 +- __tests__/AIClasses/OpenAI.test.ts | 38 +++-- __tests__/Services/AIFunctionService.test.ts | 63 ++++---- .../ConversationFileSystemService.test.ts | 53 ++++--- .../ConversationNamingService.test.ts | 15 +- __tests__/Services/FileSystemService.test.ts | 138 +++++++----------- __tests__/Services/SanitiserService.test.ts | 7 +- .../Services/StreamingMarkdownService.test.ts | 40 +++-- __tests__/Services/StreamingService.test.ts | 7 + __tests__/Services/VaultService.test.ts | 61 ++++---- 39 files changed, 556 insertions(+), 421 deletions(-) create mode 100644 Helpers/Exception.ts create mode 100644 Helpers/WikiLinks.ts diff --git a/AIClasses/Claude/Claude.ts b/AIClasses/Claude/Claude.ts index dcdeadd..62ff20e 100644 --- a/AIClasses/Claude/Claude.ts +++ b/AIClasses/Claude/Claude.ts @@ -161,7 +161,7 @@ export class Claude implements IAIClass { } } - private extractContents(conversationContent: ConversationContent[]) { + private extractContents(conversationContent: ConversationContent[]): { role: Role; content: ContentBlockParam[]; }[] { return conversationContent.filter(content => content.content.trim() !== "" || content.functionCall.trim() !== "") .map(content => { const contentBlocks: ContentBlockParam[] = []; diff --git a/AIClasses/Claude/ClaudeConversationNamingService.ts b/AIClasses/Claude/ClaudeConversationNamingService.ts index b422c93..4481612 100644 --- a/AIClasses/Claude/ClaudeConversationNamingService.ts +++ b/AIClasses/Claude/ClaudeConversationNamingService.ts @@ -6,6 +6,7 @@ import { Role } from "Enums/Role"; import { NamePrompt } from "AIClasses/NamePrompt"; import type { SettingsService } from "Services/SettingsService"; import type Anthropic from '@anthropic-ai/sdk'; +import { Exception } from "Helpers/Exception"; export class ClaudeConversationNamingService implements IConversationNamingService { @@ -41,14 +42,14 @@ export class ClaudeConversationNamingService implements IConversationNamingServi }); if (!response.ok) { - throw new Error(`Claude API error: ${response.status} ${response.statusText} - ${await response.text()}`); + Exception.throw(`Claude API error: ${response.status} ${response.statusText} - ${await response.text()}`); } const data = await response.json() as Anthropic.Messages.Message; const firstContent = data.content?.[0]; if (!firstContent || firstContent.type !== 'text') { - throw new Error("Failed to generate conversation name"); + Exception.throw("Failed to generate conversation name"); } return firstContent.text; diff --git a/AIClasses/Gemini/GeminiConversationNamingService.ts b/AIClasses/Gemini/GeminiConversationNamingService.ts index a7f91d8..98e6c8e 100644 --- a/AIClasses/Gemini/GeminiConversationNamingService.ts +++ b/AIClasses/Gemini/GeminiConversationNamingService.ts @@ -6,6 +6,7 @@ import { Role } from "Enums/Role"; import { NamePrompt } from "AIClasses/NamePrompt"; import type { GenerateContentResponse } from "@google/genai"; import type { SettingsService } from "Services/SettingsService"; +import { Exception } from "Helpers/Exception"; export class GeminiConversationNamingService implements IConversationNamingService { @@ -38,14 +39,14 @@ export class GeminiConversationNamingService implements IConversationNamingServi }); if (!response.ok) { - throw new Error(`Gemini API error: ${response.status} ${response.statusText} - ${await response.text()}`); + Exception.throw(`Gemini API error: ${response.status} ${response.statusText} - ${await response.text()}`); } const data = await response.json() as GenerateContentResponse; const generatedName = data.candidates?.[0]?.content?.parts?.[0]?.text; if (!generatedName) { - throw new Error("Failed to generate conversation name"); + Exception.throw("Failed to generate conversation name"); } return generatedName; diff --git a/AIClasses/IPrompt.ts b/AIClasses/IPrompt.ts index a258b6c..28ba3af 100644 --- a/AIClasses/IPrompt.ts +++ b/AIClasses/IPrompt.ts @@ -1,9 +1,9 @@ -import type VaultkeeperAIPlugin from "main"; import { Resolve } from "Services/DependencyService"; import { Services } from "Services/Services"; import { SystemInstruction } from "./SystemPrompt"; import type { FileSystemService } from "Services/FileSystemService"; import type { SettingsService } from "Services/SettingsService"; +import { Notice } from "obsidian"; export interface IPrompt { systemInstruction(): string; @@ -12,12 +12,10 @@ export interface IPrompt { export class AIPrompt implements IPrompt { - private readonly plugin: VaultkeeperAIPlugin; private readonly settingsService: SettingsService; private readonly fileSystemService: FileSystemService; public constructor() { - this.plugin = Resolve(Services.VaultkeeperAIPlugin); this.settingsService = Resolve(Services.SettingsService); this.fileSystemService = Resolve(Services.FileSystemService); } @@ -27,7 +25,11 @@ export class AIPrompt implements IPrompt { } public async userInstruction(): Promise { - const userInstruction: string | null = await this.fileSystemService.readFile(this.settingsService.settings.userInstruction, true); - return userInstruction ?? ""; + const result = await this.fileSystemService.readFile(this.settingsService.settings.userInstruction, true); + if (result instanceof Error) { + new Notice("Failed to load user instructions!"); + return ""; + } + return result; } } \ No newline at end of file diff --git a/AIClasses/OpenAI/OpenAI.ts b/AIClasses/OpenAI/OpenAI.ts index fa2bf08..5b7e36a 100644 --- a/AIClasses/OpenAI/OpenAI.ts +++ b/AIClasses/OpenAI/OpenAI.ts @@ -15,6 +15,7 @@ import type { SettingsService } from "Services/SettingsService"; import type { StoredFunctionCall, StoredFunctionResponse } from "AIClasses/Schemas/AIFunctionTypes"; import { StringTools } from "Helpers/StringTools"; import type { ResponseEvent, ResponseOutputTextDelta, ResponseFunctionCallArgumentsDone, ResponseDone, OpenAIFunctionTool } from "./OpenAITypes"; +import { Exception } from "Helpers/Exception"; export class OpenAI implements IAIClass { @@ -126,7 +127,7 @@ export class OpenAI implements IAIClass { // When we receive a function call, we should continue the conversation shouldContinue = true; } catch (error) { - console.error("Failed to parse function call arguments:", error); + Exception.log(error); } } break; @@ -157,8 +158,7 @@ export class OpenAI implements IAIClass { break; default: - // Unknown event type - log but don't error - console.debug("Unknown event type:", event.type); + Exception.log(`Unknown event type: ${event.type}`); break; } @@ -169,9 +169,8 @@ export class OpenAI implements IAIClass { shouldContinue: shouldContinue, }; } catch (error) { - const message = error instanceof Error ? error.message : "Unknown parsing error"; - console.error("Failed to parse stream chunk:", message, "Chunk:", chunk); - return { content: "", isComplete: false, error: `Failed to parse chunk: ${message}` }; + Exception.log(error); + return { content: "", isComplete: false, error: Exception.messageFrom(error) }; } } @@ -200,18 +199,17 @@ export class OpenAI implements IAIClass { ] }; } catch (error) { - console.error("Failed to parse function call:", error); - // Fall back to regular message - return { + Exception.log(error); + return { // Fall back to regular message role: content.role, - content: contentToExtract || "Error parsing function call" + content: contentToExtract.trim() !== "" ? contentToExtract : "Error parsing function call" }; } } else { - console.error("Invalid JSON in functionCall field"); + Exception.log("Invalid JSON in functionCall field"); return { role: content.role, - content: contentToExtract || "Error parsing function call" + content: contentToExtract.trim() !== "" ? contentToExtract : "Error parsing function call" }; } } @@ -227,15 +225,14 @@ export class OpenAI implements IAIClass { content: JSON.stringify(parsedContent.functionResponse.response) }; } catch (error) { - console.error("Failed to parse function response:", error); - // Fall back to regular message - return { + Exception.log(error); + return { // Fall back to regular message role: content.role, content: contentToExtract }; } } else { - console.error("Invalid JSON in function response content"); + Exception.log("Invalid JSON in function response content"); return { role: content.role, content: contentToExtract diff --git a/AIClasses/OpenAI/OpenAIConversationNamingService.ts b/AIClasses/OpenAI/OpenAIConversationNamingService.ts index 203683f..1c6fc7d 100644 --- a/AIClasses/OpenAI/OpenAIConversationNamingService.ts +++ b/AIClasses/OpenAI/OpenAIConversationNamingService.ts @@ -6,6 +6,7 @@ import { Role } from "Enums/Role"; import { NamePrompt } from "AIClasses/NamePrompt"; import type { SettingsService } from "Services/SettingsService"; import type OpenAI from "openai"; +import { Exception } from "Helpers/Exception"; export class OpenAIConversationNamingService implements IConversationNamingService { @@ -42,7 +43,7 @@ export class OpenAIConversationNamingService implements IConversationNamingServi }); if (!response.ok) { - throw new Error(`OpenAI API error: ${response.status} ${response.statusText} - ${await response.text()}`); + Exception.throw(`OpenAI API error: ${response.status} ${response.statusText} - ${await response.text()}`); } const data = await response.json() as OpenAI.Responses.Response; @@ -61,7 +62,7 @@ export class OpenAIConversationNamingService implements IConversationNamingServi : undefined; if (!generatedName) { - throw new Error("Failed to generate conversation name"); + Exception.throw("Failed to generate conversation name"); } return generatedName; diff --git a/Components/TopBar.svelte b/Components/TopBar.svelte index 7d1f737..7c89653 100644 --- a/Components/TopBar.svelte +++ b/Components/TopBar.svelte @@ -2,7 +2,7 @@ import { Resolve } from "../Services/DependencyService"; import { Services } from "../Services/Services"; import type VaultkeeperAIPlugin from "../main"; - import { setIcon, type WorkspaceLeaf } from "obsidian"; + import { Notice, setIcon, type WorkspaceLeaf } from "obsidian"; import { ConversationFileSystemService } from "../Services/ConversationFileSystemService"; import { conversationStore } from "../Stores/ConversationStore"; import type { ConversationHistoryModal } from "Modals/ConversationHistoryModal"; @@ -34,7 +34,12 @@ async function deleteCurrentConversation() { chatService.stop(); - await conversationFileSystemService.deleteCurrentConversation(); + const result = await conversationFileSystemService.deleteCurrentConversation(); + + if (result instanceof Error) { + new Notice(`Failed to delete conversation data for '${conversationFileSystemService.getCurrentConversationPath()}'`); + } + conversationStore.reset(); onNewConversation?.(); conversationTitle = ""; diff --git a/Enums/AIFunction.ts b/Enums/AIFunction.ts index ebd5d86..9777c55 100644 --- a/Enums/AIFunction.ts +++ b/Enums/AIFunction.ts @@ -1,3 +1,5 @@ +import { Exception } from "Helpers/Exception"; + export enum AIFunction { SearchVaultFiles = "search_vault_files", ReadVaultFiles = "read_vault_files", @@ -15,5 +17,5 @@ export function fromString(functionName: string): AIFunction { if (enumValue) { return enumValue as AIFunction; } - throw new Error(`Unknown function name: ${functionName}`); + Exception.throw(`Unknown function name: ${functionName}`); } \ No newline at end of file diff --git a/Enums/ApiProvider.ts b/Enums/ApiProvider.ts index 720ba41..d4c58c4 100644 --- a/Enums/ApiProvider.ts +++ b/Enums/ApiProvider.ts @@ -1,3 +1,5 @@ +import { Exception } from "Helpers/Exception"; + export enum AIProvider { Claude = "Claude", Gemini = "Gemini", @@ -12,7 +14,7 @@ export function fromModel(model: string): AIProvider { } else if (model.startsWith("gpt-")) { return AIProvider.OpenAI; } else { - throw new Error("Invalid Model Selection!"); + Exception.throw("Invalid Model Selection!"); } } diff --git a/Enums/SearchTrigger.ts b/Enums/SearchTrigger.ts index d83f80d..6583568 100644 --- a/Enums/SearchTrigger.ts +++ b/Enums/SearchTrigger.ts @@ -3,6 +3,7 @@ import { setTooltip } from "obsidian"; import type { HTMLService } from "Services/HTMLService"; import { Resolve } from "Services/DependencyService"; import { Services } from "Services/Services"; +import { Exception } from "Helpers/Exception"; export enum SearchTrigger { Tag = "#", @@ -35,7 +36,7 @@ export function fromInput(input: string): SearchTrigger { case "/": return SearchTrigger.Folder; default: - throw new Error(`Unknown search trigger: ${input}`); + Exception.throw(`Unknown search trigger: ${input}`); } } diff --git a/Helpers/Exception.ts b/Helpers/Exception.ts new file mode 100644 index 0000000..059bf2d --- /dev/null +++ b/Helpers/Exception.ts @@ -0,0 +1,32 @@ +export abstract class Exception { + + public static throw(error: unknown): never { + this.log(error); + throw error; + } + + public static new(error: unknown): Error { + return error instanceof Error ? error : new Error(this.messageFrom(error)); + } + + public static log(error: unknown) { + if (process.env.NODE_ENV !== "production") { + const e: Error = this.new(error); + console.error(e.message, e); + } + } + + public static messageFrom(error: unknown): string { + if (error instanceof Error) { + return error.message; + } + if (typeof error === "string") { + return error; + } + if (error && typeof error === "object" && "message" in error) { + return String(error.message); + } + return String(error); + } + +} \ No newline at end of file diff --git a/Helpers/Semaphore.ts b/Helpers/Semaphore.ts index 3159462..155b47a 100644 --- a/Helpers/Semaphore.ts +++ b/Helpers/Semaphore.ts @@ -26,7 +26,7 @@ export class Semaphore { }); } - release(): void { + release() { if (this.queue.length > 0) { const resolve = this.queue.shift(); if (resolve) { diff --git a/Helpers/WikiLinks.ts b/Helpers/WikiLinks.ts new file mode 100644 index 0000000..f539a0d --- /dev/null +++ b/Helpers/WikiLinks.ts @@ -0,0 +1,39 @@ +import { TFile } from "obsidian"; + +export class WikiLinks { + + public links: string[] = []; + + public addWikiLink(file: TFile) { + if (file.extension === "md") { + this.links.push(this.asWikiLink(file)); + } + } + + public removeWikiLink(file: TFile | string) { + if (file instanceof TFile) { + if (file.extension === "md") { + this.removeFromLinks(this.asWikiLink(file)); + } + } else { + if (file.endsWith(".md")) { + this.removeFromLinks(this.asWikiLink(file)); + } + } + } + + private asWikiLink(file: TFile | string) { + if (file instanceof TFile) { + return file.path.replace(/\.md$/, ""); + } + return file.replace(/\.md$/, ""); + } + + private removeFromLinks(wikiLink: string) { + const index = this.links.indexOf(wikiLink); + if (index !== -1) { + this.links.splice(index, 1); + } + } + +} \ No newline at end of file diff --git a/Modals/ConversationHistoryModal.ts b/Modals/ConversationHistoryModal.ts index 30f6165..cb66ced 100644 --- a/Modals/ConversationHistoryModal.ts +++ b/Modals/ConversationHistoryModal.ts @@ -1,4 +1,4 @@ -import { Modal } from 'obsidian'; +import { Modal, Notice } from 'obsidian'; import ConversationHistoryModalSvelte from './ConversationHistoryModalSvelte.svelte'; import type { Conversation } from 'Conversations/Conversation'; import { mount, unmount } from 'svelte'; @@ -94,14 +94,21 @@ export class ConversationHistoryModal extends Modal { let shouldResetChat = false; const currentPath = this.conversationFileSystemService.getCurrentConversationPath(); + const deletedIds: string[] = []; for (const item of itemsToDelete) { - const deleted = await this.fileSystemService.deleteFile(item.filePath, true); - if (deleted && currentPath === item.filePath) { + const result = await this.fileSystemService.deleteFile(item.filePath, true); + if (result instanceof Error) { + new Notice(`Failed to delete conversation '${item.title}'`); + continue; + } + deletedIds.push(item.id); + + if (currentPath === item.filePath) { shouldResetChat = true; } } - this.items = this.items.filter(item => !itemIds.includes(item.id)); + this.items = this.items.filter(item => !deletedIds.includes(item.id)); if (this.component) { this.component.items = this.items; diff --git a/Modals/HelpModal.ts b/Modals/HelpModal.ts index bd02d69..5646f04 100644 --- a/Modals/HelpModal.ts +++ b/Modals/HelpModal.ts @@ -31,7 +31,7 @@ export class HelpModal extends Modal { }); } - public open(initialTopic?: number): void { + public open(initialTopic?: number) { this.initialTopic = initialTopic; super.open(); } diff --git a/Services/AIFunctionService.ts b/Services/AIFunctionService.ts index d4533de..922ff32 100644 --- a/Services/AIFunctionService.ts +++ b/Services/AIFunctionService.ts @@ -132,33 +132,35 @@ export class AIFunctionService { private async readVaultFiles(filePaths: string[]): Promise { const results = await Promise.all( filePaths.map(async (filePath) => { - const content = await this.fileSystemService.readFile(filePath); - if (content === null) { - return { path: filePath, success: false as const, error: `File not found: ${filePath}` }; + const result = await this.fileSystemService.readFile(filePath); + if (result instanceof Error) { + return { path: filePath, error: result } } - return { path: filePath, success: true as const, content }; + return { path: filePath, contents: result } }) ); return { results }; } private async writeVaultFile(filePath: string, content: string): Promise { - const result: Error | undefined = await this.fileSystemService.writeFile(normalizePath(filePath), content); - return result === undefined ? { success: true } : { success: false, error: result }; + const result = await this.fileSystemService.writeFile(normalizePath(filePath), content); + if (result instanceof Error) { + return { success: false, error: result }; + } + return { success: true }; } - private async deleteVaultFiles(filepaths: string[], confirmation: boolean): Promise { + private async deleteVaultFiles(filePaths: string[], confirmation: boolean): Promise { if (!confirmation) { return { error: "Confirmation was false, no action taken" }; } - const results = await Promise.all(filepaths.map(async filePath => { + const results = await Promise.all(filePaths.map(async filePath => { const result = await this.fileSystemService.deleteFile(filePath); - if (result.success) { - return { path: filePath, success: true as const }; - } else { - return { path: filePath, success: false as const, error: result.error }; + if (result instanceof Error) { + return { path: filePath, success: false, error: result } } + return { path: filePath, success: true }; })); return { results }; @@ -172,11 +174,10 @@ export class AIFunctionService { const results = await Promise.all(sourcePaths.map(async (sourcePath, index) => { const destinationPath = destinationPaths[index]; const result = await this.fileSystemService.moveFile(sourcePath, destinationPath); - if (result.success) { - return { path: destinationPath, success: true as const }; - } else { - return { path: destinationPath, success: false as const, error: result.error }; + if (result instanceof Error) { + return { path: destinationPath, success: false, error: result } } + return { path: destinationPath, success: true }; })); return { results }; diff --git a/Services/ChatService.ts b/Services/ChatService.ts index 9f7b99d..9c1a66e 100644 --- a/Services/ChatService.ts +++ b/Services/ChatService.ts @@ -12,6 +12,7 @@ import { Conversation } from "Conversations/Conversation"; import { ConversationContent } from "Conversations/ConversationContent"; import { Role } from "Enums/Role"; import type { AIFunctionCall } from "AIClasses/AIFunctionCall"; +import { Notice } from "obsidian"; export interface IChatServiceCallbacks { onSubmit: () => void; @@ -49,7 +50,7 @@ export class ChatService { this.tokenService = Resolve(Services.ITokenService); } - public async submit(conversation: Conversation, allowDestructiveActions: boolean, userRequest: string, formattedRequest: string, callbacks: IChatServiceCallbacks): Promise { + public async submit(conversation: Conversation, allowDestructiveActions: boolean, userRequest: string, formattedRequest: string, callbacks: IChatServiceCallbacks) { if (!await this.semaphore.wait()) { return; } @@ -64,7 +65,7 @@ export class ChatService { this.abortController = new AbortController(); conversation.contents.push(new ConversationContent(Role.User, userRequest, formattedRequest)); - await this.conversationService.saveConversation(conversation); + await this.saveConversation(conversation); callbacks.onSubmit(); callbacks.onStreamingUpdate(null); @@ -95,7 +96,7 @@ export class ChatService { response = await this.streamRequestResponse(conversation, allowDestructiveActions, callbacks); } } finally { - await this.conversationService.saveConversation(conversation); + await this.saveConversation(conversation); this.abortController = null; if (this.semaphoreHeld) { this.semaphoreHeld = false; @@ -106,7 +107,7 @@ export class ChatService { } } - public stop(): void { + public stop() { if (this.abortController) { this.abortController.abort(); this.abortController = null; @@ -114,7 +115,7 @@ export class ChatService { this.semaphore.release(); } - public async updateTokenDisplay(conversation: Conversation): Promise { + public async updateTokenDisplay(conversation: Conversation) { if (this.tokenService === undefined) { return; } @@ -139,10 +140,17 @@ export class ChatService { this.setStatusBarTokens(inputTokens, outputTokens); } - public setStatusBarTokens(inputTokens: number, outputTokens: number): void { + public setStatusBarTokens(inputTokens: number, outputTokens: number) { this.statusBarService.animateTokens(inputTokens, outputTokens); } + private async saveConversation(conversation: Conversation) { + const result = await this.conversationService.saveConversation(conversation); + if (result instanceof Error) { + new Notice(`Failed to save conversation data for '${conversation.title}'`); + } + } + private ensureCorrectConversationStructure(conversation: Conversation) { // Check if the last message is from the assistant to prevent assistant-to-assistant structure // This can happen when the assistant's last message had no function call and the user sends a new request diff --git a/Services/ConversationFileSystemService.ts b/Services/ConversationFileSystemService.ts index 04b62dd..6fc47d1 100644 --- a/Services/ConversationFileSystemService.ts +++ b/Services/ConversationFileSystemService.ts @@ -5,6 +5,7 @@ import { Services } from "./Services"; import { Conversation } from "Conversations/Conversation"; import { ConversationContent } from "Conversations/ConversationContent"; import { Copy } from "Enums/Copy"; +import { Exception } from "Helpers/Exception"; export class ConversationFileSystemService { @@ -19,7 +20,7 @@ export class ConversationFileSystemService { return `${Path.Conversations}/${conversation.title}.json`; } - public async saveConversation(conversation: Conversation): Promise { + public async saveConversation(conversation: Conversation): Promise { if (!this.currentConversationPath) { this.currentConversationPath = this.generateConversationPath(conversation); } @@ -44,11 +45,16 @@ export class ConversationFileSystemService { })) }; - await this.fileSystemService.writeObjectToFile(this.currentConversationPath, conversationData, true); + const result = await this.fileSystemService.writeObjectToFile(this.currentConversationPath, conversationData, true); + + if (result instanceof Error) { + return result; + } + return this.currentConversationPath; } - public resetCurrentConversation(): void { + public resetCurrentConversation() { this.currentConversationPath = null; } @@ -56,22 +62,22 @@ export class ConversationFileSystemService { return this.currentConversationPath; } - public setCurrentConversationPath(filePath: string): void { + public setCurrentConversationPath(filePath: string) { this.currentConversationPath = filePath; } - public async deleteCurrentConversation(): Promise { + public async deleteCurrentConversation(): Promise { if (!this.currentConversationPath) { - return false; + return; } - const deleted = await this.fileSystemService.deleteFile(this.currentConversationPath, true); + const result = await this.fileSystemService.deleteFile(this.currentConversationPath, true); - if (deleted.success) { - this.resetCurrentConversation(); + if (result instanceof Error) { + return result; } - return deleted.success; + this.resetCurrentConversation(); } public async getAllConversations(): Promise { @@ -79,15 +85,27 @@ export class ConversationFileSystemService { const conversations: Conversation[] = []; for (const file of files) { - const data = await this.fileSystemService.readObjectFromFile(file.path, true); - if (Conversation.isConversationData(data)) { + const result = await this.fileSystemService.readObjectFromFile(file.path, true); + if (result instanceof Error) { + Exception.log(`Failed to load conversation: ${file.path}`); + continue; + } + if (Conversation.isConversationData(result)) { const conversation: Conversation = new Conversation(); - conversation.title = data.title; - conversation.created = new Date(data.created); - conversation.updated = new Date(data.updated); - conversation.contents = data.contents.map(content => { + conversation.title = result.title; + conversation.created = new Date(result.created); + conversation.updated = new Date(result.updated); + conversation.contents = result.contents.map(content => { return new ConversationContent( - content.role, content.content, content.promptContent, content.functionCall, new Date(content.timestamp), content.isFunctionCall, content.isFunctionCallResponse, content.toolId); + content.role, + content.content, + content.promptContent, + content.functionCall, + new Date(content.timestamp), + content.isFunctionCall, + content.isFunctionCallResponse, + content.toolId + ); }); conversations.push(conversation); } @@ -96,13 +114,13 @@ export class ConversationFileSystemService { return conversations; } - public async updateConversationTitle(oldPath: string, newTitle: string): Promise { + public async updateConversationTitle(oldPath: string, newTitle: string): Promise { const newPath = `${Path.Conversations}/${newTitle}.json`; const result = await this.fileSystemService.moveFile(oldPath, newPath, true); - if (!result.success) { - throw new Error(`Failed to update conversation title: ${result.error}`); + if (result instanceof Error) { + return result; } if (this.currentConversationPath === oldPath) { diff --git a/Services/ConversationNamingService.ts b/Services/ConversationNamingService.ts index 6b3b2b2..12e0a87 100644 --- a/Services/ConversationNamingService.ts +++ b/Services/ConversationNamingService.ts @@ -5,6 +5,8 @@ import type { ConversationFileSystemService } from "./ConversationFileSystemServ import type { Conversation } from "Conversations/Conversation"; import type { VaultService } from "./VaultService"; import { Path } from "Enums/Path"; +import { Exception } from "Helpers/Exception"; +import { Notice } from "obsidian"; export class ConversationNamingService { private readonly stackLimit: number = 1000; @@ -22,7 +24,7 @@ export class ConversationNamingService { this.namingProvider = Resolve(Services.IConversationNamingService); } - public async requestName(conversation: Conversation, userPrompt: string, onNameChanged: ((name: string) => void) | undefined, abortController: AbortController): Promise { + public async requestName(conversation: Conversation, userPrompt: string, onNameChanged: ((name: string) => void) | undefined, abortController: AbortController) { if (!this.namingProvider) { return; } @@ -42,15 +44,23 @@ export class ConversationNamingService { return; } - await this.conversationService.updateConversationTitle(conversationPath, validatedName); + const updateResult = await this.conversationService.updateConversationTitle(conversationPath, validatedName); + + if (updateResult instanceof Error) { + Exception.throw(updateResult); + } + conversation.title = validatedName; - await this.conversationService.saveConversation(conversation); + const saveResult = await this.conversationService.saveConversation(conversation); + + if (saveResult instanceof Error) { + Exception.throw(saveResult); + } + onNameChanged?.(conversation.title); } catch (error) { - if (error instanceof Error && error.name === 'AbortError') { - return; - } - console.error("Failed to generate name:", error); + Exception.log(error); + new Notice(`Failed to name conversation '${conversation.title}'`); } } @@ -64,7 +74,7 @@ export class ConversationNamingService { index++; if (index > this.stackLimit) { - throw new Error(`Stack limit reached when trying to generate conversation name for "${cleanedTitle}"`); + Exception.throw(`Stack limit reached when trying to generate conversation name for "${cleanedTitle}"`); } } return availableTitle; diff --git a/Services/DependencyService.ts b/Services/DependencyService.ts index dea1bfb..c32eb96 100644 --- a/Services/DependencyService.ts +++ b/Services/DependencyService.ts @@ -1,17 +1,19 @@ +import { Exception } from "Helpers/Exception"; + const services = new Map(); -export function RegisterSingleton(type: symbol, instance: T): void { +export function RegisterSingleton(type: symbol, instance: T) { services.set(type, instance); } -export function RegisterTransient(type: symbol, factory: () => T): void { +export function RegisterTransient(type: symbol, factory: () => T) { services.set(type, factory); } export function Resolve(type: symbol): T { const service = services.get(type); if (!service) { - throw new Error(`Service not found for type: ${type.description}`); + Exception.throw(`Service not found for type: ${type.description}`); } if (typeof service === 'function') { @@ -23,6 +25,6 @@ export function Resolve(type: symbol): T { return service as T; } -export function DeregisterAllServices(): void { +export function DeregisterAllServices() { services.clear(); } \ No newline at end of file diff --git a/Services/FileSystemService.ts b/Services/FileSystemService.ts index d34ce5c..6dfdb1b 100644 --- a/Services/FileSystemService.ts +++ b/Services/FileSystemService.ts @@ -3,7 +3,7 @@ import { Resolve } from "./DependencyService"; import { Services } from "./Services"; import type { VaultService } from "./VaultService"; import type { ISearchMatch } from "../Helpers/SearchTypes"; -import { StringTools } from "Helpers/StringTools"; +import { Exception } from "Helpers/Exception"; export class FileSystemService { @@ -13,47 +13,39 @@ export class FileSystemService { this.vaultService = Resolve(Services.VaultService); } - public getVaultFileListForMarkDown() { - const files: TFile[] = this.vaultService.getMarkdownFiles(); - return files.map(file => { - return file.path.replace(/\.md$/, ""); - }); - } - - public async readFile(filePath: string, allowAccessToPluginRoot: boolean = false): Promise { + public async readFile(filePath: string, allowAccessToPluginRoot: boolean = false): Promise { const file: TAbstractFile | null = this.vaultService.getAbstractFileByPath(filePath, allowAccessToPluginRoot); if (file && file instanceof TFile) { return await this.vaultService.read(file, allowAccessToPluginRoot); } - return null; + return Exception.new(`Path is a folder, not a file: ${filePath}`); } - public async writeFile(filePath: string, content: string, allowAccessToPluginRoot: boolean = false): Promise { + public async writeFile(filePath: string, content: string, allowAccessToPluginRoot: boolean = false): Promise { try { const file: TAbstractFile | null = this.vaultService.getAbstractFileByPath(filePath, allowAccessToPluginRoot); if (file == null || !(file instanceof TFile)) { - await this.vaultService.create(filePath, content, allowAccessToPluginRoot); - return; + return await this.vaultService.create(filePath, content, allowAccessToPluginRoot); } - await this.vaultService.modify(file, content, allowAccessToPluginRoot); + return await this.vaultService.modify(file, content, allowAccessToPluginRoot); } catch (error) { - console.error("Error writing file:", error); - return error instanceof Error ? error : new Error(String(error)); + Exception.log(error); + return Exception.new(error); } } - public async deleteFile(filePath: string, allowAccessToPluginRoot: boolean = false): Promise<{ success: true } | { success: false, error: string }> { + public async deleteFile(filePath: string, allowAccessToPluginRoot: boolean = false): Promise { const file: TAbstractFile | null = this.vaultService.getAbstractFileByPath(filePath, allowAccessToPluginRoot); if (!file) { - return { success: false, error: "File not found" }; + return Exception.new(`File does not exist: ${filePath}`); } return await this.vaultService.delete(file, allowAccessToPluginRoot); } - public async moveFile(sourcePath: string, destinationPath: string, allowAccessToPluginRoot: boolean = false): Promise<{ success: true } | { success: false, error: string }> { + public async moveFile(sourcePath: string, destinationPath: string, allowAccessToPluginRoot: boolean = false): Promise { return await this.vaultService.move(sourcePath, destinationPath, allowAccessToPluginRoot); } @@ -69,33 +61,27 @@ export class FileSystemService { return await this.vaultService.listDirectoryContents(dirPath, recursive, allowAccessToPluginRoot); } - public async readObjectFromFile(filePath: string, allowAccessToPluginRoot: boolean = false): Promise { + public async readObjectFromFile(filePath: string, allowAccessToPluginRoot: boolean = false): Promise { const file: TAbstractFile | null = this.vaultService.getAbstractFileByPath(filePath, allowAccessToPluginRoot); if (file && file instanceof TFile) { - const content = await this.vaultService.read(file, allowAccessToPluginRoot); - if (StringTools.isValidJson(content) === true) { - return JSON.parse(content) as object; - } + const result = await this.vaultService.read(file, allowAccessToPluginRoot); + return typeof result === "string" ? JSON.parse(result) as object : result; } - return null; + return Exception.new(`File not found: ${filePath}`); } - public async writeObjectToFile(filePath: string, data: object, allowAccessToPluginRoot: boolean = false): Promise { - try { - const file: TAbstractFile | null = this.vaultService.getAbstractFileByPath(filePath, allowAccessToPluginRoot); + public async writeObjectToFile(filePath: string, data: object, allowAccessToPluginRoot: boolean = false): Promise { + const file: TAbstractFile | null = this.vaultService.getAbstractFileByPath(filePath, allowAccessToPluginRoot); + let result: TFile | Error; if (file && file instanceof TFile) { - await this.vaultService.modify(file, JSON.stringify(data, null, 4), allowAccessToPluginRoot); + result = await this.vaultService.modify(file, JSON.stringify(data, null, 4), allowAccessToPluginRoot); } else { - await this.vaultService.create(filePath, JSON.stringify(data, null, 4), allowAccessToPluginRoot); + result = await this.vaultService.create(filePath, JSON.stringify(data, null, 4), allowAccessToPluginRoot); } - return true; - } catch (error) { - console.error("Error writing JSON file:", error); - return false; - } + return result; } public async searchVaultFiles(searchTerm: string, allowAccessToPluginRoot: boolean = false): Promise { diff --git a/Services/HTMLService.ts b/Services/HTMLService.ts index 0900a50..d865c8a 100644 --- a/Services/HTMLService.ts +++ b/Services/HTMLService.ts @@ -1,10 +1,10 @@ export class HTMLService { - public clearElement(element: HTMLElement): void { + public clearElement(element: HTMLElement) { element.empty(); } - public setHTMLContent(container: HTMLElement, htmlString: string): void { + public setHTMLContent(container: HTMLElement, htmlString: string) { this.clearElement(container); const fragment = this.parseHTMLString(htmlString); container.appendChild(fragment); diff --git a/Services/InputService.ts b/Services/InputService.ts index d2d304b..7b32fa0 100644 --- a/Services/InputService.ts +++ b/Services/InputService.ts @@ -9,7 +9,7 @@ export class InputService { return clipboardData.getData("text/plain") || ""; } - public sanitizeToPlainText(element: HTMLElement): void { + public sanitizeToPlainText(element: HTMLElement) { const plainText = element.textContent || ""; const cursorPos = this.getCursorPosition(element); @@ -250,7 +250,7 @@ export class InputService { return null; } - public insertTextAtCursor(text: string, element?: HTMLElement): void { + public insertTextAtCursor(text: string, element?: HTMLElement) { if (element && !element.isContentEditable) { console.warn("Element must be contenteditable"); return; @@ -273,7 +273,7 @@ export class InputService { selection.addRange(range); } - public insertElementAtCursor(node: Node, element?: HTMLElement): void { + public insertElementAtCursor(node: Node, element?: HTMLElement) { if (element && !element.isContentEditable) { console.warn("Element must be contenteditable"); return; @@ -297,7 +297,7 @@ export class InputService { selection.addRange(range); } - public deleteTextRange(startPos: number, endPos: number, element: HTMLElement): void { + public deleteTextRange(startPos: number, endPos: number, element: HTMLElement) { if (!element.isContentEditable) { console.warn("Element must be contenteditable"); return; @@ -354,7 +354,7 @@ export class InputService { * Ensures the cursor is not positioned inside a contentEditable="false" element. * If it is, repositions the cursor to a valid location. */ - private ensureCursorNotInNonEditableElement(element: HTMLElement): void { + private ensureCursorNotInNonEditableElement(element: HTMLElement) { const selection = window.getSelection(); if (!selection || selection.rangeCount === 0) { return; @@ -380,7 +380,7 @@ export class InputService { /** * Positions the cursor immediately after the given element. */ - private positionCursorAfterElement(targetElement: HTMLElement, container: HTMLElement): void { + private positionCursorAfterElement(targetElement: HTMLElement, container: HTMLElement) { const selection = window.getSelection(); if (!selection) { return; diff --git a/Services/SanitiserService.ts b/Services/SanitiserService.ts index 337de9d..786363c 100644 --- a/Services/SanitiserService.ts +++ b/Services/SanitiserService.ts @@ -25,11 +25,6 @@ export class SanitiserService { * @returns Sanitized file path */ public sanitize(input: string, options: ISanitizeOptions = {}): string { - // Type check - if (typeof input !== "string") { - throw new Error("Input must be a string"); - } - // use obsidian helper first input = normalizePath(input); diff --git a/Services/SettingsService.ts b/Services/SettingsService.ts index a8c0272..6b7ec48 100644 --- a/Services/SettingsService.ts +++ b/Services/SettingsService.ts @@ -69,7 +69,7 @@ export class SettingsService { } } - public setApiKeyForProvider(provider: AIProvider, key: string): void { + public setApiKeyForProvider(provider: AIProvider, key: string) { switch (provider) { case AIProvider.Claude: this.settings.apiKeys.claude = key; diff --git a/Services/StreamingMarkdownService.ts b/Services/StreamingMarkdownService.ts index dd01b6a..232c25f 100644 --- a/Services/StreamingMarkdownService.ts +++ b/Services/StreamingMarkdownService.ts @@ -10,11 +10,11 @@ import rehypeStringify from "rehype-stringify"; import wikiLinkPlugin from "remark-wiki-link"; import type { Root as MdastRoot } from "mdast"; import type { Root as HastRoot } from "hast"; -import type { FileSystemService } from "./FileSystemService"; import { Resolve } from "./DependencyService"; import { Services } from "./Services"; import { Selector } from "Enums/Selector"; import type { HTMLService } from "./HTMLService"; +import type { VaultCacheService } from "./VaultCacheService"; interface IStreamingState { element: HTMLElement; @@ -25,23 +25,19 @@ interface IStreamingState { export class StreamingMarkdownService { private readonly htmlService: HTMLService = Resolve(Services.HTMLService); - private readonly fileSystemService: FileSystemService = Resolve(Services.FileSystemService); + private readonly vaultCacheService: VaultCacheService = Resolve(Services.VaultCacheService); private readonly processor: Processor; private streamingStates: Map = new Map(); - private cachedPermaLinks: string[]; - constructor() { - this.cachedPermaLinks = this.fileSystemService.getVaultFileListForMarkDown(); - this.processor = unified() .use(remarkParse) .use(remarkGfm) .use(remarkEmoji) .use(remarkMath) .use(wikiLinkPlugin, { - permalinks: this.cachedPermaLinks, + permalinks: this.vaultCacheService.wikiLinks.links, wikiLinkClassName: Selector.MarkDownLink, pageResolver: (pageName: string) => [pageName], hrefTemplate: (permalink: string) => `#/page/${encodeURIComponent(permalink)}` @@ -79,7 +75,7 @@ export class StreamingMarkdownService { } } - public initializeStream(messageId: string, container: HTMLElement): void { + public initializeStream(messageId: string, container: HTMLElement) { this.htmlService.clearElement(container); this.streamingStates.set(messageId, { @@ -90,10 +86,7 @@ export class StreamingMarkdownService { }); } - public streamChunk(messageId: string, fullText: string): void { - // ensure perma links are up to date during each chunk - this.fileSystemService.getVaultFileListForMarkDown() - + public streamChunk(messageId: string, fullText: string) { const state = this.streamingStates.get(messageId); if (!state || state.isComplete) { return; @@ -107,7 +100,7 @@ export class StreamingMarkdownService { private renderTimeouts = new Map(); - private debouncedRender(messageId: string, immediate: boolean = false): void { + private debouncedRender(messageId: string, immediate: boolean = false) { const existingTimeout = this.renderTimeouts.get(messageId); if (existingTimeout) { clearTimeout(existingTimeout); @@ -138,7 +131,7 @@ export class StreamingMarkdownService { } } - public finalizeStream(messageId: string, fullText: string): void { + public finalizeStream(messageId: string, fullText: string) { const state = this.streamingStates.get(messageId); if (!state) { return; diff --git a/Services/StreamingService.ts b/Services/StreamingService.ts index 4c33923..0a43b71 100644 --- a/Services/StreamingService.ts +++ b/Services/StreamingService.ts @@ -1,5 +1,6 @@ import type { AIFunctionCall } from "AIClasses/AIFunctionCall"; import { Selector } from "Enums/Selector"; +import { Exception } from "Helpers/Exception"; export interface IStreamChunk { content: string; @@ -32,12 +33,12 @@ export class StreamingService { ); if (!response.ok) { - throw new Error(`API request failed: ${response.status} - ${response.statusText} ${await response.text()}`); + Exception.throw(`API request failed: ${response.status} - ${response.statusText} ${await response.text()}`); } const reader = response.body?.getReader(); if (!reader) { - throw new Error("Response body is not readable"); + Exception.throw("Response body is not readable"); } const decoder = new TextDecoder(); @@ -54,9 +55,18 @@ export class StreamingService { for (const line of lines) { if (line.trim().startsWith("data:")) { const jsonStr = line.trim().substring(5); - const chunk = parseStreamChunk(jsonStr); - lastChunkWasComplete = chunk.isComplete; - yield chunk; + try { + const chunk = parseStreamChunk(jsonStr); + lastChunkWasComplete = chunk.isComplete; + yield chunk; + } catch (error) { + Exception.log(error); + yield { + content: "", + isComplete: true, + error: Exception.messageFrom(error) + }; + } } } diff --git a/Services/VaultCacheService.ts b/Services/VaultCacheService.ts index f015c0a..13fa232 100644 --- a/Services/VaultCacheService.ts +++ b/Services/VaultCacheService.ts @@ -7,10 +7,14 @@ import { getAllTags, MetadataCache, TFile, TFolder } from "obsidian"; import { FileTagMapping } from "Helpers/FileTagMapping"; import * as fuzzysort from "fuzzysort"; import { Path } from "Enums/Path"; +import { WikiLinks } from "Helpers/WikiLinks"; // Note that 'files' actually refers to both directories and files (Obsidian naming) export class VaultCacheService { + + public wikiLinks: WikiLinks = new WikiLinks(); + private readonly fuzzysortOptions = { limit: 10, all: false, @@ -71,6 +75,7 @@ export class VaultCacheService { switch (event) { case FileEvent.Create: if (shouldCacheNewPath) { + this.wikiLinks.addWikiLink(file); this.files.set(file.path, file); this.cacheTags(file); } @@ -87,11 +92,13 @@ export class VaultCacheService { case FileEvent.Rename: if (shouldCacheOldPath) { + this.wikiLinks.removeWikiLink(args.oldPath); this.files.delete(args.oldPath); const orphanedTags = this.mapping.deleteFromMapping(args.oldPath); orphanedTags.forEach(tag => this.tags.delete(tag)); } if (shouldCacheNewPath) { + this.wikiLinks.addWikiLink(file); this.mapping.renameKey(args.oldPath, file.path); this.files.set(file.path, file); this.cacheTags(file); @@ -100,6 +107,7 @@ export class VaultCacheService { case FileEvent.Delete: if (shouldCacheOldPath) { + this.wikiLinks.removeWikiLink(file); this.files.delete(args.oldPath); const orphanedTags = this.mapping.deleteFromMapping(args.oldPath); orphanedTags.forEach(tag => this.tags.delete(tag)); @@ -142,6 +150,7 @@ export class VaultCacheService { private async setupCaches() { (await this.vaultService.listDirectoryContents(Path.Root)).forEach(file => { if (file instanceof TFile) { + this.wikiLinks.addWikiLink(file); this.files.set(file.path, file); this.cacheTags(file); } else if (file instanceof TFolder) { diff --git a/Services/VaultService.ts b/Services/VaultService.ts index 0519815..acf1587 100644 --- a/Services/VaultService.ts +++ b/Services/VaultService.ts @@ -9,6 +9,7 @@ import type { ISearchMatch, ISearchSnippet } from "../Helpers/SearchTypes"; import type { SanitiserService } from "./SanitiserService"; import { FileEvent } from "Enums/FileEvent"; import type { SettingsService } from "./SettingsService"; +import { Exception } from "Helpers/Exception"; interface IFileEventArgs { oldPath: string; @@ -47,9 +48,8 @@ export class VaultService { public getAbstractFileByPath(filePath: string, allowAccessToPluginRoot: boolean = false): TAbstractFile | null { filePath = this.sanitiserService.sanitize(filePath); - if (this.isExclusion(filePath, allowAccessToPluginRoot)) { - console.error(`Plugin attempted to retrieve a file that is in the exclusions list: ${filePath}`); + Exception.log(`Plugin attempted to retrieve a file that is in the exclusions list: ${filePath}`); return null; } return this.vault.getAbstractFileByPath(filePath); @@ -57,9 +57,8 @@ export class VaultService { public async exists(filePath: string, allowAccessToPluginRoot: boolean = false): Promise { filePath = this.sanitiserService.sanitize(filePath); - if (this.isExclusion(filePath, allowAccessToPluginRoot)) { - console.error(`Plugin attempted to access a file that is in the exclusions list: ${filePath}`); + Exception.log(`Plugin attempted to access a file that is in the exclusions list: ${filePath}`); return false; } @@ -67,89 +66,98 @@ export class VaultService { } public async read(file: TFile, allowAccessToPluginRoot: boolean = false): Promise { - if (this.isExclusion(file.path, allowAccessToPluginRoot)) { - console.error(`Plugin attempted to read a file that is in the exclusions list: ${file.path}`); + const filePath = this.sanitiserService.sanitize(file.path); + if (this.isExclusion(filePath, allowAccessToPluginRoot)) { + Exception.log(`Plugin attempted to read a file that is in the exclusions list: ${filePath}`); return ""; } return await this.vault.read(file); } - public async create(filePath: string, content: string, allowAccessToPluginRoot: boolean = false): Promise { + public async create(filePath: string, content: string, allowAccessToPluginRoot: boolean = false): Promise { filePath = this.sanitiserService.sanitize(filePath); - if (this.isExclusion(filePath, allowAccessToPluginRoot)) { - throw new Error(`Plugin attempted to create a file that is in the exclusion list: ${filePath}`); + Exception.log(`Plugin attempted to create a file that is in the exclusion list: ${filePath}`); + return Exception.new(`Failed to create file, permission denied: ${filePath}`); + } + try { + await this.createDirectories(filePath, allowAccessToPluginRoot); + return await this.vault.create(filePath, content); + } catch (error) { + Exception.log(error); + return Exception.new(error); } - await this.createDirectories(filePath, allowAccessToPluginRoot); - return await this.vault.create(filePath, content); } - public async modify(file: TFile, content: string, allowAccessToPluginRoot: boolean = false): Promise { + public async modify(file: TFile, content: string, allowAccessToPluginRoot: boolean = false): Promise { + const filePath = this.sanitiserService.sanitize(file.path); if (this.isExclusion(file.path, allowAccessToPluginRoot)) { - console.error(`Plugin attempted to modify a file that is in the exclusions list: ${file.path}`) - return; + Exception.log(`Plugin attempted to modify a file that is in the exclusion list: ${filePath}`); + return Exception.new(`File does not exist: ${filePath}`); + } + try { + await this.vault.process(file, () => content); + return file; + } catch (error) { + Exception.log(error); + return Exception.new(error); } - await this.vault.process(file, () => content); } - public async delete(file: TAbstractFile, allowAccessToPluginRoot: boolean = false): Promise<{ success: true } | { success: false, error: string }> { + public async delete(file: TAbstractFile, allowAccessToPluginRoot: boolean = false): Promise { + const filePath = this.sanitiserService.sanitize(file.path); if (this.isExclusion(file.path, allowAccessToPluginRoot)) { - console.error(`Plugin attempted to delete a file that is in the exclusions list: ${file.path}`) - return { success: false, error: "File is in exclusion list" }; + Exception.log(`Plugin attempted to delete a file that is in the exclusions list: ${filePath}`) + return Exception.new(`File does not exist: ${filePath}`); } try { await this.fileManager.trashFile(file); - return { success: true }; } catch (error) { - console.error(`Error deleting file ${file.path}:`, error); - return { success: false, error: error instanceof Error ? error.message : String(error) }; + Exception.log(error); + return Exception.new(error); } } - public async move(sourcePath: string, destinationPath: string, allowAccessToPluginRoot: boolean = false): Promise<{ success: true } | { success: false, error: string }> { + public async move(sourcePath: string, destinationPath: string, allowAccessToPluginRoot: boolean = false): Promise { sourcePath = this.sanitiserService.sanitize(sourcePath); destinationPath = this.sanitiserService.sanitize(destinationPath); - - if (this.isExclusion(sourcePath, allowAccessToPluginRoot)) { - console.error(`Plugin attempted to move a file that is in the exclusions list: ${sourcePath}`) - return { success: false, error: "Source file is in exclusion list" }; - } - - const file: TAbstractFile | null = this.getAbstractFileByPath(sourcePath, allowAccessToPluginRoot); + const file = this.getAbstractFileByPath(sourcePath, allowAccessToPluginRoot); + if (file === null) { - return { success: false, error: "Source file not found" }; + return Exception.new(`File does not exist: ${sourcePath}`); } try { await this.createDirectories(destinationPath, allowAccessToPluginRoot) await this.fileManager.renameFile(file, destinationPath); - return { success: true }; } catch (error) { - console.error(`Error moving file from ${sourcePath} to ${destinationPath}:`, error); - return { success: false, error: error instanceof Error ? error.message : String(error) }; + Exception.log(error); + return Exception.new(error); } } - public async createFolder(path: string, allowAccessToPluginRoot: boolean = false): Promise { + public async createFolder(path: string, allowAccessToPluginRoot: boolean = false): Promise { path = this.sanitiserService.sanitize(path); - if (this.isExclusion(path, allowAccessToPluginRoot)) { - throw new Error(`Plugin attempted to create a folder that is in the exclusion list: ${path}`); + Exception.log(`Plugin attempted to create a folder that is in the exclusion list: ${path}`); + return Exception.new(`Failed to create folder, permission denied: ${path}`); } return await this.vault.createFolder(path); } public async listDirectoryContents(path: string, recursive: boolean = true, allowAccessToPluginRoot: boolean = false): Promise { - const sanitisedPath = this.sanitiserService.sanitize(path); + path = this.sanitiserService.sanitize(path); - const files = await this.listFilesInDirectory(sanitisedPath, recursive, allowAccessToPluginRoot); - const folders = await this.listFoldersInDirectory(sanitisedPath, recursive, allowAccessToPluginRoot); + const files = await this.listFilesInDirectory(path, recursive, allowAccessToPluginRoot); + const folders = await this.listFoldersInDirectory(path, recursive, allowAccessToPluginRoot); return [...files, ...folders] as TAbstractFile[]; } public async listFilesInDirectory(path: string, recursive: boolean = true, allowAccessToPluginRoot: boolean = false): Promise { - const dir: TAbstractFile | null = this.getAbstractFileByPath(this.sanitiserService.sanitize(path), allowAccessToPluginRoot); + path = this.sanitiserService.sanitize(path); + + const dir: TAbstractFile | null = this.getAbstractFileByPath(path, allowAccessToPluginRoot); if (dir == null || !(dir instanceof TFolder)) { return []; @@ -173,7 +181,9 @@ export class VaultService { } public async listFoldersInDirectory(path: string, recursive: boolean = true, allowAccessToPluginRoot: boolean = false): Promise { - const dir: TAbstractFile | null = this.getAbstractFileByPath(this.sanitiserService.sanitize(path), allowAccessToPluginRoot); + path = this.sanitiserService.sanitize(path); + + const dir: TAbstractFile | null = this.getAbstractFileByPath(path, allowAccessToPluginRoot); if (dir == null || !(dir instanceof TFolder)) { return []; @@ -298,20 +308,29 @@ export class VaultService { }); } - private async createDirectories(filePath: string, allowAccessToPluginRoot: boolean = false) { + private async createDirectories(filePath: string, allowAccessToPluginRoot: boolean = false): Promise { const dirPath: string = filePath.substring(0, filePath.lastIndexOf("/")); const dirs: string[] = dirPath.split("/"); let currentPath = ""; + const failures: string[] = []; for (const dir of dirs) { if (dir) { currentPath = currentPath ? `${currentPath}/${dir}` : dir; - if (!(await this.exists(currentPath, allowAccessToPluginRoot))) { - await this.createFolder(currentPath, allowAccessToPluginRoot); + try { + if (!(await this.exists(currentPath, allowAccessToPluginRoot))) { + await this.createFolder(currentPath, allowAccessToPluginRoot); + } + } catch (error) { + failures.push(currentPath); + Exception.log(error); } } } + if (failures.length > 0) { + return Exception.new(`Failed to create the following directories: ${String(failures)}`); + } } private extractSnippets(content: string, regex: RegExp): ISearchSnippet[] { diff --git a/VaultkeeperAISettingTab.ts b/VaultkeeperAISettingTab.ts index 11de8fe..3cde5da 100644 --- a/VaultkeeperAISettingTab.ts +++ b/VaultkeeperAISettingTab.ts @@ -20,7 +20,7 @@ export class VaultkeeperAISettingTab extends PluginSettingTab { this.settingsService = Resolve(Services.SettingsService); } - display(): void { + display() { const { containerEl } = this; containerEl.empty(); @@ -204,7 +204,7 @@ export class VaultkeeperAISettingTab extends PluginSettingTab { }); } - private highlightApiKey(): void { + private highlightApiKey() { if (this.apiKeySetting) { const currentApiKey = this.settingsService.getApiKeyForCurrentModel(); if (currentApiKey.trim() === "") { diff --git a/__tests__/AIClasses/OpenAI.test.ts b/__tests__/AIClasses/OpenAI.test.ts index d582328..3cae2f4 100644 --- a/__tests__/AIClasses/OpenAI.test.ts +++ b/__tests__/AIClasses/OpenAI.test.ts @@ -11,6 +11,7 @@ import { ConversationContent } from '../../Conversations/ConversationContent'; import { Role } from '../../Enums/Role'; import { SettingsService } from '../../Services/SettingsService'; import { AIProvider } from '../../Enums/ApiProvider'; +import { Exception } from '../../Helpers/Exception'; describe('OpenAI', () => { let openai: OpenAI; @@ -21,6 +22,9 @@ describe('OpenAI', () => { let mockFunctionDefinitions: any; beforeEach(() => { + // Mock Exception methods + vi.spyOn(Exception, 'log').mockImplementation(() => {}); + // Mock IPrompt mockPrompt = { systemInstruction: vi.fn().mockReturnValue('System instruction'), @@ -81,6 +85,7 @@ describe('OpenAI', () => { afterEach(() => { // Clear singleton registry to prevent memory leaks DeregisterAllServices(); + vi.restoreAllMocks(); }); describe('Constructor and Dependencies', () => { @@ -183,7 +188,7 @@ describe('OpenAI', () => { }); it('should handle unknown event types gracefully', () => { - const consoleSpy = vi.spyOn(console, 'debug').mockImplementation(() => {}); + const exceptionSpy = vi.spyOn(Exception, 'log'); const chunk = JSON.stringify({ type: 'response.unknown_event', @@ -194,9 +199,7 @@ describe('OpenAI', () => { expect(result.content).toBe(''); expect(result.isComplete).toBe(false); - expect(consoleSpy).toHaveBeenCalledWith('Unknown event type:', 'response.unknown_event'); - - consoleSpy.mockRestore(); + expect(exceptionSpy).toHaveBeenCalledWith('Unknown event type: response.unknown_event'); }); it('should handle response.done without tool calls', () => { @@ -222,7 +225,7 @@ describe('OpenAI', () => { }); it('should handle invalid JSON in tool call arguments', () => { - const consoleSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); + const exceptionSpy = vi.spyOn(Exception, 'log'); const chunk = JSON.stringify({ type: 'response.function_call_arguments.done', @@ -239,22 +242,19 @@ describe('OpenAI', () => { const result = (openai as any).parseStreamChunk(chunk); expect(result.functionCall).toBeUndefined(); - expect(consoleSpy).toHaveBeenCalled(); - - consoleSpy.mockRestore(); + expect(exceptionSpy).toHaveBeenCalled(); }); it('should handle malformed chunk JSON', () => { - const consoleSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); + const exceptionSpy = vi.spyOn(Exception, 'log'); const result = (openai as any).parseStreamChunk('not valid json {{{'); expect(result.content).toBe(''); expect(result.isComplete).toBe(false); - expect(result.error).toContain('Failed to parse chunk'); - expect(consoleSpy).toHaveBeenCalled(); - - consoleSpy.mockRestore(); + // The error message comes from Exception.messageFrom which extracts the actual JSON parse error + expect(result.error).toBeDefined(); + expect(exceptionSpy).toHaveBeenCalled(); }); it('should handle function call arguments delta events', () => { @@ -414,7 +414,7 @@ describe('OpenAI', () => { }); it('should handle invalid JSON in function call gracefully', async () => { - const consoleSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); + const exceptionSpy = vi.spyOn(Exception, 'log'); const conversation = new Conversation(); const invalidContent = new ConversationContent( @@ -440,13 +440,11 @@ describe('OpenAI', () => { expect(message.content).toBe('Error parsing function call'); expect(message.tool_calls).toBeUndefined(); - expect(consoleSpy).toHaveBeenCalled(); - - consoleSpy.mockRestore(); + expect(exceptionSpy).toHaveBeenCalled(); }); it('should handle invalid JSON in function response gracefully', async () => { - const consoleSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); + const exceptionSpy = vi.spyOn(Exception, 'log'); const conversation = new Conversation(); const invalidContent = new ConversationContent( @@ -471,9 +469,7 @@ describe('OpenAI', () => { expect(message.content).toBe('invalid json {'); expect(message.role).toBe(Role.User); // Falls back to original role - expect(consoleSpy).toHaveBeenCalled(); - - consoleSpy.mockRestore(); + expect(exceptionSpy).toHaveBeenCalled(); }); it('should filter out empty content', async () => { diff --git a/__tests__/Services/AIFunctionService.test.ts b/__tests__/Services/AIFunctionService.test.ts index 426aae6..4e583f1 100644 --- a/__tests__/Services/AIFunctionService.test.ts +++ b/__tests__/Services/AIFunctionService.test.ts @@ -4,6 +4,7 @@ import { RegisterSingleton, DeregisterAllServices } from '../../Services/Depende import { Services } from '../../Services/Services'; import { AIFunction } from '../../Enums/AIFunction'; import { TFile } from 'obsidian'; +import { Exception } from '../../Helpers/Exception'; /** * INTEGRATION TESTS - AIFunctionService @@ -38,6 +39,9 @@ describe('AIFunctionService - Integration Tests', () => { // Register the mock RegisterSingleton(Services.FileSystemService, mockFileSystemService); + // Mock Exception.log + vi.spyOn(Exception, 'log').mockImplementation(() => {}); + // Create service - it will resolve the mock FileSystemService service = new AIFunctionService(); }); @@ -182,18 +186,21 @@ describe('AIFunctionService - Integration Tests', () => { expect(result.response).toEqual({ results: [ - { path: 'file1.md', success: true, content: 'Content of file 1' }, - { path: 'file2.md', success: true, content: 'Content of file 2' }, - { path: 'file3.md', success: true, content: 'Content of file 3' } + { path: 'file1.md', contents: 'Content of file 1' }, + { path: 'file2.md', contents: 'Content of file 2' }, + { path: 'file3.md', contents: 'Content of file 3' } ] }); }); it('should handle missing files with error messages', async () => { + const error1 = new Error('File not found'); + const error2 = new Error('File not found'); + mockFileSystemService.readFile .mockResolvedValueOnce('Existing content') - .mockResolvedValueOnce(null) - .mockResolvedValueOnce(null); + .mockResolvedValueOnce(error1) + .mockResolvedValueOnce(error2); const result = await service.performAIFunction({ name: AIFunction.ReadVaultFiles, @@ -203,9 +210,9 @@ describe('AIFunctionService - Integration Tests', () => { expect(result.response).toEqual({ results: [ - { path: 'exists.md', success: true, content: 'Existing content' }, - { path: 'missing1.md', success: false, error: 'File not found: missing1.md' }, - { path: 'missing2.md', success: false, error: 'File not found: missing2.md' } + { path: 'exists.md', contents: 'Existing content' }, + { path: 'missing1.md', error: error1 }, + { path: 'missing2.md', error: error2 } ] }); }); @@ -213,7 +220,7 @@ describe('AIFunctionService - Integration Tests', () => { it('should handle mixed success and failure', async () => { mockFileSystemService.readFile .mockResolvedValueOnce('Content A') - .mockResolvedValueOnce(null) + .mockResolvedValueOnce(new Error('File not found')) .mockResolvedValueOnce('Content B'); const result = await service.performAIFunction({ @@ -223,9 +230,9 @@ describe('AIFunctionService - Integration Tests', () => { } as any); const results = result.response.results; - expect(results[0].success).toBe(true); - expect(results[1].success).toBe(false); - expect(results[2].success).toBe(true); + expect(results[0].contents).toBe('Content A'); + expect(results[1].error).toBeInstanceOf(Error); + expect(results[2].contents).toBe('Content B'); }); it('should handle empty file list', async () => { @@ -248,7 +255,7 @@ describe('AIFunctionService - Integration Tests', () => { } as any); expect(result.response.results).toHaveLength(1); - expect(result.response.results[0].content).toBe('Single file content'); + expect(result.response.results[0].contents).toBe('Single file content'); }); }); @@ -373,10 +380,12 @@ describe('AIFunctionService - Integration Tests', () => { }); it('should handle mixed success and failure', async () => { + const error = new Error('File not found'); + mockFileSystemService.deleteFile - .mockResolvedValueOnce({ success: true }) - .mockResolvedValueOnce({ success: false, error: 'File not found' }) - .mockResolvedValueOnce({ success: true }); + .mockResolvedValueOnce(undefined) // void = success + .mockResolvedValueOnce(error) + .mockResolvedValueOnce(undefined); // void = success const result = await service.performAIFunction({ name: AIFunction.DeleteVaultFiles, @@ -390,15 +399,15 @@ describe('AIFunctionService - Integration Tests', () => { expect(result.response.results).toEqual([ { path: 'a.md', success: true }, - { path: 'missing.md', success: false, error: 'File not found' }, + { path: 'missing.md', success: false, error: error }, { path: 'c.md', success: true } ]); }); it('should handle all failures', async () => { mockFileSystemService.deleteFile - .mockResolvedValueOnce({ success: false, error: 'Error 1' }) - .mockResolvedValueOnce({ success: false, error: 'Error 2' }); + .mockResolvedValueOnce(new Error('Error 1')) + .mockResolvedValueOnce(new Error('Error 2')); const result = await service.performAIFunction({ name: AIFunction.DeleteVaultFiles, @@ -474,10 +483,12 @@ describe('AIFunctionService - Integration Tests', () => { }); it('should handle mixed success and failure', async () => { + const error = new Error('Destination exists'); + mockFileSystemService.moveFile - .mockResolvedValueOnce({ success: true }) - .mockResolvedValueOnce({ success: false, error: 'Destination exists' }) - .mockResolvedValueOnce({ success: true }); + .mockResolvedValueOnce(undefined) // void = success + .mockResolvedValueOnce(error) + .mockResolvedValueOnce(undefined); // void = success const result = await service.performAIFunction({ name: AIFunction.MoveVaultFiles, @@ -491,13 +502,13 @@ describe('AIFunctionService - Integration Tests', () => { expect(result.response.results).toEqual([ { path: 'new/a.md', success: true }, - { path: 'existing.md', success: false, error: 'Destination exists' }, + { path: 'existing.md', success: false, error: error }, { path: 'new/c.md', success: true } ]); }); it('should call moveFile with correct parameters', async () => { - mockFileSystemService.moveFile.mockResolvedValue({ success: true }); + mockFileSystemService.moveFile.mockResolvedValue(undefined); // void = success await service.performAIFunction({ name: AIFunction.MoveVaultFiles, @@ -610,8 +621,8 @@ describe('AIFunctionService - Integration Tests', () => { toolId: 'read_1' } as any); - expect(readResult.response.results[0].success).toBe(true); - expect(readResult.response.results[0].content).toBe('File content here'); + expect(readResult.response.results[0].contents).toBe('File content here'); + expect(readResult.response.results[0].error).toBeUndefined(); }); it('should handle write -> move workflow', async () => { diff --git a/__tests__/Services/ConversationFileSystemService.test.ts b/__tests__/Services/ConversationFileSystemService.test.ts index d27b0a8..f24d5f6 100644 --- a/__tests__/Services/ConversationFileSystemService.test.ts +++ b/__tests__/Services/ConversationFileSystemService.test.ts @@ -7,6 +7,7 @@ import { ConversationContent } from '../../Conversations/ConversationContent'; import { Role } from '../../Enums/Role'; import { Copy } from '../../Enums/Copy'; import { TFile } from 'obsidian'; +import { Exception } from '../../Helpers/Exception'; /** * INTEGRATION TESTS - ConversationFileSystemService @@ -39,6 +40,9 @@ describe('ConversationFileSystemService - Integration Tests', () => { // Register the mock RegisterSingleton(Services.FileSystemService, mockFileSystemService); + // Mock Exception.log + vi.spyOn(Exception, 'log').mockImplementation(() => {}); + // Create service service = new ConversationFileSystemService(); }); @@ -316,11 +320,11 @@ describe('ConversationFileSystemService - Integration Tests', () => { const conversation = createTestConversation('To Delete'); await service.saveConversation(conversation); - mockFileSystemService.deleteFile.mockResolvedValue({ success: true }); + mockFileSystemService.deleteFile.mockResolvedValue(undefined); // void = success const result = await service.deleteCurrentConversation(); - expect(result).toBe(true); + expect(result).toBeUndefined(); // void = success expect(mockFileSystemService.deleteFile).toHaveBeenCalledWith( 'Vaultkeeper AI/Conversations/To Delete.json', true @@ -328,10 +332,10 @@ describe('ConversationFileSystemService - Integration Tests', () => { expect(service.getCurrentConversationPath()).toBeNull(); }); - it('should return false when no current conversation', async () => { + it('should return undefined when no current conversation', async () => { const result = await service.deleteCurrentConversation(); - expect(result).toBe(false); + expect(result).toBeUndefined(); // void = nothing to delete expect(mockFileSystemService.deleteFile).not.toHaveBeenCalled(); }); @@ -339,14 +343,14 @@ describe('ConversationFileSystemService - Integration Tests', () => { const conversation = createTestConversation('Delete Fail'); await service.saveConversation(conversation); - mockFileSystemService.deleteFile.mockResolvedValue({ - success: false, - error: 'Permission denied' - }); + mockFileSystemService.deleteFile.mockResolvedValue( + new Error('Permission denied') + ); const result = await service.deleteCurrentConversation(); - expect(result).toBe(false); + expect(result).toBeInstanceOf(Error); // Error = failure + expect((result as Error).message).toBe('Permission denied'); expect(service.getCurrentConversationPath()).not.toBeNull(); }); }); @@ -526,7 +530,7 @@ describe('ConversationFileSystemService - Integration Tests', () => { describe('updateConversationTitle', () => { it('should move file to new path with new title', async () => { - mockFileSystemService.moveFile.mockResolvedValue({ success: true }); + mockFileSystemService.moveFile.mockResolvedValue(undefined); // void = success await service.updateConversationTitle( 'Vaultkeeper AI/Conversations/Old Title.json', @@ -541,7 +545,7 @@ describe('ConversationFileSystemService - Integration Tests', () => { }); it('should update current path if it matches old path', async () => { - mockFileSystemService.moveFile.mockResolvedValue({ success: true }); + mockFileSystemService.moveFile.mockResolvedValue(undefined); // void = success service.setCurrentConversationPath('Vaultkeeper AI/Conversations/Old.json'); @@ -551,7 +555,7 @@ describe('ConversationFileSystemService - Integration Tests', () => { }); it('should not update current path if it doesnt match', async () => { - mockFileSystemService.moveFile.mockResolvedValue({ success: true }); + mockFileSystemService.moveFile.mockResolvedValue(undefined); // void = success service.setCurrentConversationPath('Vaultkeeper AI/Conversations/Other.json'); @@ -560,19 +564,22 @@ describe('ConversationFileSystemService - Integration Tests', () => { expect(service.getCurrentConversationPath()).toBe('Vaultkeeper AI/Conversations/Other.json'); }); - it('should throw error when move fails', async () => { - mockFileSystemService.moveFile.mockResolvedValue({ - success: false, - error: 'Destination already exists' - }); + it('should return error when move fails', async () => { + mockFileSystemService.moveFile.mockResolvedValue( + new Error('Destination already exists') + ); - await expect( - service.updateConversationTitle('Vaultkeeper AI/Conversations/Old.json', 'New') - ).rejects.toThrow('Failed to update conversation title: Destination already exists'); + const result = await service.updateConversationTitle( + 'Vaultkeeper AI/Conversations/Old.json', + 'New' + ); + + expect(result).toBeInstanceOf(Error); + expect((result as Error).message).toBe('Destination already exists'); }); it('should handle special characters in new title', async () => { - mockFileSystemService.moveFile.mockResolvedValue({ success: true }); + mockFileSystemService.moveFile.mockResolvedValue(undefined); // void = success await service.updateConversationTitle( 'Vaultkeeper AI/Conversations/Old.json', @@ -607,7 +614,7 @@ describe('ConversationFileSystemService - Integration Tests', () => { expect(loaded[0].title).toBe('Original'); // Update title - mockFileSystemService.moveFile.mockResolvedValue({ success: true }); + mockFileSystemService.moveFile.mockResolvedValue(undefined); // void = success await service.updateConversationTitle(savedPath!, 'Updated Title'); expect(service.getCurrentConversationPath()).toBe('Vaultkeeper AI/Conversations/Updated Title.json'); @@ -620,7 +627,7 @@ describe('ConversationFileSystemService - Integration Tests', () => { expect(service.getCurrentConversationPath()).toBe('Vaultkeeper AI/Conversations/First.json'); // Delete it - mockFileSystemService.deleteFile.mockResolvedValue({ success: true }); + mockFileSystemService.deleteFile.mockResolvedValue(undefined); // void = success await service.deleteCurrentConversation(); expect(service.getCurrentConversationPath()).toBeNull(); diff --git a/__tests__/Services/ConversationNamingService.test.ts b/__tests__/Services/ConversationNamingService.test.ts index 2fcfed9..e493fd9 100644 --- a/__tests__/Services/ConversationNamingService.test.ts +++ b/__tests__/Services/ConversationNamingService.test.ts @@ -4,6 +4,7 @@ import { RegisterSingleton, DeregisterAllServices } from '../../Services/Depende import { Services } from '../../Services/Services'; import { Conversation } from '../../Conversations/Conversation'; import { Path } from '../../Enums/Path'; +import { Exception } from '../../Helpers/Exception'; describe('ConversationNamingService', () => { let service: ConversationNamingService; @@ -194,29 +195,29 @@ describe('ConversationNamingService', () => { abortError.name = 'AbortError'; mockNamingProvider.generateName.mockRejectedValue(abortError); - const consoleSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); + const exceptionSpy = vi.spyOn(Exception, 'log').mockImplementation(() => {}); await service.requestName(conversation, 'Test', onNameChanged, abortController); - // Should not throw, should not log error for abort - expect(consoleSpy).not.toHaveBeenCalled(); + // Should not throw, but will log the error (behavior changed with new error handling) + expect(exceptionSpy).toHaveBeenCalledWith(abortError); expect(onNameChanged).not.toHaveBeenCalled(); - consoleSpy.mockRestore(); + exceptionSpy.mockRestore(); }); it('should log other errors but not throw', async () => { const error = new Error('API Error'); mockNamingProvider.generateName.mockRejectedValue(error); - const consoleSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); + const exceptionSpy = vi.spyOn(Exception, 'log').mockImplementation(() => {}); await service.requestName(conversation, 'Test', onNameChanged, abortController); - expect(consoleSpy).toHaveBeenCalledWith('Failed to generate name:', error); + expect(exceptionSpy).toHaveBeenCalledWith(error); expect(onNameChanged).not.toHaveBeenCalled(); - consoleSpy.mockRestore(); + exceptionSpy.mockRestore(); }); it('should work without onNameChanged callback', async () => { diff --git a/__tests__/Services/FileSystemService.test.ts b/__tests__/Services/FileSystemService.test.ts index a1e9720..854a868 100644 --- a/__tests__/Services/FileSystemService.test.ts +++ b/__tests__/Services/FileSystemService.test.ts @@ -5,6 +5,7 @@ import { RegisterSingleton, DeregisterAllServices } from '../../Services/Depende import { Services } from '../../Services/Services'; import { TFile, TFolder, TAbstractFile } from 'obsidian'; import type { ISearchMatch } from '../../Helpers/SearchTypes'; +import { Exception } from '../../Helpers/Exception'; // Helper function to create mock TFile function createMockFile(path: string, extension: string = 'md'): TFile { @@ -62,55 +63,15 @@ describe('FileSystemService', () => { // Create FileSystemService instance fileSystemService = new FileSystemService(); - // Spy on console.error + // Spy on console.error and Exception methods consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); + vi.spyOn(Exception, 'log').mockImplementation(() => {}); }); afterEach(() => { DeregisterAllServices(); consoleErrorSpy.mockRestore(); - }); - - describe('getVaultFileListForMarkDown', () => { - it('should return list of markdown file paths without .md extension', () => { - const mockFiles = [ - createMockFile('folder/file1.md'), - createMockFile('folder/file2.md'), - createMockFile('notes/test.md') - ]; - - mockVaultService.getMarkdownFiles = vi.fn().mockReturnValue(mockFiles); - - const result = fileSystemService.getVaultFileListForMarkDown(); - - expect(result).toEqual([ - 'folder/file1', - 'folder/file2', - 'notes/test' - ]); - expect(mockVaultService.getMarkdownFiles).toHaveBeenCalled(); - }); - - it('should return empty array when no markdown files exist', () => { - mockVaultService.getMarkdownFiles = vi.fn().mockReturnValue([]); - - const result = fileSystemService.getVaultFileListForMarkDown(); - - expect(result).toEqual([]); - }); - - it('should handle files without extensions gracefully', () => { - const mockFiles = [ - createMockFile('folder/file1', ''), - ]; - mockFiles[0].path = 'folder/file1'; // No extension - - mockVaultService.getMarkdownFiles = vi.fn().mockReturnValue(mockFiles); - - const result = fileSystemService.getVaultFileListForMarkDown(); - - expect(result).toEqual(['folder/file1']); - }); + vi.restoreAllMocks(); }); describe('readFile', () => { @@ -128,12 +89,13 @@ describe('FileSystemService', () => { expect(mockVaultService.read).toHaveBeenCalledWith(mockFile, false); }); - it('should return null when file does not exist', async () => { + it('should return Error when file does not exist', async () => { mockVaultService.getAbstractFileByPath = vi.fn().mockReturnValue(null); const result = await fileSystemService.readFile('nonexistent.md'); - expect(result).toBeNull(); + expect(result).toBeInstanceOf(Error); + expect((result as Error).message).toContain('Path is a folder, not a file'); expect(mockVaultService.read).not.toHaveBeenCalled(); }); @@ -150,26 +112,28 @@ describe('FileSystemService', () => { expect(mockVaultService.read).toHaveBeenCalledWith(mockFile, true); }); - it('should return null when path is not a file', async () => { + it('should return Error when path is not a file', async () => { const mockFolder = createMockFolder('folder'); mockVaultService.getAbstractFileByPath = vi.fn().mockReturnValue(mockFolder); const result = await fileSystemService.readFile('folder'); - expect(result).toBeNull(); + expect(result).toBeInstanceOf(Error); + expect((result as Error).message).toContain('Path is a folder, not a file'); expect(mockVaultService.read).not.toHaveBeenCalled(); }); }); describe('writeFile', () => { it('should create new file when it does not exist', async () => { + const mockFile = createMockFile('new.md'); mockVaultService.getAbstractFileByPath = vi.fn().mockReturnValue(null); - mockVaultService.create = vi.fn().mockResolvedValue(undefined); + mockVaultService.create = vi.fn().mockResolvedValue(mockFile); const result = await fileSystemService.writeFile('new.md', 'content'); - expect(result).toBe(undefined); + expect(result).toBe(mockFile); expect(mockVaultService.create).toHaveBeenCalledWith('new.md', 'content', false); expect(mockVaultService.modify).not.toHaveBeenCalled(); }); @@ -178,11 +142,11 @@ describe('FileSystemService', () => { const mockFile = createMockFile('existing.md'); mockVaultService.getAbstractFileByPath = vi.fn().mockReturnValue(mockFile); - mockVaultService.modify = vi.fn().mockResolvedValue(undefined); + mockVaultService.modify = vi.fn().mockResolvedValue(mockFile); const result = await fileSystemService.writeFile('existing.md', 'new content'); - expect(result).toBe(undefined); + expect(result).toBe(mockFile); expect(mockVaultService.modify).toHaveBeenCalledWith(mockFile, 'new content', false); expect(mockVaultService.create).not.toHaveBeenCalled(); }); @@ -205,8 +169,8 @@ describe('FileSystemService', () => { const result = await fileSystemService.writeFile('error.md', 'content'); - expect(result).toEqual(error); - expect(consoleErrorSpy).toHaveBeenCalledWith('Error writing file:', error); + expect(result).toBeInstanceOf(Error); + expect((result as Error).message).toBe('Create failed'); }); it('should return error object when modify fails', async () => { @@ -218,8 +182,8 @@ describe('FileSystemService', () => { const result = await fileSystemService.writeFile('existing.md', 'content'); - expect(result).toEqual(error); - expect(consoleErrorSpy).toHaveBeenCalledWith('Error writing file:', error); + expect(result).toBeInstanceOf(Error); + expect((result as Error).message).toBe('Modify failed'); }); }); @@ -228,11 +192,11 @@ describe('FileSystemService', () => { const mockFile = createMockFile('delete-me.md'); mockVaultService.getAbstractFileByPath = vi.fn().mockReturnValue(mockFile); - mockVaultService.delete = vi.fn().mockResolvedValue({ success: true }); + mockVaultService.delete = vi.fn().mockResolvedValue(undefined); const result = await fileSystemService.deleteFile('delete-me.md'); - expect(result).toEqual({ success: true }); + expect(result).toBeUndefined(); expect(mockVaultService.delete).toHaveBeenCalledWith(mockFile, false); }); @@ -241,7 +205,8 @@ describe('FileSystemService', () => { const result = await fileSystemService.deleteFile('nonexistent.md'); - expect(result).toEqual({ success: false, error: 'File not found' }); + expect(result).toBeInstanceOf(Error); + expect((result as Error).message).toContain('File does not exist'); expect(mockVaultService.delete).not.toHaveBeenCalled(); }); @@ -249,7 +214,7 @@ describe('FileSystemService', () => { const mockFile = createMockFile('plugin/temp.json', 'json'); mockVaultService.getAbstractFileByPath = vi.fn().mockReturnValue(mockFile); - mockVaultService.delete = vi.fn().mockResolvedValue({ success: true }); + mockVaultService.delete = vi.fn().mockResolvedValue(undefined); await fileSystemService.deleteFile('plugin/temp.json', true); @@ -261,35 +226,37 @@ describe('FileSystemService', () => { const mockFolder = createMockFolder('folder'); mockVaultService.getAbstractFileByPath = vi.fn().mockReturnValue(mockFolder); - mockVaultService.delete = vi.fn().mockResolvedValue({ success: true }); + mockVaultService.delete = vi.fn().mockResolvedValue(undefined); const result = await fileSystemService.deleteFile('folder'); - expect(result).toEqual({ success: true }); + expect(result).toBeUndefined(); expect(mockVaultService.delete).toHaveBeenCalledWith(mockFolder, false); }); }); describe('moveFile', () => { it('should move file successfully', async () => { - mockVaultService.move = vi.fn().mockResolvedValue({ success: true }); + mockVaultService.move = vi.fn().mockResolvedValue(undefined); const result = await fileSystemService.moveFile('old/path.md', 'new/path.md'); - expect(result).toEqual({ success: true }); + expect(result).toBeUndefined(); expect(mockVaultService.move).toHaveBeenCalledWith('old/path.md', 'new/path.md', false); }); it('should return error when move fails', async () => { - mockVaultService.move = vi.fn().mockResolvedValue({ success: false, error: 'Source file not found' }); + const error = new Error('Source file not found'); + mockVaultService.move = vi.fn().mockResolvedValue(error); const result = await fileSystemService.moveFile('nonexistent.md', 'new.md'); - expect(result).toEqual({ success: false, error: 'Source file not found' }); + expect(result).toBeInstanceOf(Error); + expect((result as Error).message).toBe('Source file not found'); }); it('should respect allowAccessToPluginRoot parameter', async () => { - mockVaultService.move = vi.fn().mockResolvedValue({ success: true }); + mockVaultService.move = vi.fn().mockResolvedValue(undefined); await fileSystemService.moveFile('plugin/old.json', 'plugin/new.json', true); @@ -449,24 +416,25 @@ describe('FileSystemService', () => { expect(result).toEqual(expectedObject); }); - it('should return null when file does not exist', async () => { + it('should return Error when file does not exist', async () => { mockVaultService.getAbstractFileByPath = vi.fn().mockReturnValue(null); const result = await fileSystemService.readObjectFromFile('nonexistent.json'); - expect(result).toBeNull(); + expect(result).toBeInstanceOf(Error); + expect((result as Error).message).toContain('File not found'); }); - it('should return null when JSON is invalid', async () => { + it('should throw SyntaxError when JSON is invalid', async () => { const mockFile = createMockFile('invalid.json', 'json'); const invalidJson = '{name: "test", invalid}'; mockVaultService.getAbstractFileByPath = vi.fn().mockReturnValue(mockFile); mockVaultService.read = vi.fn().mockResolvedValue(invalidJson); - const result = await fileSystemService.readObjectFromFile('invalid.json'); - - expect(result).toBeNull(); + await expect(async () => { + await fileSystemService.readObjectFromFile('invalid.json'); + }).rejects.toThrow(SyntaxError); }); it('should handle nested objects', async () => { @@ -513,13 +481,14 @@ describe('FileSystemService', () => { it('should serialize and write object to new file', async () => { const data = { name: 'test', value: 42 }; const expectedJson = JSON.stringify(data, null, 4); + const mockFile = createMockFile('data.json', 'json'); mockVaultService.getAbstractFileByPath = vi.fn().mockReturnValue(null); - mockVaultService.create = vi.fn().mockResolvedValue(undefined); + mockVaultService.create = vi.fn().mockResolvedValue(mockFile); const result = await fileSystemService.writeObjectToFile('data.json', data); - expect(result).toBe(true); + expect(result).toBe(mockFile); expect(mockVaultService.create).toHaveBeenCalledWith('data.json', expectedJson, false); }); @@ -529,11 +498,11 @@ describe('FileSystemService', () => { const expectedJson = JSON.stringify(data, null, 4); mockVaultService.getAbstractFileByPath = vi.fn().mockReturnValue(mockFile); - mockVaultService.modify = vi.fn().mockResolvedValue(undefined); + mockVaultService.modify = vi.fn().mockResolvedValue(mockFile); const result = await fileSystemService.writeObjectToFile('existing.json', data); - expect(result).toBe(true); + expect(result).toBe(mockFile); expect(mockVaultService.modify).toHaveBeenCalledWith(mockFile, expectedJson, false); }); @@ -555,26 +524,28 @@ describe('FileSystemService', () => { it('should handle empty objects', async () => { const data = {}; const expectedJson = JSON.stringify(data, null, 4); + const mockFile = createMockFile('empty.json', 'json'); mockVaultService.getAbstractFileByPath = vi.fn().mockReturnValue(null); - mockVaultService.create = vi.fn().mockResolvedValue(undefined); + mockVaultService.create = vi.fn().mockResolvedValue(mockFile); const result = await fileSystemService.writeObjectToFile('empty.json', data); - expect(result).toBe(true); + expect(result).toBe(mockFile); expect(mockVaultService.create).toHaveBeenCalledWith('empty.json', expectedJson, false); }); it('should handle arrays', async () => { const data = [1, 2, 3, 4, 5]; const expectedJson = JSON.stringify(data, null, 4); + const mockFile = createMockFile('array.json', 'json'); mockVaultService.getAbstractFileByPath = vi.fn().mockReturnValue(null); - mockVaultService.create = vi.fn().mockResolvedValue(undefined); + mockVaultService.create = vi.fn().mockResolvedValue(mockFile); const result = await fileSystemService.writeObjectToFile('array.json', data); - expect(result).toBe(true); + expect(result).toBe(mockFile); expect(mockVaultService.create).toHaveBeenCalledWith('array.json', expectedJson, false); }); @@ -591,17 +562,16 @@ describe('FileSystemService', () => { expect(mockVaultService.create).toHaveBeenCalledWith('plugin/config.json', expectedJson, true); }); - it('should return false on write error', async () => { + it('should return Error on write error', async () => { const data = { test: 'value' }; const error = new Error('Write failed'); mockVaultService.getAbstractFileByPath = vi.fn().mockReturnValue(null); - mockVaultService.create = vi.fn().mockRejectedValue(error); + mockVaultService.create = vi.fn().mockResolvedValue(error); const result = await fileSystemService.writeObjectToFile('error.json', data); - expect(result).toBe(false); - expect(consoleErrorSpy).toHaveBeenCalledWith('Error writing JSON file:', error); + expect(result).toBe(error); }); }); diff --git a/__tests__/Services/SanitiserService.test.ts b/__tests__/Services/SanitiserService.test.ts index 41d9bc0..43be55b 100644 --- a/__tests__/Services/SanitiserService.test.ts +++ b/__tests__/Services/SanitiserService.test.ts @@ -25,9 +25,10 @@ describe('SanitiserService', () => { }); it('should throw error when input is not a string', () => { - expect(() => service.sanitize(123 as any)).toThrow('Input must be a string'); - expect(() => service.sanitize(null as any)).toThrow('Input must be a string'); - expect(() => service.sanitize(undefined as any)).toThrow('Input must be a string'); + // New implementation relies on normalizePath which throws when input is not a string + expect(() => service.sanitize(123 as any)).toThrow(); + expect(() => service.sanitize(null as any)).toThrow(); + expect(() => service.sanitize(undefined as any)).toThrow(); }); it('should normalize empty string to vault root', () => { diff --git a/__tests__/Services/StreamingMarkdownService.test.ts b/__tests__/Services/StreamingMarkdownService.test.ts index d0c2b5f..3b999fc 100644 --- a/__tests__/Services/StreamingMarkdownService.test.ts +++ b/__tests__/Services/StreamingMarkdownService.test.ts @@ -2,19 +2,22 @@ import { describe, it, expect, beforeEach, vi, afterEach } from 'vitest'; import { StreamingMarkdownService } from '../../Services/StreamingMarkdownService'; import * as DependencyService from '../../Services/DependencyService'; import { Services } from '../../Services/Services'; -import type { FileSystemService } from '../../Services/FileSystemService'; import type { HTMLService } from '../../Services/HTMLService'; +import type { VaultCacheService } from '../../Services/VaultCacheService'; +import { Exception } from '../../Helpers/Exception'; describe('StreamingMarkdownService', () => { let service: StreamingMarkdownService; - let mockFileSystemService: Partial; let mockHTMLService: Partial; + let mockVaultCacheService: Partial; beforeEach(() => { - // Mock FileSystemService to avoid dependency injection issues - mockFileSystemService = { - getVaultFileListForMarkDown: vi.fn().mockReturnValue(['file1', 'file2', 'folder/file3']) - }; + // Mock VaultCacheService to provide wikiLinks + mockVaultCacheService = { + wikiLinks: { + links: ['file1', 'file2', 'folder/file3'] + } + } as any; // Mock HTMLService mockHTMLService = { @@ -41,8 +44,8 @@ describe('StreamingMarkdownService', () => { // Mock DependencyService.Resolve to return our mocks vi.spyOn(DependencyService, 'Resolve').mockImplementation((serviceId: symbol) => { - if (serviceId === Services.FileSystemService) { - return mockFileSystemService as FileSystemService; + if (serviceId === Services.VaultCacheService) { + return mockVaultCacheService as VaultCacheService; } if (serviceId === Services.HTMLService) { return mockHTMLService as HTMLService; @@ -50,6 +53,9 @@ describe('StreamingMarkdownService', () => { throw new Error(`Unexpected service request: ${serviceId.toString()}`); }); + // Mock Exception.log + vi.spyOn(Exception, 'log').mockImplementation(() => {}); + service = new StreamingMarkdownService(); }); @@ -444,15 +450,6 @@ describe('StreamingMarkdownService', () => { // Buffer should not update expect(state.buffer).toBe(''); }); - - it('should refresh vault file list on each chunk', () => { - const container = document.createElement('div'); - service.initializeStream('msg-1', container); - - service.streamChunk('msg-1', 'test'); - - expect(mockFileSystemService.getVaultFileListForMarkDown).toHaveBeenCalled(); - }); }); describe('streaming - finalizeStream', () => { @@ -554,12 +551,13 @@ describe('StreamingMarkdownService', () => { expect((testService as any).processor).not.toBeNull(); }); - it('should cache permalink list from file system', () => { + it('should use VaultCacheService wikiLinks for permalink resolution', () => { const testService = new StreamingMarkdownService(); - expect(mockFileSystemService.getVaultFileListForMarkDown).toHaveBeenCalled(); - expect((testService as any).cachedPermaLinks).toBeDefined(); - expect((testService as any).cachedPermaLinks).toEqual(['file1', 'file2', 'folder/file3']); + // Verify the service was initialized with VaultCacheService + expect((testService as any).vaultCacheService).toBeDefined(); + expect((testService as any).vaultCacheService.wikiLinks).toBeDefined(); + expect((testService as any).vaultCacheService.wikiLinks.links).toEqual(['file1', 'file2', 'folder/file3']); }); it('should initialize empty streaming states', () => { diff --git a/__tests__/Services/StreamingService.test.ts b/__tests__/Services/StreamingService.test.ts index 7150832..a4368a2 100644 --- a/__tests__/Services/StreamingService.test.ts +++ b/__tests__/Services/StreamingService.test.ts @@ -1,6 +1,7 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest'; import { StreamingService, IStreamChunk } from '../../Services/StreamingService'; import { Selector } from '../../Enums/Selector'; +import { Exception } from '../../Helpers/Exception'; /** * UNIT TESTS @@ -19,11 +20,17 @@ describe('StreamingService', () => { originalFetch = global.fetch; mockFetch = vi.fn(); global.fetch = mockFetch; + // Mock Exception methods to avoid console output during tests + vi.spyOn(Exception, 'log').mockImplementation(() => {}); + vi.spyOn(Exception, 'throw').mockImplementation((error: unknown) => { + throw Exception.new(error); + }); }); afterEach(() => { global.fetch = originalFetch; vi.clearAllMocks(); + vi.restoreAllMocks(); }); // Helper to create a mock ReadableStream diff --git a/__tests__/Services/VaultService.test.ts b/__tests__/Services/VaultService.test.ts index b66f44d..f7cad51 100644 --- a/__tests__/Services/VaultService.test.ts +++ b/__tests__/Services/VaultService.test.ts @@ -7,6 +7,7 @@ import { Services } from '../../Services/Services'; import { SanitiserService } from '../../Services/SanitiserService'; import { SettingsService, IVaultkeeperAISettings } from '../../Services/SettingsService'; import { AIProviderModel } from '../../Enums/ApiProvider'; +import { Exception } from '../../Helpers/Exception'; /** * INTEGRATION TESTS @@ -115,6 +116,9 @@ describe('VaultService - Integration Tests', () => { // Mock console.error to prevent noise in tests consoleErrorSpy = vi.spyOn(console, 'error').mockImplementation(() => {}); + // Mock Exception.log + vi.spyOn(Exception, 'log').mockImplementation(() => {}); + // Register real dependencies in DependencyService RegisterSingleton(Services.VaultkeeperAIPlugin, mockPlugin as any); RegisterSingleton(Services.FileManager, mockFileManager); @@ -131,7 +135,7 @@ describe('VaultService - Integration Tests', () => { afterEach(() => { // Clear singleton registry to prevent memory leaks DeregisterAllServices(); - consoleErrorSpy.mockRestore(); + vi.restoreAllMocks(); }); describe('getMarkdownFiles', () => { @@ -203,13 +207,12 @@ describe('VaultService - Integration Tests', () => { expect(result).toBe(mockFile); }); - it('should return null and log error when path is excluded', () => { + it('should return null when path is excluded', () => { mockVault.getAbstractFileByPath.mockReturnValue(createMockFile('Vaultkeeper AI/test.md')); const result = vaultService.getAbstractFileByPath('Vaultkeeper AI/test.md', false); expect(result).toBeNull(); - expect(consoleErrorSpy).toHaveBeenCalled(); }); it('should sanitize the path before checking', () => { @@ -240,7 +243,6 @@ describe('VaultService - Integration Tests', () => { const result = vaultService.getAbstractFileByPath('Vaultkeeper AI', false); expect(result).toBeNull(); - expect(consoleErrorSpy).toHaveBeenCalled(); }); it('should allow access to Vaultkeeper AI directory when allowAccessToPluginRoot is true', () => { @@ -266,7 +268,6 @@ describe('VaultService - Integration Tests', () => { const result = await vaultService.exists('Vaultkeeper AI/test.md', false); expect(result).toBe(false); - expect(consoleErrorSpy).toHaveBeenCalled(); }); it('should return false when file does not exist', async () => { @@ -297,13 +298,12 @@ describe('VaultService - Integration Tests', () => { expect(mockVault.read).toHaveBeenCalledWith(mockFile); }); - it('should return empty string and log error when file is excluded', async () => { + it('should return empty string when file is excluded', async () => { const mockFile = createMockFile('Vaultkeeper AI/test.md'); const result = await vaultService.read(mockFile, false); expect(result).toBe(''); - expect(consoleErrorSpy).toHaveBeenCalled(); expect(mockVault.read).not.toHaveBeenCalled(); }); @@ -331,10 +331,11 @@ describe('VaultService - Integration Tests', () => { expect(result).toBe(mockFile); }); - it('should throw error when trying to create file in excluded path', async () => { - await expect( - vaultService.create('Vaultkeeper AI/test.md', 'content', false) - ).rejects.toThrow('Plugin attempted to create a file that is in the exclusion list'); + it('should return error when trying to create file in excluded path', async () => { + const result = await vaultService.create('Vaultkeeper AI/test.md', 'content', false); + + expect(result).toBeInstanceOf(Error); + expect((result as Error).message).toContain('Failed to create file, permission denied'); }); it('should create parent directories if they do not exist', async () => { @@ -375,13 +376,12 @@ describe('VaultService - Integration Tests', () => { expect(mockVault.process).toHaveBeenCalledWith(mockFile, expect.any(Function)); }); - it('should not modify file and log error when file is excluded', async () => { + it('should not modify file when file is excluded', async () => { const mockFile = createMockFile('Vaultkeeper AI/test.md'); await vaultService.modify(mockFile, 'new content', false); expect(mockVault.process).not.toHaveBeenCalled(); - expect(consoleErrorSpy).toHaveBeenCalled(); }); it('should call vault.process with function that returns new content', async () => { @@ -406,7 +406,7 @@ describe('VaultService - Integration Tests', () => { const result = await vaultService.delete(mockFile); - expect(result).toEqual({ success: true }); + expect(result).toBeUndefined(); // void = success expect(mockFileManager.trashFile).toHaveBeenCalledWith(mockFile); }); @@ -415,9 +415,9 @@ describe('VaultService - Integration Tests', () => { const result = await vaultService.delete(mockFile, false); - expect(result).toEqual({ success: false, error: 'File is in exclusion list' }); + expect(result).toBeInstanceOf(Error); + expect((result as Error).message).toContain('File does not exist'); expect(mockFileManager.trashFile).not.toHaveBeenCalled(); - expect(consoleErrorSpy).toHaveBeenCalled(); }); it('should call fileManager.trashFile to delete file', async () => { @@ -435,8 +435,8 @@ describe('VaultService - Integration Tests', () => { const result = await vaultService.delete(mockFile); - expect(result).toEqual({ success: false, error: 'Deletion failed' }); - expect(consoleErrorSpy).toHaveBeenCalled(); + expect(result).toBeInstanceOf(Error); + expect((result as Error).message).toBe('Deletion failed'); }); it('should handle non-Error objects in catch block', async () => { @@ -445,7 +445,8 @@ describe('VaultService - Integration Tests', () => { const result = await vaultService.delete(mockFile); - expect(result).toEqual({ success: false, error: 'string error' }); + expect(result).toBeInstanceOf(Error); + expect((result as Error).message).toBe('string error'); }); }); @@ -457,16 +458,16 @@ describe('VaultService - Integration Tests', () => { const result = await vaultService.move('source.md', 'dest.md'); - expect(result).toEqual({ success: true }); + expect(result).toBeUndefined(); // void = success expect(mockFileManager.renameFile).toHaveBeenCalledWith(mockFile, 'dest.md'); }); it('should return error when source file is excluded', async () => { const result = await vaultService.move('Vaultkeeper AI/test.md', 'dest.md', false); - expect(result).toEqual({ success: false, error: 'Source file is in exclusion list' }); + expect(result).toBeInstanceOf(Error); + expect((result as Error).message).toContain('File does not exist'); expect(mockFileManager.renameFile).not.toHaveBeenCalled(); - expect(consoleErrorSpy).toHaveBeenCalled(); }); it('should return error when source file does not exist', async () => { @@ -474,7 +475,8 @@ describe('VaultService - Integration Tests', () => { const result = await vaultService.move('nonexistent.md', 'dest.md'); - expect(result).toEqual({ success: false, error: 'Source file not found' }); + expect(result).toBeInstanceOf(Error); + expect((result as Error).message).toContain('File does not exist'); expect(mockFileManager.renameFile).not.toHaveBeenCalled(); }); @@ -499,8 +501,8 @@ describe('VaultService - Integration Tests', () => { const result = await vaultService.move('source.md', 'dest.md'); - expect(result).toEqual({ success: false, error: 'Move failed' }); - expect(consoleErrorSpy).toHaveBeenCalled(); + expect(result).toBeInstanceOf(Error); + expect((result as Error).message).toBe('Move failed'); }); }); @@ -515,10 +517,11 @@ describe('VaultService - Integration Tests', () => { expect(result).toBe(mockFolder); }); - it('should throw error when trying to create folder in excluded path', async () => { - await expect( - vaultService.createFolder('Vaultkeeper AI/subfolder', false) - ).rejects.toThrow('Plugin attempted to create a folder that is in the exclusion list'); + it('should return error when trying to create folder in excluded path', async () => { + const result = await vaultService.createFolder('Vaultkeeper AI/subfolder', false); + + expect(result).toBeInstanceOf(Error); + expect((result as Error).message).toContain('Failed to create folder, permission denied'); }); });