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'); }); });