From ccd2c3e0d8c58e84b8dad673faed9cee13b61688 Mon Sep 17 00:00:00 2001 From: Florian Egger Date: Thu, 7 May 2026 00:06:19 +0200 Subject: [PATCH] Refactor message handling in chat view and improve API error handling Simplify message construction and update logic in chat-view.js Add abort controller support and improve error handling in ollama-client.js Remove unused sanitizeFilePath function from utils.js Export LogLevel enum in utils.js Improve vault indexing to process all batches --- src/chat-view.js | 18 +-- src/constants.js | 1 - src/ollama-client.js | 49 ++++---- src/utils.js | 264 ++++++++++++++++++++++--------------------- src/vault-indexer.js | 3 - 5 files changed, 170 insertions(+), 165 deletions(-) diff --git a/src/chat-view.js b/src/chat-view.js index 561091b..941e381 100644 --- a/src/chat-view.js +++ b/src/chat-view.js @@ -239,14 +239,16 @@ class ChatView extends obsidian_1.ItemView { if (context.length > MAX_CONTEXT_LENGTH) { context = context.substring(0, MAX_CONTEXT_LENGTH) + '\n\n... (truncated)'; } + const systemContent = context + ? `You are a helpful assistant.\n\nRelevant vault context:\n${context}` + : 'You are a helpful assistant.'; const systemMessage = { role: 'system', - content: 'You are a helpful assistant.', + content: systemContent, }; - const userContent = context ? `${context}\n\n${userMessage}` : userMessage; const userMessageWithContext = { role: 'user', - content: userContent, + content: userMessage, }; const messages = [ systemMessage, @@ -368,13 +370,11 @@ class ChatView extends obsidian_1.ItemView { }); } } - // Update last message immutably — only if no tool calls were processed + // Update assistant message immutably — only if no tool calls were processed if (toolCalls.length === 0) { - const lastMessageIndex = this.messages.length - 1; - if (lastMessageIndex >= 0) { - const lastMessage = { ...this.messages[lastMessageIndex], isStreaming: false }; - this.messages = [...this.messages.slice(0, lastMessageIndex), lastMessage]; - } + this.updateMessageById(assistantMessageId, { + isStreaming: false, + }); } // Limit conversation history to prevent memory issues if (this.messages.length > MAX_MESSAGE_HISTORY) { diff --git a/src/constants.js b/src/constants.js index c02a151..5477057 100644 --- a/src/constants.js +++ b/src/constants.js @@ -9,4 +9,3 @@ exports.DEFAULT_SETTINGS = { maxMessageHistory: 50, lastIndexTime: 0, }; -// Model validation regex - lowercase letters, numbers, dashes, underscores only diff --git a/src/ollama-client.js b/src/ollama-client.js index 538664f..e4dc4b2 100644 --- a/src/ollama-client.js +++ b/src/ollama-client.js @@ -132,29 +132,36 @@ class OllamaClient { return this.chatWithRetry(messages, tools, 0); } async chatWithRetry(messages, tools = [], attempt = 0) { - const response = await this.fetchFn(`${this.baseURL}/api/chat`, { - method: 'POST', - headers: { - 'Content-Type': 'application/json', - }, - body: JSON.stringify({ - model: this.model, - messages, - tools, - stream: false, - }), - }); - if (!response.ok) { - if (response.status >= 500 && attempt < this.maxRetries) { - const retryDelay = Math.pow(2, attempt) * 100; - utils_1.Logger.warn(`Network error (status ${response.status}), retrying in ${retryDelay}ms (attempt ${attempt + 1}/${this.maxRetries})`, 'ollama-client'); - await new Promise((resolve) => setTimeout(resolve, retryDelay)); - return this.chatWithRetry(messages, tools, attempt + 1); + this.abortController = new AbortController(); + try { + const response = await this.fetchFn(`${this.baseURL}/api/chat`, { + method: 'POST', + headers: { + 'Content-Type': 'application/json', + }, + body: JSON.stringify({ + model: this.model, + messages, + tools, + stream: false, + }), + signal: this.abortController.signal, + }); + if (!response.ok) { + if (response.status >= 500 && attempt < this.maxRetries) { + const retryDelay = Math.pow(2, attempt) * 100; + utils_1.Logger.warn(`Network error (status ${response.status}), retrying in ${retryDelay}ms (attempt ${attempt + 1}/${this.maxRetries})`, 'ollama-client'); + await new Promise((resolve) => setTimeout(resolve, retryDelay)); + return this.chatWithRetry(messages, tools, attempt + 1); + } + throw new types_1.ApiError(`Ollama API error: ${response.status}`, response.status); } - throw new types_1.ApiError(`Ollama API error: ${response.status}`, response.status); + const data = (await response.json()); + return (this.toOllamaMessage(data.message) ?? { role: 'assistant', content: '', tool_calls: [] }); + } + finally { + this.abortController = null; } - const data = (await response.json()); - return this.toOllamaMessage(data.message) ?? { role: 'assistant', content: '', tool_calls: [] }; } throwIfOllamaError(parsed) { if (parsed.error) { diff --git a/src/utils.js b/src/utils.js index ea8e2af..59e6c35 100644 --- a/src/utils.js +++ b/src/utils.js @@ -1,165 +1,167 @@ -'use strict'; -// src/utils.ts -Object.defineProperty(exports, '__esModule', { value: true }); -exports.Logger = void 0; +"use strict"; +Object.defineProperty(exports, "__esModule", { value: true }); +exports.Logger = exports.LogLevel = void 0; exports.validateOllamaUrl = validateOllamaUrl; exports.validateModelName = validateModelName; exports.validatePluginSettings = validatePluginSettings; exports.safeParseJson = safeParseJson; -exports.sanitizeFilePath = sanitizeFilePath; - -// ==================== Logger ==================== var LogLevel; (function (LogLevel) { - LogLevel[(LogLevel['DEBUG'] = 0)] = 'DEBUG'; - LogLevel[(LogLevel['INFO'] = 1)] = 'INFO'; - LogLevel[(LogLevel['WARN'] = 2)] = 'WARN'; - LogLevel[(LogLevel['ERROR'] = 3)] = 'ERROR'; -})(LogLevel || (LogLevel = {})); + LogLevel[LogLevel["DEBUG"] = 0] = "DEBUG"; + LogLevel[LogLevel["INFO"] = 1] = "INFO"; + LogLevel[LogLevel["WARN"] = 2] = "WARN"; + LogLevel[LogLevel["ERROR"] = 3] = "ERROR"; +})(LogLevel || (exports.LogLevel = LogLevel = {})); const SEVERITY_ORDER = { - debug: LogLevel.DEBUG, - info: LogLevel.INFO, - warn: LogLevel.WARN, - error: LogLevel.ERROR, + debug: LogLevel.DEBUG, + info: LogLevel.INFO, + warn: LogLevel.WARN, + error: LogLevel.ERROR, }; class Logger { - static setLevel(level) { - if (typeof level === 'string') { - const lowerLevel = level.toLowerCase(); - Logger.minLevel = SEVERITY_ORDER[lowerLevel] ?? LogLevel.DEBUG; - } else { - Logger.minLevel = level; + static setLevel(level) { + if (typeof level === 'string') { + const lowerLevel = level.toLowerCase(); + Logger.minLevel = SEVERITY_ORDER[lowerLevel] ?? LogLevel.DEBUG; + } + else { + Logger.minLevel = level; + } } - } - static debug(message, category = 'general') { - if (LogLevel.DEBUG >= Logger.minLevel) { - console.debug(`[${category}] DEBUG: ${message}`); + static debug(message, category = 'general') { + if (LogLevel.DEBUG >= Logger.minLevel) { + console.debug(`[${category}] DEBUG: ${message}`); + } } - } - static info(message, category = 'general') { - if (LogLevel.INFO >= Logger.minLevel) { - console.info(`[${category}] INFO: ${message}`); + static info(message, category = 'general') { + if (LogLevel.INFO >= Logger.minLevel) { + console.info(`[${category}] INFO: ${message}`); + } } - } - static warn(message, category = 'general') { - if (LogLevel.WARN >= Logger.minLevel) { - console.warn(`[${category}] WARN: ${message}`); + static warn(message, category = 'general') { + if (LogLevel.WARN >= Logger.minLevel) { + console.warn(`[${category}] WARN: ${message}`); + } } - } - static error(message, category = 'general') { - if (LogLevel.ERROR >= Logger.minLevel) { - console.error(`[${category}] ERROR: ${message}`); + static error(message, category = 'general') { + if (LogLevel.ERROR >= Logger.minLevel) { + console.error(`[${category}] ERROR: ${message}`); + } } - } } exports.Logger = Logger; Logger.minLevel = LogLevel.DEBUG; // ==================== URL & Model Validation ==================== function validateOllamaUrl(url) { - if (typeof url !== 'string' || !url.trim()) { - return { valid: false, error: 'URL cannot be empty' }; - } - const trimmedUrl = url.trim(); - if (trimmedUrl.endsWith('/')) { - return { valid: false, error: 'URL should not end with a slash' }; - } - try { - const parsed = new URL(trimmedUrl); - if (parsed.protocol !== 'http:' && parsed.protocol !== 'https:') { - return { valid: false, error: 'Must be a valid HTTP or HTTPS URL' }; + if (typeof url !== 'string' || !url.trim()) { + return { valid: false, error: 'URL cannot be empty' }; + } + const trimmedUrl = url.trim(); + if (trimmedUrl.endsWith('/')) { + return { valid: false, error: 'URL should not end with a slash' }; + } + try { + const parsed = new URL(trimmedUrl); + if (parsed.protocol !== 'http:' && parsed.protocol !== 'https:') { + return { valid: false, error: 'Must be a valid HTTP or HTTPS URL' }; + } + return { valid: true }; + } + catch { + return { valid: false, error: 'Must be a valid HTTP or HTTPS URL' }; } - return { valid: true }; - } catch { - return { valid: false, error: 'Must be a valid HTTP or HTTPS URL' }; - } } function validateModelName(model) { - if (typeof model !== 'string') { - return { valid: false, error: 'Model name must be a string' }; - } - const trimmedModel = model.trim(); - // Explicit check for empty string after trimming - if (!trimmedModel || trimmedModel.length === 0) { - return { valid: false, error: 'Model name cannot be empty' }; - } - if (trimmedModel.length < 2) { - return { valid: false, error: 'Model name must be at least 2 characters long' }; - } - if (trimmedModel.length > 100) { - return { valid: false, error: 'Model name must be less than 100 characters long' }; - } - if (!/^[a-zA-Z0-9._:-]+$/.test(trimmedModel)) { - return { - valid: false, - error: 'Model name can only contain letters, numbers, dots, dashes, underscores, and colons', - }; - } - return { valid: true }; + if (typeof model !== 'string') { + return { valid: false, error: 'Model name must be a string' }; + } + const trimmedModel = model.trim(); + // Explicit check for empty string after trimming + if (!trimmedModel || trimmedModel.length === 0) { + return { valid: false, error: 'Model name cannot be empty' }; + } + if (trimmedModel.length < 2) { + return { valid: false, error: 'Model name must be at least 2 characters long' }; + } + if (trimmedModel.length > 100) { + return { valid: false, error: 'Model name must be less than 100 characters long' }; + } + if (!/^[a-zA-Z0-9._:-]+$/.test(trimmedModel)) { + return { + valid: false, + error: 'Model name can only contain letters, numbers, dots, dashes, underscores, and colons', + }; + } + return { valid: true }; } function validatePluginSettings(settings) { - const errors = []; - const urlValidation = validateOllamaUrl(settings.ollamaUrl); - if (!urlValidation.valid) { - errors.push(`Invalid Ollama URL: ${urlValidation.error}`); - } - const modelValidation = validateModelName(settings.model); - if (!modelValidation.valid) { - errors.push(`Invalid Model Name: ${modelValidation.error}`); - } - return errors; + const errors = []; + const urlValidation = validateOllamaUrl(settings.ollamaUrl); + if (!urlValidation.valid) { + errors.push(`Invalid Ollama URL: ${urlValidation.error}`); + } + const modelValidation = validateModelName(settings.model); + if (!modelValidation.valid) { + errors.push(`Invalid Model Name: ${modelValidation.error}`); + } + return errors; } // ==================== Safe JSON Parsing ==================== const MAX_JSON_SIZE = 1000000; const MAX_JSON_NESTING = 24; function countNestingDepth(value, depth = 0) { - if (depth > MAX_JSON_NESTING) { + if (depth > MAX_JSON_NESTING) { + return depth; + } + if (Array.isArray(value)) { + return Math.max(...value.map((item) => countNestingDepth(item, depth + 1)), depth); + } + if (value !== null && typeof value === 'object') { + const entries = Object.values(value); + if (entries.length === 0) + return depth; + return Math.max(...entries.map((item) => countNestingDepth(item, depth + 1)), depth); + } return depth; - } - if (Array.isArray(value)) { - return Math.max(...value.map((item) => countNestingDepth(item, depth + 1)), depth); - } - if (value !== null && typeof value === 'object') { - const entries = Object.values(value); - if (entries.length === 0) return depth; - return Math.max(...entries.map((item) => countNestingDepth(item, depth + 1)), depth); - } - return depth; } function safeParseJson(jsonString) { - if (typeof jsonString !== 'string') { - throw new Error('Input must be a string'); - } - if (jsonString.length > MAX_JSON_SIZE) { - throw new Error('JSON input too large'); - } - let parsed; - try { - parsed = JSON.parse(jsonString); - } catch { - throw new Error('Invalid JSON'); - } - // Check for dangerous prototype pollution patterns - const reStringified = JSON.stringify(parsed); - if ( - reStringified.includes('constructor') || - reStringified.includes('prototype') || - reStringified.includes('__proto__') || - reStringified.includes('function') - ) { - throw new Error('dangerous code pattern detected'); - } - // Check nesting depth - if (countNestingDepth(parsed) > MAX_JSON_NESTING) { - throw new Error('JSON nesting too deep'); - } - return parsed; + if (typeof jsonString !== 'string') { + throw new Error('Input must be a string'); + } + if (jsonString.length > MAX_JSON_SIZE) { + throw new Error('JSON input too large'); + } + let parsed; + try { + parsed = JSON.parse(jsonString); + } + catch { + throw new Error('Invalid JSON'); + } + // Check for dangerous prototype pollution patterns in object keys only + const checkDangerousPatterns = (obj) => { + if (typeof obj !== 'object' || obj === null) { + return false; + } + const dangerousKeys = ['constructor', 'prototype', '__proto__']; + if (dangerousKeys.some((key) => Object.keys(obj).includes(key))) { + return true; + } + // Recursively check nested objects + for (const key in obj) { + if (checkDangerousPatterns(obj[key])) { + return true; + } + } + return false; + }; + if (checkDangerousPatterns(parsed)) { + throw new Error('dangerous code pattern detected'); + } + // Check nesting depth + if (countNestingDepth(parsed) > MAX_JSON_NESTING) { + throw new Error('JSON nesting too deep'); + } + return parsed; } -// ==================== Path & File Utilities ==================== -function sanitizeFilePath(path) { - if (path.includes('..')) { - throw new Error('Invalid path - cannot contain .. segments'); - } - return path; -} - // ==================== Markdown Utilities ==================== diff --git a/src/vault-indexer.js b/src/vault-indexer.js index e74353e..8bb2824 100644 --- a/src/vault-indexer.js +++ b/src/vault-indexer.js @@ -57,9 +57,6 @@ class VaultIndexer { })); const validResults = batchResults.filter((result) => result !== null); results.push(...validResults); - if (results.length >= 50) { - break; - } } return results; }