From ca29f86cd7409a52a66860b35bdf373401bb0239 Mon Sep 17 00:00:00 2001 From: Stephen Dolan Date: Tue, 10 Mar 2026 16:10:23 -0400 Subject: [PATCH 1/2] fix(mcp): return structured error results instead of raw stdout writes API errors were bypassing the MCP protocol by writing multi-line JSON to stdout and calling process.exit(1), breaking stdio transports that read line-by-line. Errors now propagate from the client and are caught by MCP tool handlers as proper isError results. Closes #34 Co-Authored-By: Claude Opus 4.6 --- src/lib/api-client.ts | 498 ++++++++++++++++++------------------------ src/mcp/server.ts | 38 ++++ 2 files changed, 253 insertions(+), 283 deletions(-) diff --git a/src/lib/api-client.ts b/src/lib/api-client.ts index 391e2ee..79c47a0 100644 --- a/src/lib/api-client.ts +++ b/src/lib/api-client.ts @@ -1,6 +1,6 @@ import * as ynab from 'ynab'; import { config } from './config.js'; -import { YnabCliError, handleYnabError, sanitizeApiError } from './errors.js'; +import { YnabCliError, sanitizeApiError } from './errors.js'; import { auth } from './auth.js'; type TransactionTypeFilter = 'uncategorized' | 'unapproved' | undefined; @@ -55,94 +55,70 @@ export class YnabClient { return budgetId; } - private async withErrorHandling(fn: () => Promise): Promise { - try { - return await fn(); - } catch (error) { - handleYnabError(error); - } - } - async getUser() { - return this.withErrorHandling(async () => { - const api = await this.getApi(); - const response = await api.user.getUser(); - return response.data.user; - }); + const api = await this.getApi(); + const response = await api.user.getUser(); + return response.data.user; } async getBudgets(includeAccounts = false) { - return this.withErrorHandling(async () => { - const api = await this.getApi(); - const response = await api.budgets.getBudgets(includeAccounts); - return { - budgets: response.data.budgets, - server_knowledge: 0, - }; - }); + const api = await this.getApi(); + const response = await api.budgets.getBudgets(includeAccounts); + return { + budgets: response.data.budgets, + server_knowledge: 0, + }; } async getBudget(budgetId?: string, lastKnowledgeOfServer?: number) { - return this.withErrorHandling(async () => { - const api = await this.getApi(); - const id = await this.getBudgetId(budgetId); - const response = await api.budgets.getBudgetById(id, lastKnowledgeOfServer); - return { - budget: response.data.budget, - server_knowledge: response.data.server_knowledge, - }; - }); + const api = await this.getApi(); + const id = await this.getBudgetId(budgetId); + const response = await api.budgets.getBudgetById(id, lastKnowledgeOfServer); + return { + budget: response.data.budget, + server_knowledge: response.data.server_knowledge, + }; } async getBudgetSettings(budgetId?: string) { - return this.withErrorHandling(async () => { - const api = await this.getApi(); - const id = await this.getBudgetId(budgetId); - const response = await api.budgets.getBudgetSettingsById(id); - return response.data.settings; - }); + const api = await this.getApi(); + const id = await this.getBudgetId(budgetId); + const response = await api.budgets.getBudgetSettingsById(id); + return response.data.settings; } async getAccounts(budgetId?: string, lastKnowledgeOfServer?: number) { - return this.withErrorHandling(async () => { - const api = await this.getApi(); - const id = await this.getBudgetId(budgetId); - const response = await api.accounts.getAccounts(id, lastKnowledgeOfServer); - return { - accounts: response.data.accounts, - server_knowledge: response.data.server_knowledge, - }; - }); + const api = await this.getApi(); + const id = await this.getBudgetId(budgetId); + const response = await api.accounts.getAccounts(id, lastKnowledgeOfServer); + return { + accounts: response.data.accounts, + server_knowledge: response.data.server_knowledge, + }; } async getAccount(accountId: string, budgetId?: string) { - return this.withErrorHandling(async () => { - const api = await this.getApi(); - const id = await this.getBudgetId(budgetId); - const response = await api.accounts.getAccountById(id, accountId); - return response.data.account; - }); + const api = await this.getApi(); + const id = await this.getBudgetId(budgetId); + const response = await api.accounts.getAccountById(id, accountId); + return response.data.account; } async getCategories(budgetId?: string, lastKnowledgeOfServer?: number) { - return this.withErrorHandling(async () => { - const api = await this.getApi(); - const id = await this.getBudgetId(budgetId); - const response = await api.categories.getCategories(id, lastKnowledgeOfServer); - return { - category_groups: response.data.category_groups, - server_knowledge: response.data.server_knowledge, - }; - }); + const api = await this.getApi(); + const id = await this.getBudgetId(budgetId); + const response = await api.categories.getCategories(id, lastKnowledgeOfServer); + return { + category_groups: response.data.category_groups, + server_knowledge: response.data.server_knowledge, + }; } async getCategory(categoryId: string, budgetId?: string) { - return this.withErrorHandling(async () => { - const api = await this.getApi(); - const id = await this.getBudgetId(budgetId); - const response = await api.categories.getCategoryById(id, categoryId); - return response.data.category; - }); + const api = await this.getApi(); + const id = await this.getBudgetId(budgetId); + const response = await api.categories.getCategoryById(id, categoryId); + return response.data.category; } async updateMonthCategory( @@ -151,81 +127,65 @@ export class YnabClient { data: ynab.PatchMonthCategoryWrapper, budgetId?: string ) { - return this.withErrorHandling(async () => { - const api = await this.getApi(); - const id = await this.getBudgetId(budgetId); - const response = await api.categories.updateMonthCategory(id, month, categoryId, data); - return response.data.category; - }); + const api = await this.getApi(); + const id = await this.getBudgetId(budgetId); + const response = await api.categories.updateMonthCategory(id, month, categoryId, data); + return response.data.category; } async updateCategory(categoryId: string, data: ynab.PatchCategoryWrapper, budgetId?: string) { - return this.withErrorHandling(async () => { - const api = await this.getApi(); - const id = await this.getBudgetId(budgetId); - const response = await api.categories.updateCategory(id, categoryId, data); - return response.data.category; - }); + const api = await this.getApi(); + const id = await this.getBudgetId(budgetId); + const response = await api.categories.updateCategory(id, categoryId, data); + return response.data.category; } async getPayees(budgetId?: string, lastKnowledgeOfServer?: number) { - return this.withErrorHandling(async () => { - const api = await this.getApi(); - const id = await this.getBudgetId(budgetId); - const response = await api.payees.getPayees(id, lastKnowledgeOfServer); - return { - payees: response.data.payees, - server_knowledge: response.data.server_knowledge, - }; - }); + const api = await this.getApi(); + const id = await this.getBudgetId(budgetId); + const response = await api.payees.getPayees(id, lastKnowledgeOfServer); + return { + payees: response.data.payees, + server_knowledge: response.data.server_knowledge, + }; } async getPayee(payeeId: string, budgetId?: string) { - return this.withErrorHandling(async () => { - const api = await this.getApi(); - const id = await this.getBudgetId(budgetId); - const response = await api.payees.getPayeeById(id, payeeId); - return response.data.payee; - }); + const api = await this.getApi(); + const id = await this.getBudgetId(budgetId); + const response = await api.payees.getPayeeById(id, payeeId); + return response.data.payee; } async updatePayee(payeeId: string, data: ynab.PatchPayeeWrapper, budgetId?: string) { - return this.withErrorHandling(async () => { - const api = await this.getApi(); - const id = await this.getBudgetId(budgetId); - const response = await api.payees.updatePayee(id, payeeId, data); - return response.data.payee; - }); + const api = await this.getApi(); + const id = await this.getBudgetId(budgetId); + const response = await api.payees.updatePayee(id, payeeId, data); + return response.data.payee; } async getPayeeLocationsByPayee(payeeId: string, budgetId?: string) { - return this.withErrorHandling(async () => { - const api = await this.getApi(); - const id = await this.getBudgetId(budgetId); - const response = await api.payeeLocations.getPayeeLocationsByPayee(id, payeeId); - return response.data.payee_locations; - }); + const api = await this.getApi(); + const id = await this.getBudgetId(budgetId); + const response = await api.payeeLocations.getPayeeLocationsByPayee(id, payeeId); + return response.data.payee_locations; } async getBudgetMonths(budgetId?: string, lastKnowledgeOfServer?: number) { - return this.withErrorHandling(async () => { - const api = await this.getApi(); - const id = await this.getBudgetId(budgetId); - const response = await api.months.getBudgetMonths(id, lastKnowledgeOfServer); - return { - months: response.data.months, - server_knowledge: response.data.server_knowledge, - }; - }); + const api = await this.getApi(); + const id = await this.getBudgetId(budgetId); + const response = await api.months.getBudgetMonths(id, lastKnowledgeOfServer); + return { + months: response.data.months, + server_knowledge: response.data.server_knowledge, + }; } async getBudgetMonth(month: string, budgetId?: string) { - return this.withErrorHandling(async () => { - const api = await this.getApi(); - const id = await this.getBudgetId(budgetId); - const response = await api.months.getBudgetMonth(id, month); - return response.data.month; - }); + const api = await this.getApi(); + const id = await this.getBudgetId(budgetId); + const response = await api.months.getBudgetMonth(id, month); + return response.data.month; } async getTransactions(params: { @@ -234,20 +194,18 @@ export class YnabClient { type?: string; lastKnowledgeOfServer?: number; }) { - return this.withErrorHandling(async () => { - const api = await this.getApi(); - const id = await this.getBudgetId(params.budgetId); - const response = await api.transactions.getTransactions( - id, - params.sinceDate, - params.type as TransactionTypeFilter, - params.lastKnowledgeOfServer - ); - return { - transactions: response.data.transactions, - server_knowledge: response.data.server_knowledge, - }; - }); + const api = await this.getApi(); + const id = await this.getBudgetId(params.budgetId); + const response = await api.transactions.getTransactions( + id, + params.sinceDate, + params.type as TransactionTypeFilter, + params.lastKnowledgeOfServer + ); + return { + transactions: response.data.transactions, + server_knowledge: response.data.server_knowledge, + }; } async getTransactionsByAccount( @@ -259,21 +217,19 @@ export class YnabClient { lastKnowledgeOfServer?: number; } ) { - return this.withErrorHandling(async () => { - const api = await this.getApi(); - const id = await this.getBudgetId(params.budgetId); - const response = await api.transactions.getTransactionsByAccount( - id, - accountId, - params.sinceDate, - params.type as TransactionTypeFilter, - params.lastKnowledgeOfServer - ); - return { - transactions: response.data.transactions, - server_knowledge: response.data.server_knowledge, - }; - }); + const api = await this.getApi(); + const id = await this.getBudgetId(params.budgetId); + const response = await api.transactions.getTransactionsByAccount( + id, + accountId, + params.sinceDate, + params.type as TransactionTypeFilter, + params.lastKnowledgeOfServer + ); + return { + transactions: response.data.transactions, + server_knowledge: response.data.server_knowledge, + }; } async getTransactionsByCategory( @@ -285,21 +241,19 @@ export class YnabClient { lastKnowledgeOfServer?: number; } ) { - return this.withErrorHandling(async () => { - const api = await this.getApi(); - const id = await this.getBudgetId(params.budgetId); - const response = await api.transactions.getTransactionsByCategory( - id, - categoryId, - params.sinceDate, - params.type as TransactionTypeFilter, - params.lastKnowledgeOfServer - ); - return { - transactions: response.data.transactions, - server_knowledge: response.data.server_knowledge, - }; - }); + const api = await this.getApi(); + const id = await this.getBudgetId(params.budgetId); + const response = await api.transactions.getTransactionsByCategory( + id, + categoryId, + params.sinceDate, + params.type as TransactionTypeFilter, + params.lastKnowledgeOfServer + ); + return { + transactions: response.data.transactions, + server_knowledge: response.data.server_knowledge, + }; } async getTransactionsByPayee( @@ -311,39 +265,33 @@ export class YnabClient { lastKnowledgeOfServer?: number; } ) { - return this.withErrorHandling(async () => { - const api = await this.getApi(); - const id = await this.getBudgetId(params.budgetId); - const response = await api.transactions.getTransactionsByPayee( - id, - payeeId, - params.sinceDate, - params.type as TransactionTypeFilter, - params.lastKnowledgeOfServer - ); - return { - transactions: response.data.transactions, - server_knowledge: response.data.server_knowledge, - }; - }); + const api = await this.getApi(); + const id = await this.getBudgetId(params.budgetId); + const response = await api.transactions.getTransactionsByPayee( + id, + payeeId, + params.sinceDate, + params.type as TransactionTypeFilter, + params.lastKnowledgeOfServer + ); + return { + transactions: response.data.transactions, + server_knowledge: response.data.server_knowledge, + }; } async getTransaction(transactionId: string, budgetId?: string) { - return this.withErrorHandling(async () => { - const api = await this.getApi(); - const id = await this.getBudgetId(budgetId); - const response = await api.transactions.getTransactionById(id, transactionId); - return response.data.transaction; - }); + const api = await this.getApi(); + const id = await this.getBudgetId(budgetId); + const response = await api.transactions.getTransactionById(id, transactionId); + return response.data.transaction; } async createTransaction(transactionData: ynab.PostTransactionsWrapper, budgetId?: string) { - return this.withErrorHandling(async () => { - const api = await this.getApi(); - const id = await this.getBudgetId(budgetId); - const response = await api.transactions.createTransaction(id, transactionData); - return response.data.transaction; - }); + const api = await this.getApi(); + const id = await this.getBudgetId(budgetId); + const response = await api.transactions.createTransaction(id, transactionData); + return response.data.transaction; } async updateTransaction( @@ -351,122 +299,106 @@ export class YnabClient { transactionData: ynab.PutTransactionWrapper, budgetId?: string ) { - return this.withErrorHandling(async () => { - const api = await this.getApi(); - const id = await this.getBudgetId(budgetId); - const response = await api.transactions.updateTransaction(id, transactionId, transactionData); - return response.data.transaction; - }); + const api = await this.getApi(); + const id = await this.getBudgetId(budgetId); + const response = await api.transactions.updateTransaction(id, transactionId, transactionData); + return response.data.transaction; } async updateTransactions( transactions: ynab.PatchTransactionsWrapper, budgetId?: string ) { - return this.withErrorHandling(async () => { - const api = await this.getApi(); - const id = await this.getBudgetId(budgetId); - const response = await api.transactions.updateTransactions(id, transactions); - return { - transactions: response.data.transactions, - transaction_ids: response.data.transaction_ids, - server_knowledge: response.data.server_knowledge, - }; - }); + const api = await this.getApi(); + const id = await this.getBudgetId(budgetId); + const response = await api.transactions.updateTransactions(id, transactions); + return { + transactions: response.data.transactions, + transaction_ids: response.data.transaction_ids, + server_knowledge: response.data.server_knowledge, + }; } async deleteTransaction(transactionId: string, budgetId?: string) { - return this.withErrorHandling(async () => { - const api = await this.getApi(); - const id = await this.getBudgetId(budgetId); - const response = await api.transactions.deleteTransaction(id, transactionId); - return response.data.transaction; - }); + const api = await this.getApi(); + const id = await this.getBudgetId(budgetId); + const response = await api.transactions.deleteTransaction(id, transactionId); + return response.data.transaction; } async importTransactions(budgetId?: string) { - return this.withErrorHandling(async () => { - const api = await this.getApi(); - const id = await this.getBudgetId(budgetId); - const response = await api.transactions.importTransactions(id); - return response.data.transaction_ids; - }); + const api = await this.getApi(); + const id = await this.getBudgetId(budgetId); + const response = await api.transactions.importTransactions(id); + return response.data.transaction_ids; } async getScheduledTransactions(budgetId?: string, lastKnowledgeOfServer?: number) { - return this.withErrorHandling(async () => { - const api = await this.getApi(); - const id = await this.getBudgetId(budgetId); - const response = await api.scheduledTransactions.getScheduledTransactions( - id, - lastKnowledgeOfServer - ); - return { - scheduled_transactions: response.data.scheduled_transactions, - server_knowledge: response.data.server_knowledge, - }; - }); + const api = await this.getApi(); + const id = await this.getBudgetId(budgetId); + const response = await api.scheduledTransactions.getScheduledTransactions( + id, + lastKnowledgeOfServer + ); + return { + scheduled_transactions: response.data.scheduled_transactions, + server_knowledge: response.data.server_knowledge, + }; } async getScheduledTransaction(scheduledTransactionId: string, budgetId?: string) { - return this.withErrorHandling(async () => { - const api = await this.getApi(); - const id = await this.getBudgetId(budgetId); - const response = await api.scheduledTransactions.getScheduledTransactionById( - id, - scheduledTransactionId - ); - return response.data.scheduled_transaction; - }); + const api = await this.getApi(); + const id = await this.getBudgetId(budgetId); + const response = await api.scheduledTransactions.getScheduledTransactionById( + id, + scheduledTransactionId + ); + return response.data.scheduled_transaction; } async deleteScheduledTransaction(scheduledTransactionId: string, budgetId?: string) { - return this.withErrorHandling(async () => { - const api = await this.getApi(); - const id = await this.getBudgetId(budgetId); - const response = await api.scheduledTransactions.deleteScheduledTransaction( - id, - scheduledTransactionId - ); - return response.data.scheduled_transaction; - }); + const api = await this.getApi(); + const id = await this.getBudgetId(budgetId); + const response = await api.scheduledTransactions.deleteScheduledTransaction( + id, + scheduledTransactionId + ); + return response.data.scheduled_transaction; } async rawApiCall(method: string, path: string, data?: unknown, budgetId?: string) { - return this.withErrorHandling(async () => { - await this.getApi(); - - const fullPath = path.includes('{budget_id}') - ? path.replace('{budget_id}', await this.getBudgetId(budgetId)) - : path; - - const url = `https://api.ynab.com/v1${fullPath}`; - const accessToken = (await auth.getAccessToken()) || process.env.YNAB_API_KEY; - const headers = { - Authorization: `Bearer ${accessToken}`, - 'Content-Type': 'application/json', - }; - - const httpMethod = method.toUpperCase(); - const hasBody = ['POST', 'PUT', 'PATCH'].includes(httpMethod); - - if (!['GET', 'POST', 'PUT', 'PATCH', 'DELETE'].includes(httpMethod)) { - throw new YnabCliError(`Unsupported HTTP method: ${method}`, 400); - } - - const response = await fetch(url, { - method: httpMethod, - headers, - ...(hasBody && { body: JSON.stringify(data) }), - }); - - if (!response.ok) { - const errorData = (await response.json()) as Record; - throw { error: sanitizeApiError(errorData.error || errorData) }; - } - - return await response.json(); + await this.getApi(); + + const fullPath = path.includes('{budget_id}') + ? path.replace('{budget_id}', await this.getBudgetId(budgetId)) + : path; + + const url = `https://api.ynab.com/v1${fullPath}`; + const accessToken = (await auth.getAccessToken()) || process.env.YNAB_API_KEY; + const headers = { + Authorization: `Bearer ${accessToken}`, + 'Content-Type': 'application/json', + }; + + const httpMethod = method.toUpperCase(); + const hasBody = ['POST', 'PUT', 'PATCH'].includes(httpMethod); + + if (!['GET', 'POST', 'PUT', 'PATCH', 'DELETE'].includes(httpMethod)) { + throw new YnabCliError(`Unsupported HTTP method: ${method}`, 400); + } + + const response = await fetch(url, { + method: httpMethod, + headers, + ...(hasBody && { body: JSON.stringify(data) }), }); + + if (!response.ok) { + const errorData = (await response.json()) as Record; + throw { error: sanitizeApiError(errorData.error || errorData) }; + } + + return await response.json(); } } diff --git a/src/mcp/server.ts b/src/mcp/server.ts index 749e3fb..62807a1 100644 --- a/src/mcp/server.ts +++ b/src/mcp/server.ts @@ -3,6 +3,7 @@ import { StdioServerTransport } from '@modelcontextprotocol/sdk/server/stdio.js' import { z } from 'zod'; import { client } from '../lib/api-client.js'; import { auth } from '../lib/auth.js'; +import { YnabCliError, sanitizeApiError, sanitizeErrorMessage } from '../lib/errors.js'; import { amountToMilliunits, applyFieldSelection, applyTransactionFilters, convertMilliunitsToAmounts, summarizeTransactions, findTransferCandidates, type SummaryTransaction, type TransactionLike } from '../lib/utils.js'; const toolRegistry = [ @@ -45,6 +46,43 @@ const server = new McpServer({ version: '1.0.0', }); +function mcpErrorResult(error: unknown) { + let errorBody: Record; + + if (typeof error === 'object' && error !== null && 'error' in error) { + const apiError = sanitizeApiError((error as { error: unknown }).error); + errorBody = { name: apiError.name, detail: apiError.detail }; + } else if (error instanceof YnabCliError) { + errorBody = { name: 'cli_error', detail: sanitizeErrorMessage(error.message) }; + } else if (error instanceof Error) { + errorBody = { name: error.name, detail: sanitizeErrorMessage(error.message) }; + } else { + errorBody = { name: 'unknown_error', detail: 'An unexpected error occurred' }; + } + + return { + content: [{ type: 'text' as const, text: JSON.stringify({ error: errorBody }) }], + isError: true as const, + }; +} + +// Wrap all tool handlers so errors become structured MCP error results +// instead of propagating as unhandled exceptions +const _serverTool = server.tool.bind(server); +(server.tool as unknown) = (...args: unknown[]) => { + const handler = args[args.length - 1]; + if (typeof handler === 'function') { + args[args.length - 1] = async (...handlerArgs: unknown[]) => { + try { + return await (handler as Function)(...handlerArgs); + } catch (error) { + return mcpErrorResult(error); + } + }; + } + return (_serverTool as Function).apply(null, args); +}; + function jsonResponse(data: unknown) { return { content: [{ type: 'text' as const, text: JSON.stringify(data, null, 2) }] }; } From ffbee833466243c32ab27120e078690697e51d6c Mon Sep 17 00:00:00 2001 From: Stephen Dolan Date: Tue, 10 Mar 2026 16:10:48 -0400 Subject: [PATCH 2/2] chore(release): v2.8.1 Co-Authored-By: Claude Opus 4.6 --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 42ef017..0562dea 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@stephendolan/ynab-cli", - "version": "2.8.0", + "version": "2.8.1", "description": "A command-line interface for You Need a Budget (YNAB)", "type": "module", "main": "./dist/cli.js",