Skip to content

Remove fragile error handling from sendChatHistory methods#159

Merged
pontemonti merged 2 commits intousers/johanb/RTP_OpenAIfrom
copilot/sub-pr-157-again
Jan 27, 2026
Merged

Remove fragile error handling from sendChatHistory methods#159
pontemonti merged 2 commits intousers/johanb/RTP_OpenAIfrom
copilot/sub-pr-157-again

Conversation

Copy link
Copy Markdown
Contributor

Copilot AI commented Jan 27, 2026

Addresses feedback on #157 regarding fragile err.message.includes('is required') string check in error handling. This pattern is brittle and unnecessary since sendChatHistory already implements proper error contracts.

Changes

  • Removed try-catch blocks with string-based error detection from sendChatHistoryAsync and sendChatHistoryMessagesAsync
  • Removed unused import of OperationError
  • Simplified control flow to let validation errors propagate naturally while runtime errors return as OperationResult

Before

try {
  return await this.configService.sendChatHistory(
    turnContext,
    chatHistoryMessages,
    effectiveOptions
  );
} catch (err) {
  if (err instanceof Error && err.message.includes('is required')) {
    throw err;
  }
  return OperationResult.failed(new OperationError(err as Error));
}

After

return await this.configService.sendChatHistory(
  turnContext,
  chatHistoryMessages,
  effectiveOptions
);

The underlying sendChatHistory method already throws validation errors and returns OperationResult.failed() for HTTP/runtime failures, making the catch block redundant.


✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.

Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com>
Copilot AI changed the title [WIP] Fix fragile 'is required' check in chat history API Remove fragile error handling from sendChatHistory methods Jan 27, 2026
Copilot AI requested a review from pontemonti January 27, 2026 17:20
@pontemonti pontemonti marked this pull request as ready for review January 27, 2026 17:36
@pontemonti pontemonti requested a review from a team as a code owner January 27, 2026 17:36
@pontemonti pontemonti merged commit dcf9890 into users/johanb/RTP_OpenAI Jan 27, 2026
1 check passed
@pontemonti pontemonti deleted the copilot/sub-pr-157-again branch January 27, 2026 17:36
pontemonti added a commit that referenced this pull request Jan 28, 2026
* Add chat history API for OpenAI

* fix: update uuid and @types/uuid dependencies to use catalog

* fix: correct header formatting in PRD for OpenAI sendChatHistoryAsync API

* refactor: remove unnecessary logging and improve message role validation in McpToolRegistrationService

* refactor: simplify role extraction in McpToolRegistrationService and remove validation

* Add @types/uuid and uuid dependencies

* Remove early return for empty messages in sendChatHistoryMessagesAsync (#158)

* Initial plan

* fix: ensure MCP call is made even when messages array is empty

Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com>

* Remove fragile error handling from sendChatHistory methods (#159)

* Initial plan

* Remove fragile 'is required' check from sendChatHistory methods

Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com>

* Update tests/tooling-extensions-openai/sendChatHistoryAsync.test.ts

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

* Fix error handling in sendChatHistoryAsync and empty array test expectations (#160)

* Initial plan

* Fix test failures: wrap session.getItems() in try-catch and update test expectations for empty arrays

Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com>
Co-authored-by: Johan Broberg <johan@pontemonti.net>

* Add error handling for message conversion in sendChatHistoryMessagesAsync (#163)

* Initial plan

* Add error handling for convertToChatHistoryMessages and test

Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com>

* Address code review feedback: safer error handling and remove extra blank line

Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com>

---------

Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com>
Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com>

---------

Co-authored-by: Johan Broberg <johanb@microsoft.com>
Co-authored-by: Copilot <198982749+Copilot@users.noreply.github.com>
Co-authored-by: pontemonti <7850950+pontemonti@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants