Skip to content

fix(mcp): Adding error handling for generate keys panel#8795

Merged
preetriti1 merged 3 commits intomainfrom
priti/mcpfixeserror
Feb 6, 2026
Merged

fix(mcp): Adding error handling for generate keys panel#8795
preetriti1 merged 3 commits intomainfrom
priti/mcpfixeserror

Conversation

@preetriti1
Copy link
Contributor

@preetriti1 preetriti1 commented Feb 5, 2026

Commit Type

  • feature - New functionality
  • fix - Bug fix
  • refactor - Code restructuring without behavior change
  • perf - Performance improvement
  • docs - Documentation update
  • test - Test-related changes
  • chore - Maintenance/tooling

Risk Level

  • Low - Minor changes, limited scope
  • Medium - Moderate changes, some user impact
  • High - Major changes, significant user/system impact

What & Why

Adding error handling in generate keys panel to show errors when Generate keys fail
Minor fix on forward links on create server panel

Impact of Change

  • Users: Users will see error messages when Generate keys fails
  • Developers: Note the change to error handling pattern (using getObjectPropertyValue and throwing formatted Error with {errorMessage}) which may affect callers that expected different error shapes — confirm backwards-compatibility or list required consumer changes.
  • System: Others are just test page changes, do not impact production in any way

Test Plan

  • Unit tests added/updated
  • E2E tests added/updated
  • Manual testing completed
  • Tested in:

Contributors

@kewear

Screenshots/Videos

image

Copilot AI review requested due to automatic review settings February 5, 2026 22:36
@github-actions
Copy link

github-actions bot commented Feb 5, 2026

🤖 AI PR Validation Report

PR Review Results

Thank you for your submission! Here's detailed feedback on your PR title and body compliance:

PR Title

  • Current: fix(mcp): Adding error handling for generate keys panel
  • Issue: Minor style suggestion — PR titles are typically written in imperative form (e.g., "Add" instead of "Adding") and can be slightly more specific about the scope of changes (e.g., mentioning tests/localization if you want).
  • Recommendation: Use imperative voice and be slightly more descriptive. Example: fix(mcp): Add error handling for Generate Keys panel and update tests.

Commit Type

  • Properly selected (fix).
  • Note: Only one option is selected, which is correct.

Risk Level

  • Risk level selected in PR body: Low.
  • Label present on PR: risk:low — these match.

⚠️ What & Why

  • Current: "Adding error handling in generate keys panel to show errors when Generate keys fail\nMinor fix on forward links on create server panel"
  • Issue: The section is brief and does not call out all notable changes in this PR (e.g., localization token name changes, tests added, a hardcoded appId and location change in apps/Standalone/McpServer.tsx, and link updates in create.tsx). The PR body should clearly list key files/areas changed so reviewers can quickly understand scope from the description.
  • Recommendation: Expand this section to list the primary changes (examples shown below). Example text:
    • What: Add more robust error extraction + message formatting for generateKeys and updateAuthSettings; update localization placeholders to use {errorMessage}; add unit tests for error and success paths in GenerateKeys UI; update docs links in CreateServer; small test/demo appId/location changes in McpServer for the standalone/test harness.
    • Why: To surface clearer error messages to users and ensure UI shows proper success/error message bars; to make tests robust to nested error shapes.

⚠️ Impact of Change

  • Issue: Current impact claims "Others are just test page changes, do not impact production in any way." There are actual code updates in libs and apps (error handling shape, localization token replacements, and apps/Standalone changes) — please clarify whether the apps/Standalone changes are purely for a test/demo harness and confirm there are no production-facing behavioral or contract changes.
  • Recommendation:
    • Users: Explain that users will now see clearer error messages when key generation or auth updates fail (include example message formatting). If the UI message text changed slightly (placeholder name change), confirm that translations/localization teams are aware.
    • Developers: State clearly that error handling now extracts nested error.message via getObjectPropertyValue and that thrown errors are formatted with the {errorMessage} placeholder. Call out that callers/consumers that parse thrown Error.message should be OK but to confirm there is no expectation of previous exact text. If any consumer expects the old exact message, list required consumer changes.
    • System: If the apps/Standalone appId/location changes are test/demo-only, explicitly say so and confirm that no production resource IDs or secrets were changed.

Test Plan

  • Assessment: You marked unit tests added/updated and manual testing completed. The diff includes updates and new tests in the unit test files for GenerateKeys and server utils — this confirms unit tests are present.
  • Recommendation: Add a short sentence in Test Plan listing the updated/new test files (e.g., libs/designer/.../generatekeys.spec.tsx and libs/designer/.../server.spec.ts) and what scenarios they cover (success path, failure path, error extraction). If there is no E2E test, mention why E2E is not required for this change (e.g., limited to UI message handling and library utilities).

Contributors

  • Assessment: Contributors section lists @kewear. Good. No mandatory action. If others contributed, add them here.

Screenshots/Videos

  • Assessment: Screenshot included. Good.

Summary Table

Section Status Recommendation
Title Use imperative tense: fix(mcp): Add error handling for Generate Keys panel and update tests
Commit Type No change needed
Risk Level Label matches body; keep risk:low
What & Why ⚠️ Expand to list key files and clarify purpose
Impact of Change ⚠️ Clarify standalone appId/location changes and error-shape impact on consumers
Test Plan Add filenames and brief coverage summary
Contributors OK (add more if relevant)
Screenshots/Videos OK

Final message:
Please update the PR body with the clarifications above (especially What & Why and Impact of Change). Specific suggestions to include in the PR body:

  • A short list of changed areas/files (e.g., libs/designer/src/lib/core/mcp/utils/server.ts, generatekeys UI and tests, localization strings.json, apps/Standalone/src/mcp/app/McpServer.tsx, create.tsx link changes).
  • Explicitly state whether the McpServer appId/location change is test/demo-only and confirm it won't affect production resources or include secrets.
  • Note the new error shaping behavior: we now extract nested error messages via getObjectPropertyValue and format them with {errorMessage} — call out any possible consumer impact.
  • In Test Plan, mention the test files added/updated and what they assert, and why E2E tests were not added (if applicable).

Once you add those clarifications, this PR body will be clear and well-aligned with the code changes. Thank you for the thorough unit tests and the localization/template consistency updates — these make the change robust and easier to review.


Last updated: Thu, 05 Feb 2026 23:54:11 GMT

@preetriti1 preetriti1 added the risk:low Low risk change with minimal impact label Feb 5, 2026
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR adds error handling to the MCP generate keys panel to display error messages when key generation fails, along with minor fixes to documentation links in the create server panel.

Changes:

  • Added error state management and display in the generate keys panel
  • Improved error message extraction from API responses using getObjectPropertyValue
  • Updated unit tests to cover success and error scenarios
  • Updated documentation URLs to use forwarding links

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
libs/designer/src/lib/ui/mcp/panel/server/generatekeys.tsx Added error state, try-catch block, and error message bar rendering
libs/designer/src/lib/core/mcp/utils/server.ts Enhanced error extraction logic using getObjectPropertyValue for both generateKeys and updateAuthSettings functions
libs/designer/src/lib/ui/mcp/panel/server/test/generatekeys.spec.tsx Added tests for success and error scenarios, introduced setupService helper for mocking service behavior
libs/designer/src/lib/ui/mcp/panel/server/create.tsx Updated documentation URLs to use go.microsoft.com forwarding links
Localize/lang/strings.json Updated localization string placeholders from {error} to {errorMessage}
apps/Standalone/src/mcp/app/McpServer.tsx Changed test configuration (appId and location)
apps/Standalone/src/designer/app/AzureLogicAppsDesigner/laDesigner.tsx Removed invokeWorkflow operation from client-supported operations list

@github-actions
Copy link

github-actions bot commented Feb 5, 2026

📊 Coverage check completed. See workflow run for details.

1 similar comment
@github-actions
Copy link

github-actions bot commented Feb 5, 2026

📊 Coverage check completed. See workflow run for details.

@preetriti1 preetriti1 enabled auto-merge (squash) February 5, 2026 23:55
@github-actions
Copy link

github-actions bot commented Feb 6, 2026

📊 Coverage check completed. See workflow run for details.

@preetriti1 preetriti1 merged commit 9e4f94a into main Feb 6, 2026
13 checks passed
@preetriti1 preetriti1 deleted the priti/mcpfixeserror branch February 6, 2026 00:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr-validated risk:low Low risk change with minimal impact

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants