Skip to content

Commit dd992ef

Browse files
correctly propagate errors from normalizeError
1 parent 3545de4 commit dd992ef

File tree

10 files changed

+210
-58
lines changed

10 files changed

+210
-58
lines changed

index.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ import { ListToolsRequestSchema, CallToolRequestSchema } from "@modelcontextprot
1010
const server = new Server(
1111
{
1212
name: "task-manager-server",
13-
version: "1.2.0"
13+
version: "1.3.0"
1414
},
1515
{
1616
capabilities: {

package-lock.json

Lines changed: 13 additions & 13 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
{
22
"name": "taskqueue-mcp",
3-
"version": "1.2.0",
3+
"version": "1.3.0",
44
"description": "Task Queue MCP Server",
55
"author": "Christopher C. Smith ([email protected])",
66
"main": "dist/index.js",

src/client/cli.ts

Lines changed: 5 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -9,14 +9,13 @@ import {
99
import { TaskManager } from "../server/TaskManager.js";
1010
import { createError, normalizeError } from "../utils/errors.js";
1111
import { formatCliError } from "./errors.js";
12-
import fs from "fs/promises";
1312

1413
const program = new Command();
1514

1615
program
1716
.name("taskqueue")
1817
.description("CLI for the Task Manager MCP Server")
19-
.version("1.2.0")
18+
.version("1.3.0")
2019
.option(
2120
'-f, --file-path <path>',
2221
'Specify the path to the tasks JSON file. Overrides TASK_MANAGER_FILE_PATH env var.'
@@ -486,16 +485,17 @@ program
486485
.option("--model <model>", "LLM model to use", "gpt-4-turbo")
487486
.option("--provider <provider>", "LLM provider to use (openai, google, or deepseek)", "openai")
488487
.option("--attachment <file>", "File to attach as context (can be specified multiple times)", collect, [])
489-
.action(async (options) => {
488+
.action(async (options) => {
490489
try {
491490
console.log(chalk.blue(`Generating project plan from prompt...`));
491+
console.log(options.attachment);
492492

493493
// Pass attachment filenames directly to the server
494494
const response = await taskManager.generateProjectPlan({
495495
prompt: options.prompt,
496496
provider: options.provider,
497497
model: options.model,
498-
attachments: options.attachment // Pass the filenames directly
498+
attachments: options.attachment
499499
});
500500

501501
if ('error' in response) {
@@ -537,17 +537,8 @@ program
537537
console.log(`\n${data.message}`);
538538
}
539539
} catch (err: unknown) {
540-
if (err instanceof Error) {
541-
// Check for API key related errors and format them appropriately
542-
if (err.message.includes('API key') || err.message.includes('authentication') || err.message.includes('unauthorized')) {
543-
console.error(chalk.red(`Error: ${err.message}`));
544-
} else {
545-
console.error(chalk.yellow(`Warning: ${err.message}`));
546-
}
547-
} else {
548540
const normalized = normalizeError(err);
549-
console.error(chalk.red(formatCliError(normalized)));
550-
}
541+
console.error(`Error: ${chalk.red(formatCliError(normalized))}`);
551542
process.exit(1);
552543
}
553544
});

src/client/errors.ts

Lines changed: 23 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,28 @@
11
import { StandardError } from "../types/index.js";
22
/**
3-
* Formats an error message for CLI output
3+
* Formats an error message for CLI output, optionally including relevant details.
44
*/
5-
export function formatCliError(error: StandardError, includeDetails: boolean = false): string {
5+
export function formatCliError(error: StandardError, includeDetails: boolean = true): string {
66
const codePrefix = error.message.includes(`[${error.code}]`) ? '' : `[${error.code}] `;
7-
const message = `${codePrefix}${error.message}`;
8-
return includeDetails && error.details ? `${message}\nDetails: ${JSON.stringify(error.details, null, 2)}` : message;
7+
let message = `${codePrefix}${error.message}`;
8+
9+
if (includeDetails && error.details) {
10+
// Prioritize showing nested originalError message if it exists and is different
11+
const originalErrorMessage = (error.details as any)?.originalError?.message;
12+
if (originalErrorMessage && typeof originalErrorMessage === 'string' && originalErrorMessage !== error.message) {
13+
message += `\n -> Details: ${originalErrorMessage}`;
14+
}
15+
// Add a fallback for simpler string details or stringified objects if needed,
16+
// but avoid dumping large complex objects unless necessary for debugging.
17+
// Example: uncomment if you often have simple string details
18+
// else if (typeof error.details === 'string') {
19+
// message += `\n -> Details: ${error.details}`;
20+
// }
21+
// Example: uncomment ONLY if you need to see the raw JSON details often
22+
// else {
23+
// message += `\nDetails: ${JSON.stringify(error.details, null, 2)}`;
24+
// }
25+
}
26+
27+
return message;
928
}

src/server/TaskManager.ts

Lines changed: 26 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -136,11 +136,16 @@ export class TaskManager {
136136
const attachmentContents: string[] = [];
137137
for (const filename of attachments) {
138138
try {
139+
console.log("We are about to try to read the file.")
139140
const content = await this.fileSystemService.readAttachmentFile(filename);
140141
attachmentContents.push(content);
141142
} catch (error) {
142143
// Propagate file read errors
143-
throw error;
144+
throw createError(
145+
ErrorCode.FileReadError,
146+
`Failed to read attachment file: ${filename}`,
147+
{ originalError: error }
148+
);
144149
}
145150
}
146151

@@ -171,6 +176,7 @@ export class TaskManager {
171176
`Invalid provider: ${provider}`
172177
);
173178
}
179+
console.log("set model and provider")
174180

175181
// Define the schema for the LLM's response using jsonSchema helper
176182
const projectPlanSchema = jsonSchema<ProjectPlanOutput>({
@@ -223,6 +229,7 @@ export class TaskManager {
223229
} catch (err) {
224230
// Handle specific AI SDK errors
225231
if (err instanceof Error) {
232+
// Check for specific error names or messages
226233
if (err.name === 'NoObjectGeneratedError') {
227234
throw createError(
228235
ErrorCode.InvalidResponseFormat,
@@ -244,20 +251,33 @@ export class TaskManager {
244251
{ originalError: err }
245252
);
246253
}
254+
// --- Updated Check for API Key Errors ---
255+
// Check by name (more robust) or message content
256+
if (err.name === 'LoadAPIKeyError' || err.message.includes('API key is missing')) {
257+
throw createError(
258+
ErrorCode.ConfigurationError, // Use the correct code for config issues
259+
"Invalid or missing API key. Please check your environment variables.", // More specific message
260+
{ originalError: err }
261+
);
262+
}
263+
// Existing check for general auth errors (might still be relevant for other cases)
247264
if (err.message.includes('authentication') || err.message.includes('unauthorized')) {
248265
throw createError(
249266
ErrorCode.ConfigurationError,
250-
"Invalid API key or authentication failed. Please check your environment variables.",
267+
"Authentication failed with the LLM provider. Please check your credentials.",
251268
{ originalError: err }
252269
);
253270
}
254271
}
255272

256-
// For unknown errors, preserve the original error but wrap it
273+
// For unknown errors from the LLM/SDK, preserve the original error but wrap it.
274+
// Use a more generic error code here if it's not one of the above.
275+
// Perhaps keep InvalidResponseFormat or create a new one like LLMInteractionError?
276+
// Let's stick with InvalidResponseFormat for now as it often manifests as bad output.
257277
throw createError(
258-
ErrorCode.InvalidResponseFormat,
259-
"Failed to generate project plan",
260-
{ originalError: err }
278+
ErrorCode.InvalidResponseFormat, // Fallback code
279+
"Failed to generate project plan due to an unexpected error.", // Fallback message
280+
{ originalError: err } // Always include original error for debugging
261281
);
262282
}
263283
}

src/utils/errors.ts

Lines changed: 29 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -45,28 +45,48 @@ function getCategoryFromCode(code: ErrorCode): ErrorCategory {
4545
* Converts any error to a StandardError
4646
*/
4747
export function normalizeError(error: unknown): StandardError {
48+
// 1. Check if it already looks like a StandardError (duck typing)
49+
if (
50+
typeof error === 'object' &&
51+
error !== null &&
52+
'status' in error && error.status === 'error' &&
53+
'code' in error && typeof error.code === 'string' &&
54+
'category' in error && typeof error.category === 'string' &&
55+
'message' in error && typeof error.message === 'string' &&
56+
Object.values(ErrorCode).includes(error.code as ErrorCode) // Verify the code is valid
57+
) {
58+
// It already conforms to the StandardError structure, return as is.
59+
// We cast because TypeScript knows it's 'object', but we've verified the shape.
60+
return error as StandardError;
61+
}
62+
63+
// 2. Check if it's an instance of Error
4864
if (error instanceof Error) {
49-
// Try to parse error code from message if it exists
50-
const codeMatch = error.message.match(/\[([A-Z_]+)\]/);
51-
if (codeMatch && Object.values(ErrorCode).includes(codeMatch[1] as ErrorCode)) {
65+
const codeMatch = error.message.match(/\[([A-Z_0-9]+)\]/);
66+
// Ensure codeMatch exists and the captured group is a valid ErrorCode
67+
if (codeMatch && codeMatch[1] && Object.values(ErrorCode).includes(codeMatch[1] as ErrorCode)) {
68+
const extractedCode = codeMatch[1] as ErrorCode;
69+
// Remove the code prefix "[CODE]" from the message - use the full match codeMatch[0] for replacement
70+
const cleanedMessage = error.message.replace(codeMatch[0], '').trim();
5271
return createError(
53-
codeMatch[1] as ErrorCode,
54-
error.message.replace(`[${codeMatch[1]}]`, '').trim(),
55-
{ stack: error.stack }
72+
extractedCode,
73+
cleanedMessage,
74+
{ stack: error.stack } // Keep stack trace if available
5675
);
5776
}
5877

59-
// Default to unknown error
78+
// Fallback for generic Errors without a recognized code in the message
6079
return createError(
61-
ErrorCode.InvalidArgument,
80+
ErrorCode.InvalidArgument, // Use InvalidArgument for generic errors
6281
error.message,
6382
{ stack: error.stack }
6483
);
6584
}
6685

86+
// 3. Handle other types (string, primitive, plain object without structure)
6787
return createError(
6888
ErrorCode.Unknown,
6989
typeof error === 'string' ? error : 'An unknown error occurred',
70-
{ originalError: error }
90+
{ originalError: error } // Include the original unknown error type
7191
);
7292
}

tests/integration/cli.integration.test.ts

Lines changed: 13 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -181,25 +181,29 @@ describe("CLI Integration Tests", () => {
181181

182182
it("should handle missing API key gracefully", async () => {
183183
delete process.env.OPENAI_API_KEY;
184-
184+
185185
const { stderr } = await execAsync(
186186
`TASK_MANAGER_FILE_PATH=${tasksFilePath} tsx ${CLI_PATH} generate-plan --prompt "Create a todo app" --provider openai`
187187
).catch(error => error);
188-
188+
189189
// Verify we get an error with the error code format
190190
expect(stderr).toContain("[ERR_");
191-
// The actual error might not contain "API key" text, so we'll just check for a general error
192-
expect(stderr).toContain("An unknown error occurred");
191+
// The actual error should contain "API key" text
192+
expect(stderr).toContain("API key");
193193
}, 5000);
194194

195195
it("should handle invalid file attachments gracefully", async () => {
196-
const { stderr } = await execAsync(
196+
const { stdout, stderr } = await execAsync(
197197
`TASK_MANAGER_FILE_PATH=${tasksFilePath} tsx ${CLI_PATH} generate-plan --prompt "Create app" --attachment nonexistent.txt`
198-
).catch(error => error);
198+
).catch(error => ({ stdout: error.stdout, stderr: error.stderr }));
199+
200+
// Keep these console logs temporarily if helpful for debugging during development
201+
// console.log("Test stdout:", stdout);
202+
// console.log("Test stderr:", stderr);
199203

200-
// Just verify we get a warning about the attachment
201-
expect(stderr).toContain("Warning:");
202-
expect(stderr).toContain("nonexistent.txt");
204+
// Updated assertion to match the formatCliError output
205+
expect(stderr).toContain("[ERR_4000] Failed to read attachment file: nonexistent.txt");
206+
expect(stderr).toContain("-> Details: Attachment file not found: nonexistent.txt");
203207
}, 5000);
204208
});
205209
});

0 commit comments

Comments
 (0)