From d674c2546dfb9345cb9817eea57c4d79cea5bc3b Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 27 Jan 2026 19:48:14 +0000 Subject: [PATCH 1/3] Initial plan From 58cdf417ee3ba0c1171fa99d3dd29415b126095a Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 27 Jan 2026 19:53:38 +0000 Subject: [PATCH 2/3] Add error handling for convertToChatHistoryMessages and test Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com> --- .../src/McpToolRegistrationService.ts | 11 +++++++++-- .../sendChatHistoryMessagesAsync.test.ts | 15 +++++++++++++++ 2 files changed, 24 insertions(+), 2 deletions(-) diff --git a/packages/agents-a365-tooling-extensions-openai/src/McpToolRegistrationService.ts b/packages/agents-a365-tooling-extensions-openai/src/McpToolRegistrationService.ts index e6edb0e4..40624fd4 100644 --- a/packages/agents-a365-tooling-extensions-openai/src/McpToolRegistrationService.ts +++ b/packages/agents-a365-tooling-extensions-openai/src/McpToolRegistrationService.ts @@ -174,8 +174,15 @@ export class McpToolRegistrationService { orchestratorName: toolOptions?.orchestratorName ?? this.orchestratorName }; - // Convert OpenAI messages to ChatHistoryMessage format - const chatHistoryMessages = this.convertToChatHistoryMessages(messages); + let chatHistoryMessages: ChatHistoryMessage[]; + try { + // Convert OpenAI messages to ChatHistoryMessage format + chatHistoryMessages = this.convertToChatHistoryMessages(messages); + } catch (err: unknown) { + // Convert errors from message conversion into a failed OperationResult + const error = err as Error; + return OperationResult.failed(new OperationError(error)); + } // Delegate to core service return await this.configService.sendChatHistory( diff --git a/tests/tooling-extensions-openai/sendChatHistoryMessagesAsync.test.ts b/tests/tooling-extensions-openai/sendChatHistoryMessagesAsync.test.ts index 16acf680..493f3685 100644 --- a/tests/tooling-extensions-openai/sendChatHistoryMessagesAsync.test.ts +++ b/tests/tooling-extensions-openai/sendChatHistoryMessagesAsync.test.ts @@ -242,6 +242,21 @@ describe('McpToolRegistrationService - sendChatHistoryMessagesAsync', () => { expect(result.errors).toHaveLength(1); }); + it('EH-05: should return failed when message conversion throws unexpected error', async () => { + const messages = createMixedMessages(); + + // Mock convertToChatHistoryMessages to throw an error + jest.spyOn(service as any, 'convertToChatHistoryMessages').mockImplementation(() => { + throw new Error('Conversion error'); + }); + + const result = await service.sendChatHistoryMessagesAsync(mockTurnContext, messages); + + expect(result.succeeded).toBe(false); + expect(result.errors).toHaveLength(1); + expect(result.errors[0].message).toBe('Conversion error'); + }); + it('should re-throw validation errors from core service', async () => { const messages = createMixedMessages(); // Remove conversation ID to trigger validation error From f76c9c9b098f7e4a94152606e0aa15a89125b72a Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 27 Jan 2026 19:58:53 +0000 Subject: [PATCH 3/3] Address code review feedback: safer error handling and remove extra blank line Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com> --- .../src/McpToolRegistrationService.ts | 2 +- .../sendChatHistoryMessagesAsync.test.ts | 17 ++++++++++++++++- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/packages/agents-a365-tooling-extensions-openai/src/McpToolRegistrationService.ts b/packages/agents-a365-tooling-extensions-openai/src/McpToolRegistrationService.ts index 40624fd4..a35562c4 100644 --- a/packages/agents-a365-tooling-extensions-openai/src/McpToolRegistrationService.ts +++ b/packages/agents-a365-tooling-extensions-openai/src/McpToolRegistrationService.ts @@ -180,7 +180,7 @@ export class McpToolRegistrationService { chatHistoryMessages = this.convertToChatHistoryMessages(messages); } catch (err: unknown) { // Convert errors from message conversion into a failed OperationResult - const error = err as Error; + const error = err instanceof Error ? err : new Error(String(err)); return OperationResult.failed(new OperationError(error)); } diff --git a/tests/tooling-extensions-openai/sendChatHistoryMessagesAsync.test.ts b/tests/tooling-extensions-openai/sendChatHistoryMessagesAsync.test.ts index 493f3685..633ad405 100644 --- a/tests/tooling-extensions-openai/sendChatHistoryMessagesAsync.test.ts +++ b/tests/tooling-extensions-openai/sendChatHistoryMessagesAsync.test.ts @@ -244,7 +244,7 @@ describe('McpToolRegistrationService - sendChatHistoryMessagesAsync', () => { it('EH-05: should return failed when message conversion throws unexpected error', async () => { const messages = createMixedMessages(); - + // Mock convertToChatHistoryMessages to throw an error jest.spyOn(service as any, 'convertToChatHistoryMessages').mockImplementation(() => { throw new Error('Conversion error'); @@ -257,6 +257,21 @@ describe('McpToolRegistrationService - sendChatHistoryMessagesAsync', () => { expect(result.errors[0].message).toBe('Conversion error'); }); + it('EH-06: should handle non-Error thrown values gracefully', async () => { + const messages = createMixedMessages(); + + // Mock convertToChatHistoryMessages to throw a non-Error object + jest.spyOn(service as any, 'convertToChatHistoryMessages').mockImplementation(() => { + throw 'String error'; // eslint-disable-line no-throw-literal + }); + + const result = await service.sendChatHistoryMessagesAsync(mockTurnContext, messages); + + expect(result.succeeded).toBe(false); + expect(result.errors).toHaveLength(1); + expect(result.errors[0].message).toBe('String error'); + }); + it('should re-throw validation errors from core service', async () => { const messages = createMixedMessages(); // Remove conversation ID to trigger validation error