Skip to content

fix(designer-v2): isolate per-operation init failures so one bad op can't hang the whole load (#9223)#9338

Open
rllyy97 wants to merge 2 commits into
mainfrom
rllyy97-fix-listmcpservers-400-blocks-ui
Open

fix(designer-v2): isolate per-operation init failures so one bad op can't hang the whole load (#9223)#9338
rllyy97 wants to merge 2 commits into
mainfrom
rllyy97-fix-listmcpservers-400-blocks-ui

Conversation

@rllyy97

@rllyy97 rllyy97 commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

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

Fixes #9223. In the New Logic Apps experience (DesignerV2), when a single operation-init call fails during workflow load, the entire UI hangs on an infinite spinner. The reported trigger is a Consumption Logic App where a Standard-only API (listMcpServers) returns 400, but the failure is generic: any single per-operation init rejection blocks the whole load.

Root cause: initializeOperationMetadata in operationdeserializer.ts awaited Promise.all(...) over the per-operation init promises. Because Promise.all rejects as soon as any promise rejects, initializeNodes was never dispatched — so no nodes initialized and every node stayed stuck loading. Separately, initializeConnectorOperationDetails in agent.ts was the only init function of its family not wrapped in try/catch, so its getAllConnectors() / getAgentConnectorOperation() calls could reject unguarded.

Changes (scoped to designer-v2):

  1. operationdeserializer.tsPromise.allPromise.allSettled. A failing operation now marks only that node with a Critical error via updateErrorDetails (and logs through LoggerService), while all healthy nodes still aggregate and initialize normally.
  2. agent.ts — wrapped initializeConnectorOperationDetails in try/catch, matching the existing pattern used by its sibling init functions (...ForManagedMcpServer, ...ForManifest, ...ForSwagger).

Scope notes: the equivalent designer v1 code is intentionally left unchanged — this fix targets the New Logic Apps experience (designer-v2) only. The specific listMcpServers bootstrap call that awaits without a catch is Portal-side (different repo); this change removes the entire class of "one failed init hangs the whole load" from the designer-v2 load path, so a 400 (or any rejection) from a single operation no longer blocks the UI.

Impact of Change

  • Users: Workflows that previously hung on an infinite spinner in DesignerV2 now load; a failing operation surfaces a node-level error while the rest of the workflow renders normally.
  • Developers: initializeOperationMetadata no longer throws on a single per-operation init rejection — failures are isolated per node. initializeConnectorOperationDetails now degrades gracefully instead of rejecting.
  • System: No new dependencies. The designer-v2 load path becomes resilient to individual operation-init failures instead of all-or-nothing.

Test Plan

  • Unit tests added/updated
  • E2E tests added/updated
  • Manual testing completed
  • Tested in:
    • Added agent.spec.ts (3 tests) in designer-v2 covering graceful degradation when connector/operation lookups reject.
    • Unit tests pass: designer-v2 agent 3/3, designer-v2 operationdeserializer 3/3 (existing MCP tests unaffected by the isolation refactor).
    • Biome clean; tsc --noEmit clean on designer-v2.

Contributors

Co-authored-by: Copilot 223556219+Copilot@users.noreply.github.com

Screenshots/Videos

N/A

Copilot AI review requested due to automatic review settings July 1, 2026 17:57
@github-actions

github-actions Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

🤖 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(designer-v2): isolate per-operation init failures so one bad op can't hang the whole load (#9223)
  • Issue: None — this is a clear, specific title and matches the change.
  • Recommendation: No change needed.

Commit Type

  • Properly selected (fix).
  • Only one commit type is selected, which is correct.

⚠️ Risk Level

  • The selected risk label/body is Medium, and the repo label is risk:medium.
  • Based on the diff, medium is appropriate, so no mismatch to correct.

What & Why

  • Current: Well-written and sufficiently detailed.
  • Issue: None required. It explains the bug, root cause, scope, and behavior change clearly.
  • Recommendation: Optional: shorten slightly if you want a more concise PR body, but this is already acceptable.

Impact of Change

  • The section is present and provides clear user, developer, and system impact.
  • Recommendation:
    • Users: Good as written.
    • Developers: Good as written.
    • System: Good as written.

Test Plan

  • Unit tests are added/updated in the diff (agent.spec.ts), which satisfies the test plan requirement.
  • No E2E tests are required since unit tests are present.
  • The manual testing checkbox being unchecked is fine because automated tests exist.

Contributors

  • Contributors are listed.
  • Recommendation: No change needed.

Screenshots/Videos

  • Marked N/A, which is appropriate for this non-visual change.
  • Recommendation: No change needed.

Summary Table

Section Status Recommendation
Title No change needed
Commit Type No change needed
Risk Level No change needed
What & Why No change needed
Impact of Change No change needed
Test Plan No change needed
Contributors No change needed
Screenshots/Videos No change needed

Final note: The PR body is complete and compliant, and the risk level matches the change scope. The PR passes the documentation/title-body review.**


Last updated: Wed, 01 Jul 2026 22:07:15 GMT

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

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 fixes a workflow-load hang in the Logic Apps Designer (DesignerV2, mirrored in designer v1) by isolating per-operation initialization failures so a single rejected init promise can’t prevent node initialization for the entire workflow.

Changes:

  • Switched per-operation init aggregation from Promise.all to Promise.allSettled and mark only the failed node as Critical via updateErrorDetails.
  • Wrapped initializeConnectorOperationDetails in try/catch to prevent unhandled rejections from connector/operation lookup calls.
  • Added unit tests validating graceful degradation when connector/operation lookups reject (in both designer and designer-v2).

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 4 comments.

Show a summary per file
File Description
Localize/lang/strings.json Prunes unused localized strings (auto-generated by extraction).
libs/designer/src/lib/core/actions/bjsworkflow/operationdeserializer.ts Uses Promise.allSettled to avoid whole-load failure on single operation init rejection; marks failed node with critical error.
libs/designer/src/lib/core/actions/bjsworkflow/agent.ts Adds guarded error handling + dispatches updateErrorDetails on connector init failures.
libs/designer/src/lib/core/actions/bjsworkflow/test/agent.spec.ts Adds unit tests for connector init failure scenarios.
libs/designer-v2/src/lib/core/actions/bjsworkflow/operationdeserializer.ts Same isolation behavior as designer v1 using Promise.allSettled.
libs/designer-v2/src/lib/core/actions/bjsworkflow/agent.ts Same guarded connector init error handling as designer v1.
libs/designer-v2/src/lib/core/actions/bjsworkflow/test/agent.spec.ts Same unit tests as designer v1 for connector init failure scenarios.

Comment thread libs/designer/src/lib/core/actions/bjsworkflow/operationdeserializer.ts Outdated
Comment on lines +169 to +170
const error = result.reason;
const message = `Unable to initialize operation details for '${failedNodeId}'. Error details - ${parseErrorMessage(error)}`;

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good catch. Fixed in 330bb2f by passing a node-scoped defaultErrorMessage to parseErrorMessage, so when the error has no recognizable message field it now returns a meaningful string instead of falling through to the raw error object (which rendered as "[object Object]").

Comment thread libs/designer/src/lib/core/actions/bjsworkflow/agent.ts Outdated

return [];
} catch (error: any) {
const message = `Unable to initialize connector operation details for '${nodeId}'. Error details - ${parseErrorMessage(error)}`;

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Good catch. Fixed in 330bb2f by passing a node-scoped defaultErrorMessage to parseErrorMessage, so when the error has no recognizable message field it now returns a meaningful string instead of falling through to the raw error object (which rendered as "[object Object]").

@github-actions

github-actions Bot commented Jul 1, 2026

Copy link
Copy Markdown
Contributor

📊 Coverage Check

The following changed files need attention:

⚠️ libs/designer-v2/src/lib/core/actions/bjsworkflow/agent.ts - 26% covered (needs improvement)
⚠️ libs/designer-v2/src/lib/core/actions/bjsworkflow/operationdeserializer.ts - 16% covered (needs improvement)

Please add tests for the uncovered files before merging.

…an't hang the whole load (#9223)

initializeOperationMetadata awaited Promise.all over per-operation init calls; a single rejection (e.g. a Standard-only API like listMcpServers returning 400 for a Consumption app) prevented initializeNodes from dispatching, so every node hung on a spinner in the New Logic Apps experience (DesignerV2).

Switch to Promise.allSettled and mark only the failing node with a Critical error, and wrap the previously-unguarded initializeConnectorOperationDetails in try/catch to match its sibling init functions. Scoped to designer-v2, with unit tests.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@rllyy97 rllyy97 force-pushed the rllyy97-fix-listmcpservers-400-blocks-ui branch from 28e6548 to ead0ee0 Compare July 1, 2026 21:19
@rllyy97 rllyy97 changed the title fix(designer): isolate per-operation init failures so one bad op can't hang the whole load (#9223) fix(designer-v2): isolate per-operation init failures so one bad op can't hang the whole load (#9223) Jul 1, 2026
…[object Object]

parseErrorMessage falls back to returning the raw error when no message field is present, which renders as '[object Object]' in the logged/user-facing text. Provide a node-scoped default so the error message is always meaningful (addresses PR review feedback on #9223).

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@rllyy97 rllyy97 added the risk:medium Medium risk change with potential impact label Jul 1, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

needs-pr-update risk:medium Medium risk change with potential impact

Projects

None yet

Development

Successfully merging this pull request may close these issues.

"New Logic Apps experience" cannot handle a 400 on listMcpServers gracefully instead it blocks the entire UI load.

2 participants