fix: error and input handling (#130)

* fix error and input handling

* fix integration tests

* handle empty input and no resource in get actor details

* handle errors in runs

* lint

* Update src/tools/helpers.ts

Co-authored-by: Copilot <[email protected]>

* Improve error handling in getActor tool by catching specific ApifyApiError

* refactor ai slop

* restore comment

* Update src/mcp/server.ts

Co-authored-by: Copilot <[email protected]>

* use zod validation instead

* fix type import

---------

Co-authored-by: Copilot <[email protected]>

Author: MQ37

src/mcp/server.ts

@@ -12,7 +12,7 @@ import {
     ListToolsRequestSchema,
     McpError,
 } from '@modelcontextprotocol/sdk/types.js';
-import type { ActorCallOptions } from 'apify-client';
+import { type ActorCallOptions, ApifyApiError } from 'apify-client';
 
 import log from '@apify/log';
 
@@ -453,6 +453,14 @@ export class ActorsMcpServer {
                     return { content };
                 }
             } catch (error) {
+                if (error instanceof ApifyApiError) {
+                    log.error(`Apify API error calling tool ${name}: ${error.message}`);
+                    return {
+                        content: [
+                            { type: 'text', text: `Apify API error calling tool ${name}: ${error.message}` },
+                        ],
+                    };
+                }
                 log.error(`Error calling tool ${name}: ${error}`);
                 throw new McpError(
                     ErrorCode.InternalError,

src/tools/actor.ts

@@ -225,7 +225,9 @@ export async function getActorsAsTools(
 }
 
 const getActorArgs = z.object({
-    actorId: z.string().describe('Actor ID or a tilde-separated owner\'s username and Actor name.'),
+    actorId: z.string()
+        .min(1)
+        .describe('Actor ID or a tilde-separated owner\'s username and Actor name.'),
 });
 
 /**
@@ -245,10 +247,13 @@ export const getActor: ToolEntry = {
         ajvValidate: ajv.compile(zodToJsonSchema(getActorArgs)),
         call: async (toolArgs) => {
             const { args, apifyToken } = toolArgs;
-            const parsed = getActorArgs.parse(args);
+            const { actorId } = getActorArgs.parse(args);
             const client = new ApifyClient({ token: apifyToken });
             // Get Actor - contains a lot of irrelevant information
-            const actor = await client.actor(parsed.actorId).get();
+            const actor = await client.actor(actorId).get();
+            if (!actor) {
+                return { content: [{ type: 'text', text: `Actor '${actorId}' not found.` }] };
+            }
             return { content: [{ type: 'text', text: JSON.stringify(actor) }] };
         },
     } as InternalTool,

src/tools/build.ts

@@ -96,6 +96,7 @@ function truncateActorReadme(readme: string, limit = ACTOR_README_MAX_LENGTH): s
 
 const getActorDefinitionArgsSchema = z.object({
     actorName: z.string()
+        .min(1)
         .describe('Retrieve input, readme, and other details for Actor ID or Actor full name. '
             + 'Actor name is always composed from `username/name`'),
     limit: z.number()
@@ -124,6 +125,9 @@ export const actorDefinitionTool: ToolEntry = {
 
             const parsed = getActorDefinitionArgsSchema.parse(args);
             const v = await getActorDefinition(parsed.actorName, apifyToken, parsed.limit);
+            if (!v) {
+                return { content: [{ type: 'text', text: `Actor '${parsed.actorName}' not found.` }] };
+            }
             if (v && v.input && 'properties' in v.input && v.input) {
                 const properties = filterSchemaProperties(v.input.properties as { [key: string]: ISchemaProperties });
                 v.input.properties = shortenProperties(properties);

src/tools/dataset.ts

@@ -9,11 +9,15 @@ import type { InternalTool, ToolEntry } from '../types.js';
 const ajv = new Ajv({ coerceTypes: 'array', strict: false });
 
 const getDatasetArgs = z.object({
-    datasetId: z.string().describe('Dataset ID or username~dataset-name.'),
+    datasetId: z.string()
+        .min(1)
+        .describe('Dataset ID or username~dataset-name.'),
 });
 
 const getDatasetItemsArgs = z.object({
-    datasetId: z.string().describe('Dataset ID or username~dataset-name.'),
+    datasetId: z.string()
+        .min(1)
+        .describe('Dataset ID or username~dataset-name.'),
     clean: z.boolean().optional()
         .describe('If true, returns only non-empty items and skips hidden fields (starting with #). Shortcut for skipHidden=true and skipEmpty=true.'),
     offset: z.number().optional()
@@ -54,6 +58,9 @@ export const getDataset: ToolEntry = {
             const parsed = getDatasetArgs.parse(args);
             const client = new ApifyClient({ token: apifyToken });
             const v = await client.dataset(parsed.datasetId).get();
+            if (!v) {
+                return { content: [{ type: 'text', text: `Dataset '${parsed.datasetId}' not found.` }] };
+            }
             return { content: [{ type: 'text', text: JSON.stringify(v) }] };
         },
     } as InternalTool,
@@ -98,6 +105,9 @@ export const getDatasetItems: ToolEntry = {
                 desc: parsed.desc,
                 flatten,
             });
+            if (!v) {
+                return { content: [{ type: 'text', text: `Dataset '${parsed.datasetId}' not found.` }] };
+            }
             return { content: [{ type: 'text', text: JSON.stringify(v) }] };
         },
     } as InternalTool,

src/tools/helpers.ts

@@ -62,6 +62,7 @@ In that case, the user should check the MCP client documentation to see if the c
 
 export const addToolArgsSchema = z.object({
     actorName: z.string()
+        .min(1)
         .describe('Add a tool, Actor or MCP-Server to available tools by Actor ID or tool full name.'
             + 'Tool name is always composed from `username/name`'),
 });
@@ -79,6 +80,14 @@ export const addTool: ToolEntry = {
         call: async (toolArgs) => {
             const { apifyMcpServer, mcpServer, apifyToken, args } = toolArgs;
             const parsed = addToolArgsSchema.parse(args);
+            if (apifyMcpServer.listAllToolNames().includes(parsed.actorName)) {
+                return {
+                    content: [{
+                        type: 'text',
+                        text: `Actor ${parsed.actorName} is already available. No new tools were added.`,
+                    }],
+                };
+            }
             const tools = await getActorsAsTools([parsed.actorName], apifyToken);
             const toolsAdded = apifyMcpServer.upsertTools(tools, true);
             await mcpServer.notification({ method: 'notifications/tools/list_changed' });
@@ -98,6 +107,7 @@ export const addTool: ToolEntry = {
 };
 export const removeToolArgsSchema = z.object({
     toolName: z.string()
+        .min(1)
         .describe('Tool name to remove from available tools.')
         .transform((val) => actorNameToToolName(val)),
 });
@@ -112,8 +122,19 @@ export const removeTool: ToolEntry = {
         // TODO: I don't like that we are passing apifyMcpServer and mcpServer to the tool
         call: async (toolArgs) => {
             const { apifyMcpServer, mcpServer, args } = toolArgs;
-
             const parsed = removeToolArgsSchema.parse(args);
+            // Check if tool exists before attempting removal
+            if (!apifyMcpServer.tools.has(parsed.toolName)) {
+                // Send notification so client can update its tool list
+                // just in case the client tool list is out of sync
+                await mcpServer.notification({ method: 'notifications/tools/list_changed' });
+                return {
+                    content: [{
+                        type: 'text',
+                        text: `Tool '${parsed.toolName}' not found. No tools were removed.`,
+                    }],
+                };
+            }
             const removedTools = apifyMcpServer.removeToolsByName([parsed.toolName], true);
             await mcpServer.notification({ method: 'notifications/tools/list_changed' });
             return { content: [{ type: 'text', text: `Tools removed: ${removedTools.join(', ')}` }] };

src/tools/key_value_store.ts

@@ -10,6 +10,7 @@ const ajv = new Ajv({ coerceTypes: 'array', strict: false });
 
 const getKeyValueStoreArgs = z.object({
     storeId: z.string()
+        .min(1)
         .describe('Key-value store ID or username~store-name'),
 });
 
@@ -38,6 +39,7 @@ export const getKeyValueStore: ToolEntry = {
 
 const getKeyValueStoreKeysArgs = z.object({
     storeId: z.string()
+        .min(1)
         .describe('Key-value store ID or username~store-name'),
     exclusiveStartKey: z.string()
         .optional()
@@ -77,8 +79,10 @@ export const getKeyValueStoreKeys: ToolEntry = {
 
 const getKeyValueStoreRecordArgs = z.object({
     storeId: z.string()
+        .min(1)
         .describe('Key-value store ID or username~store-name'),
     recordKey: z.string()
+        .min(1)
         .describe('Key of the record to retrieve.'),
 });
 

src/tools/run.ts

@@ -9,11 +9,15 @@ import type { InternalTool, ToolEntry } from '../types.js';
 const ajv = new Ajv({ coerceTypes: 'array', strict: false });
 
 const getActorRunArgs = z.object({
-    runId: z.string().describe('The ID of the Actor run.'),
+    runId: z.string()
+        .min(1)
+        .describe('The ID of the Actor run.'),
 });
 
 const abortRunArgs = z.object({
-    runId: z.string().describe('The ID of the Actor run to abort.'),
+    runId: z.string()
+        .min(1)
+        .describe('The ID of the Actor run to abort.'),
     gracefully: z.boolean().optional().describe('If true, the Actor run will abort gracefully with a 30-second timeout.'),
 });
 
@@ -35,6 +39,9 @@ export const getActorRun: ToolEntry = {
             const parsed = getActorRunArgs.parse(args);
             const client = new ApifyClient({ token: apifyToken });
             const v = await client.run(parsed.runId).get();
+            if (!v) {
+                return { content: [{ type: 'text', text: `Run with ID '${parsed.runId}' not found.` }] };
+            }
             return { content: [{ type: 'text', text: JSON.stringify(v) }] };
         },
     } as InternalTool,

tests/integration/actor.server-sse.test.ts

@@ -21,7 +21,7 @@ createIntegrationTestsSuite({
     getActorsMcpServer: () => mcpServer,
     createClientFn: async (options) => await createMcpSseClient(mcpUrl, options),
     beforeAllFn: async () => {
-        mcpServer = new ActorsMcpServer({ enableAddingActors: false });
+        mcpServer = new ActorsMcpServer({ enableAddingActors: false, enableDefaultActors: false });
         log.setLevel(log.LEVELS.OFF);
 
         // Create an express app using the proper server setup

tests/integration/actor.server-streamable.test.ts

@@ -23,7 +23,7 @@ createIntegrationTestsSuite({
     beforeAllFn: async () => {
         log.setLevel(log.LEVELS.OFF);
         // Create an express app using the proper server setup
-        mcpServer = new ActorsMcpServer({ enableAddingActors: false });
+        mcpServer = new ActorsMcpServer({ enableAddingActors: false, enableDefaultActors: false });
         app = createExpressApp(httpServerHost, mcpServer);
 
         // Start a test server

tests/integration/suite.ts

@@ -103,8 +103,7 @@ export function createIntegrationTestsSuite(
             await client.close();
         });
 
-        // TODO: This test is not working as there is a problem with server reset, which loads default Actors
-        it.runIf(false)('should list all default tools and two loaded Actors', async () => {
+        it('should list all default tools and two loaded Actors', async () => {
             const actors = ['apify/website-content-crawler', 'apify/instagram-scraper'];
             const client = await createClientFn({ actors, enableAddingActors: false });
             const names = getToolNames(await client.listTools());
@@ -210,25 +209,27 @@ export function createIntegrationTestsSuite(
         });
 
         it.runIf(getActorsMcpServer)('should reset and restore tool state with default tools', async () => {
-            const client = await createClientFn({ enableAddingActors: true });
+            const firstClient = await createClientFn({ enableAddingActors: true });
             const actorsMCPServer = getActorsMcpServer!();
             const numberOfTools = defaultTools.length + addRemoveTools.length + defaults.actors.length;
             const toolList = actorsMCPServer.listAllToolNames();
             expect(toolList.length).toEqual(numberOfTools);
             // Add a new Actor
-            await addActor(client, ACTOR_PYTHON_EXAMPLE);
+            await addActor(firstClient, ACTOR_PYTHON_EXAMPLE);
 
             // Store the tool name list
             const toolListWithActor = actorsMCPServer.listAllToolNames();
             expect(toolListWithActor.length).toEqual(numberOfTools + 1); // + 1 for the added Actor
+            await firstClient.close();
 
             // Remove all tools
-            // TODO: The reset functions sets the enableAddingActors to false, which is not expected
-            // await actorsMCPServer.reset();
-            // const toolListAfterReset = actorsMCPServer.listAllToolNames();
-            // expect(toolListAfterReset.length).toEqual(numberOfTools);
-
-            await client.close();
+            await actorsMCPServer.reset();
+            // We connect second client so that the default tools are loaded
+            // if no specific list of Actors is provided
+            const secondClient = await createClientFn({ enableAddingActors: true });
+            const toolListAfterReset = actorsMCPServer.listAllToolNames();
+            expect(toolListAfterReset.length).toEqual(numberOfTools);
+            await secondClient.close();
         });
 
         it.runIf(getActorsMcpServer)('should notify tools changed handler on tool modifications', async () => {