Skip to content

Commit f6a8084

Browse files
cameroncookecameroncooke
committed
Refactor resource handlers to separate logic and improve error handling
- Extracted resource logic into separate functions for better testability in `example.ts`, `devices.ts`, `environment.ts`, `simulators.ts`, `swift-packages.ts`. - Updated handler signatures to comply with MCP SDK requirements, ensuring they only accept necessary parameters. - Enhanced error logging and response formatting in resource logic functions. - Updated tests to directly call the new resource logic functions instead of the handlers. - Added validation for exported fields in resource modules. - Implemented checks for handler signature violations in the testing script. - Adjusted tool handlers to ensure they follow the new signature requirements.
1 parent 0a0335d commit f6a8084

14 files changed

Lines changed: 488 additions & 414 deletions

‎docs/ARCHITECTURE.md‎

Lines changed: 45 additions & 38 deletions
Original file line numberDiff line numberDiff line change
@@ -201,49 +201,56 @@ Resources can reuse existing tool logic for consistency:
201201

202202
```typescript
203203
// src/mcp/resources/some_resource.ts
204+
import { log, getDefaultCommandExecutor, CommandExecutor } from '../../utils/index.js';
205+
import { getSomeResourceLogic } from '../tools/some-workflow/get_some_resource.js';
206+
207+
// Testable resource logic separated from MCP handler
208+
export async function someResourceResourceLogic(
209+
executor: CommandExecutor = getDefaultCommandExecutor(),
210+
): Promise<{ contents: Array<{ text: string }> }> {
211+
try {
212+
log('info', 'Processing some resource request');
213+
214+
const result = await getSomeResourceLogic({}, executor);
215+
216+
if (result.isError) {
217+
const errorText = result.content[0]?.text;
218+
throw new Error(
219+
typeof errorText === 'string' ? errorText : 'Failed to retrieve some resource data',
220+
);
221+
}
222+
223+
return {
224+
contents: [
225+
{
226+
text:
227+
typeof result.content[0]?.text === 'string'
228+
? result.content[0].text
229+
: 'No data for that resource is available',
230+
},
231+
],
232+
};
233+
} catch (error) {
234+
const errorMessage = error instanceof Error ? error.message : String(error);
235+
log('error', `Error in some_resource resource handler: ${errorMessage}`);
236+
237+
return {
238+
contents: [
239+
{
240+
text: `Error retrieving resource data: ${errorMessage}`,
241+
},
242+
],
243+
};
244+
}
245+
}
246+
204247
export default {
205248
uri: 'xcodebuildmcp://some_resource',
206249
name: 'some_resource',
207250
description: 'Returns some resource information',
208251
mimeType: 'text/plain',
209-
async handler(
210-
uri: URL,
211-
executor: CommandExecutor = getDefaultCommandExecutor(),
212-
): Promise<{ contents: Array<{ text: string }> }> {
213-
try {
214-
log('info', 'Processing simulators resource request');
215-
216-
const result = await getSomeResource({}, executor);
217-
218-
if (result.isError) {
219-
const errorText = result.content[0]?.text;
220-
throw new Error(
221-
typeof errorText === 'string' ? errorText : 'Failed to retrieve some resource data',
222-
);
223-
}
224-
225-
return {
226-
contents: [
227-
{
228-
text:
229-
typeof result.content[0]?.text === 'string'
230-
? result.content[0].text
231-
: 'No data for that resource is available',
232-
},
233-
],
234-
};
235-
} catch (error) {
236-
const errorMessage = error instanceof Error ? error.message : String(error);
237-
log('error', `Error in some_resource resource handler: ${errorMessage}`);
238-
239-
return {
240-
contents: [
241-
{
242-
text: `Error retrieving resource data: ${errorMessage}`,
243-
},
244-
],
245-
};
246-
}
252+
async handler(_uri: URL): Promise<{ contents: Array<{ text: string }> }> {
253+
return someResourceResourceLogic();
247254
},
248255
};
249256
```

‎docs/PLUGIN_DEVELOPMENT.md‎

Lines changed: 83 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -304,19 +304,47 @@ Resources are located in `src/resources/` and follow this pattern:
304304

305305
```typescript
306306
// src/resources/example.ts
307+
import { log, getDefaultCommandExecutor, CommandExecutor } from '../../utils/index.js';
308+
309+
// Testable resource logic separated from MCP handler
310+
export async function exampleResourceLogic(
311+
executor: CommandExecutor = getDefaultCommandExecutor(),
312+
): Promise<{ contents: Array<{ text: string }> }> {
313+
try {
314+
log('info', 'Processing example resource request');
315+
316+
// Use the executor to get data
317+
const result = await executor(['some', 'command'], 'Example Resource Operation');
318+
319+
if (!result.success) {
320+
throw new Error(result.error || 'Failed to get resource data');
321+
}
322+
323+
return {
324+
contents: [{ text: result.output || 'resource data' }]
325+
};
326+
} catch (error) {
327+
const errorMessage = error instanceof Error ? error.message : String(error);
328+
log('error', `Error in example resource handler: ${errorMessage}`);
329+
330+
return {
331+
contents: [
332+
{
333+
text: `Error retrieving resource data: ${errorMessage}`,
334+
},
335+
],
336+
};
337+
}
338+
}
339+
307340
export default {
308341
uri: 'xcodebuildmcp://example',
309-
name: 'example'
342+
name: 'example',
310343
description: 'Description of the resource data',
311344
mimeType: 'text/plain',
312-
async handler(
313-
executor: CommandExecutor = getDefaultCommandExecutor()
314-
): Promise<{ contents: Array<{ text: string }> }> {
315-
// Resource implementation
316-
return {
317-
contents: [{ text: 'resource data' }]
318-
};
319-
}
345+
async handler(_uri: URL): Promise<{ contents: Array<{ text: string }> }> {
346+
return exampleResourceLogic();
347+
},
320348
};
321349
```
322350

@@ -325,17 +353,16 @@ export default {
325353
**Reuse Existing Logic**: Resources that mirror tools should reuse existing tool logic for consistency:
326354

327355
```typescript
328-
// src/mcp/resources/simulators.ts (simplified exmaple)
329-
import { list_simsLogic } from '../plugins/simulator-shared/list_sims.js';
356+
// src/mcp/resources/simulators.ts (simplified example)
357+
import { list_simsLogic } from '../tools/simulator-shared/list_sims.js';
330358

331359
export default {
332360
uri: 'xcodebuildmcp://simulators',
333361
name: 'simulators'
334362
description: 'Available iOS simulators with UUIDs and states',
335363
mimeType: 'text/plain',
336-
async handler(
337-
executor: CommandExecutor = getDefaultCommandExecutor()
338-
): Promise<{ contents: Array<{ text: string }> }> {
364+
async handler(uri: URL): Promise<{ contents: Array<{ text: string }> }> {
365+
const executor = getDefaultCommandExecutor();
339366
const result = await list_simsLogic({}, executor);
340367
return {
341368
contents: [{ text: result.content[0].text }]
@@ -352,19 +379,52 @@ Create tests in `src/mcp/resources/__tests__/`:
352379

353380
```typescript
354381
// src/mcp/resources/__tests__/example.test.ts
355-
import exampleResource from '../example.js';
382+
import exampleResource, { exampleResourceLogic } from '../example.js';
356383
import { createMockExecutor } from '../../utils/test-common.js';
357384

358385
describe('example resource', () => {
359-
it('should return resource data', async () => {
360-
const mockExecutor = createMockExecutor({
361-
success: true,
362-
output: 'test data'
386+
describe('Export Field Validation', () => {
387+
it('should export correct uri', () => {
388+
expect(exampleResource.uri).toBe('xcodebuildmcp://example');
389+
});
390+
391+
it('should export correct description', () => {
392+
expect(exampleResource.description).toBe('Description of the resource data');
393+
});
394+
395+
it('should export correct mimeType', () => {
396+
expect(exampleResource.mimeType).toBe('text/plain');
397+
});
398+
399+
it('should export handler function', () => {
400+
expect(typeof exampleResource.handler).toBe('function');
401+
});
402+
});
403+
404+
describe('Resource Logic Functionality', () => {
405+
it('should return resource data successfully', async () => {
406+
const mockExecutor = createMockExecutor({
407+
success: true,
408+
output: 'test data'
409+
});
410+
411+
// Test the logic function directly, not the handler
412+
const result = await exampleResourceLogic(mockExecutor);
413+
414+
expect(result.contents).toHaveLength(1);
415+
expect(result.contents[0].text).toContain('expected data');
416+
});
417+
418+
it('should handle command execution errors', async () => {
419+
const mockExecutor = createMockExecutor({
420+
success: false,
421+
error: 'Command failed'
422+
});
423+
424+
const result = await exampleResourceLogic(mockExecutor);
425+
426+
expect(result.contents[0].text).toContain('Error retrieving');
363427
});
364-
365-
const result = await exampleResource.handler(mockExecutor);
366-
367-
expect(result.contents[0].text).toContain('expected data');
368428
});
369429
});
370430
```

‎docs/TESTING.md‎

Lines changed: 15 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -94,18 +94,22 @@ Test → Plugin Handler → utilities → [DEPENDENCY INJECTION] createMockExecu
9494
All plugin handlers must support dependency injection:
9595

9696
```typescript
97+
export function tool_nameLogic(
98+
args: Record<string, unknown>,
99+
commandExecutor: CommandExecutor,
100+
fileSystemExecutor?: FileSystemExecutor
101+
): Promise<ToolResponse> {
102+
// Use injected executors
103+
const result = await executeCommand(['xcrun', 'simctl', 'list'], commandExecutor);
104+
return createTextResponse(result.output);
105+
}
106+
97107
export default {
98108
name: 'tool_name',
99109
description: 'Tool description',
100110
schema: { /* zod schema */ },
101-
async handler(
102-
args: Record<string, unknown>,
103-
commandExecutor: CommandExecutor = getDefaultCommandExecutor(),
104-
fileSystemExecutor: FileSystemExecutor = getDefaultFileSystemExecutor()
105-
): Promise<ToolResponse> {
106-
// Use injected executors
107-
const result = await executeCommand(['xcrun', 'simctl', 'list'], commandExecutor);
108-
return createTextResponse(result.output);
111+
async handler(args: Record<string, unknown>): Promise<ToolResponse> {
112+
return tool_nameLogic(args, getDefaultCommandExecutor(), getDefaultFileSystemExecutor());
109113
},
110114
};
111115
```
@@ -128,7 +132,7 @@ it('should handle successful command execution', async () => {
128132
output: 'BUILD SUCCEEDED'
129133
});
130134

131-
const result = await tool.handler(
135+
const result = await tool_nameLogic(
132136
{ projectPath: '/test.xcodeproj', scheme: 'MyApp' },
133137
mockExecutor
134138
);
@@ -509,12 +513,8 @@ const result = await tool.handler(params, mockCmd, mockFS);
509513
**Fix**: Update handler signature:
510514

511515
```typescript
512-
async handler(
513-
args: Record<string, unknown>,
514-
commandExecutor: CommandExecutor = getDefaultCommandExecutor(),
515-
fileSystemExecutor: FileSystemExecutor = getDefaultFileSystemExecutor()
516-
): Promise<ToolResponse> {
517-
// Use injected executors
516+
async handler(args: Record<string, unknown>): Promise<ToolResponse> {
517+
return tool_nameLogic(args, getDefaultCommandExecutor(), getDefaultFileSystemExecutor());
518518
}
519519
```
520520

0 commit comments

Comments
 (0)