Skip to content

fix(orchestrator): preserve nested extraErrors and async validation errors#2818

Merged
lokanandaprabhu merged 1 commit intoredhat-developer:mainfrom
karthikjeeyar:orchestrator/fix-nested-errors
Apr 20, 2026
Merged

fix(orchestrator): preserve nested extraErrors and async validation errors#2818
lokanandaprabhu merged 1 commit intoredhat-developer:mainfrom
karthikjeeyar:orchestrator/fix-nested-errors

Conversation

@karthikjeeyar
Copy link
Copy Markdown
Member

@karthikjeeyar karthikjeeyar commented Apr 17, 2026

Preserve nested extraErrors and async validation errors

Fixes:
https://redhat.atlassian.net/browse/RHDHBUGS-2970

Fixes cases where only one of several backend validation errors appeared for multi-step workflows, or errors did not show inline on nested fields under a wizard step.

Changes included in the PR:

  • safeSet builds the correct nested error tree for dotted paths.
  • Sequential await avoids losing updates when multiple fields validate async.
  • toRootExtraErrors ensures extraErrors passed to the root form match schema.properties (step keys), so RJSF + StepperObjectField show inline errors correctly.
flowchart LR
  A["Deep nested paths<br/>e.g. step.x.y.z"] -->|safeSet fix| B["Correct error tree<br/>built recursively"]
  C["Concurrent async<br/>validation calls"] -->|sequential await| D["All errors preserved<br/>no lost updates"]
  E["getExtraErrors<br/>returns step-keyed tree"] -->|toRootExtraErrors| F["Root-shaped errors<br/>for RJSF + StepperObjectField"]
  B --> G["Multi-step forms<br/>show all inline errors"]
  D --> G
  F --> G
Loading

How to test

1. Add the Backstage proxy so the browser does not call 127.0.0.1:7070 directly. Merge under proxy: in app-config.yaml or app-config.local.yaml:

proxy:
  endpoints:
    '/local-validation-mock':
      target: 'http://127.0.0.1:7070'
      changeOrigin: true
      allowedMethods: ['GET', 'POST', 'OPTIONS']
      allowedHeaders: ['Content-Type', 'Accept', 'Authorization']

2. Run a mock validation HTTP server on 127.0.0.1:7070 that answers POST to paths like /validate/fieldA with 422 and a JSON body such as { "validation": ["…"] }. Save the script below to any path (e.g. mock-validation-server.cjs in your workspace) and run:

Example mock server - `mock-validation-server.cjs`. (Node.js)
const http = require('http');
const PORT = Number(process.env.PORT || 7070);
const server = http.createServer((req, res) => {
  if (req.method !== 'POST') {
    res.writeHead(405, { 'Content-Type': 'application/json' });
    res.end(JSON.stringify({ validation: ['Method not allowed'] }));
    return;
  }
  const path = req.url || '';
  const m = path.match(/\/validate\/([^/?]+)/);
  const field = m ? m[1] : 'unknown';
  let body = '';
  req.on('data', c => {
    body += c;
  });
  req.on('end', () => {
    res.writeHead(422, { 'Content-Type': 'application/json' });
    res.end(
      JSON.stringify({
        validation: [`Mock validation failed for "${field}"`],
      }),
    );
  });
});
server.listen(PORT, '127.0.0.1', () =>
  console.log(`mock listening on http://127.0.0.1:${PORT}`),
);
node mock-validation-server.cjs

3. Register the workflow and input schema using the YAML/JSON in the collapsible sections below. validate:url must use the proxy, e.g. $${{backend.baseUrl}}/api/proxy/local-validation-mock/validate/fieldA (and fieldB, fieldC), not http://127.0.0.1:7070/... directly (avoids CORS).

Workflow definition (reference)
id: extra-errors-repro
version: '0.1'
specVersion: '0.8'
name: Extra errors reproduction (nested validate:url)
description: >-
  Demo workflow with a single wizard step property (my-solution), nested xParams, and
  three ActiveTextInput fields using validate:url for async backend validation.
dataInputSchema: schemas/extra-errors-repro__main-schema.json
start: Finish
functions:
  - name: noop
    type: custom
    operation: sysout
states:
  - name: Finish
    type: operation
    actions:
      - name: noopAction
        functionRef:
          refName: noop
          arguments:
            message: '"ok"'
    end:
      terminate: true
Input schema JSON (reference)
{
  "$id": "classpath:/schemas/extra-errors-repro__main-schema.json",
  "$schema": "http://json-schema.org/draft-07/schema#",
  "title": "Extra errors reproduction",
  "type": "object",
  "properties": {
    "my-solution": {
      "type": "object",
      "title": "Solution step",
      "properties": {
        "workflowParams": {
          "type": "object",
          "title": "Workflow params (hidden)",
          "ui:hidden": true,
          "additionalProperties": true
        },
        "xParams": {
          "type": "object",
          "title": "Cross parameters",
          "required": ["fieldA", "fieldB", "fieldC"],
          "properties": {
            "fieldA": {
              "type": "string",
              "title": "Field A",
              "default": "",
              "ui:widget": "ActiveTextInput",
              "ui:props": {
                "validate:url": "$${{backend.baseUrl}}/api/proxy/local-validation-mock/validate/fieldA",
                "validate:method": "POST",
                "validate:body": { "field": "fieldA" }
              }
            },
            "fieldB": {
              "type": "string",
              "title": "Field B",
              "default": "",
              "ui:widget": "ActiveTextInput",
              "ui:props": {
                "validate:url": "$${{backend.baseUrl}}/api/proxy/local-validation-mock/validate/fieldB",
                "validate:method": "POST",
                "validate:body": { "field": "fieldB" }
              }
            },
            "fieldC": {
              "type": "string",
              "title": "Field C",
              "default": "",
              "ui:widget": "ActiveTextInput",
              "ui:props": {
                "validate:url": "$${{backend.baseUrl}}/api/proxy/local-validation-mock/validate/fieldC",
                "validate:method": "POST",
                "validate:body": { "field": "fieldC" }
              }
            }
          }
        }
      }
    }
  },
  "required": ["my-solution"]
}

4. Run the app and open Execute workflow for extra-errors-repro.

5. What to verify

  • Leave fieldA / fieldB / fieldC empty.
  • Submit or Next so getExtraErrors runs (three validate:url requests).
  • Expected: Three 422 responses in the network trace and three distinct error messages in the UI (not a single one). Inline errors should appear under each field when the step is visible.

Before:

image

After:

image

✔️ Checklist

  • A changeset describing the change and affected packages. (more info)
  • Added or Updated documentation
  • Tests for new functionality and regression tests for bug fixes
  • Screenshots attached (for UI changes)

@rhdh-qodo-merge
Copy link
Copy Markdown

rhdh-qodo-merge Bot commented Apr 17, 2026

Code Review by Qodo

🐞 Bugs (1)   📘 Rule violations (0)   📎 Requirement gaps (0)
🐞\ ☼ Reliability (1)

Grey Divider


Action required

1. Unhandled rejections in loop 🐞
Description
useGetExtraErrors builds an array of already-started validation promises and then awaits them in a
for loop; if any awaited promise rejects, the function exits immediately and remaining promises
are never awaited/caught, so they can reject later as unhandled rejections. This can surface as
global unhandledrejection events and make validation failures noisy/non-deterministic while still
doing background network work.
Code

workspaces/orchestrator/plugins/orchestrator-form-widgets/src/utils/useGetExtraErrors.ts[R146-151]

+    // Run sequentially so concurrent async callbacks cannot interleave
+    // `safeSet` mutations on the shared `errors` object (lost updates under
+    // the same path prefix, e.g. `step.xParams.fieldA` vs `fieldB`).
+    for (const p of promises) {
+      await p;
+    }
Relevance

⭐⭐⭐ High

Repo previously fixed unhandled promise rejections; this pattern can leak unhandled rejections if
one await throws.

PR-#1090

ⓘ Recommendations generated based on similar findings in past PRs

Evidence
walkThrough invokes callback(...) immediately for each matching field and returns the resulting
Promise<void> objects (so the async operations are already in-flight when collected).
useGetExtraErrors then awaits these promises sequentially; if one await throws, the loop
terminates and later promises in the array are never awaited or .catch(...)’d, meaning any later
rejection has no handler attached by this function.

workspaces/orchestrator/plugins/orchestrator-form-widgets/src/utils/useGetExtraErrors.ts[32-66]
workspaces/orchestrator/plugins/orchestrator-form-widgets/src/utils/useGetExtraErrors.ts[139-152]

Agent prompt
The issue below was found during a code review. Follow the provided context and guidance below and implement a solution

### Issue description
`useGetExtraErrors` collects a list of already-created promises and awaits them in a loop. If any promise rejects, the loop exits early and the remaining promises are never awaited/caught, which can lead to unhandled rejections and background work continuing after the function has failed.

### Issue Context
`walkThrough` calls `callback(...)` immediately and returns `Promise<void>` values; these promises can settle (fulfill/reject) independently of the loop order.

### Fix Focus Areas
- workspaces/orchestrator/plugins/orchestrator-form-widgets/src/utils/useGetExtraErrors.ts[32-66]
- workspaces/orchestrator/plugins/orchestrator-form-widgets/src/utils/useGetExtraErrors.ts[139-152]

### Suggested fix
Choose one of:
1) **Revert to `Promise.all`** (now that `safeSet` correctly handles deep paths) to ensure all promises are handled:
```ts
await Promise.all(promises);
```

2) If you need to avoid failing fast while still surfacing an error, **use `Promise.allSettled`** and then decide how to report failures:
```ts
const results = await Promise.allSettled(promises);
const firstRejection = results.find(r => r.status === 'rejected');
if (firstRejection) throw firstRejection.reason;
```

3) If true sequential execution is desired, change `walkThrough` to produce **thunks** (functions) or make it an `async` walker that `await`s `callback` inline, instead of returning already-started promises.

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools


Grey Divider

ⓘ The new review experience is currently in Beta. Learn more

Grey Divider

Qodo Logo

@rhdh-gh-app
Copy link
Copy Markdown

rhdh-gh-app Bot commented Apr 17, 2026

Changed Packages

Package Name Package Path Changeset Bump Current Version
@red-hat-developer-hub/backstage-plugin-orchestrator-form-react workspaces/orchestrator/plugins/orchestrator-form-react patch v2.8.1
@red-hat-developer-hub/backstage-plugin-orchestrator-form-widgets workspaces/orchestrator/plugins/orchestrator-form-widgets patch v1.10.1

@rhdh-qodo-merge
Copy link
Copy Markdown

Review Summary by Qodo

Fix nested extraErrors and async validation in multi-step forms

🐞 Bug fix

Grey Divider

Walkthroughs

Description
• Fix safeSet to correctly handle deeply nested dotted paths
• Preserve all async validation errors by sequentially awaiting promises
• Wrap extraErrors with active step key for RJSF root schema matching
• Add comprehensive tests for error handling and nested field validation
Diagram
flowchart LR
  A["Deep nested paths<br/>e.g. step.x.y.z"] -->|safeSet fix| B["Correct error tree<br/>built recursively"]
  C["Concurrent async<br/>validation calls"] -->|sequential await| D["All errors preserved<br/>no lost updates"]
  E["getExtraErrors<br/>returns step-keyed tree"] -->|toRootExtraErrors| F["Root-shaped errors<br/>for RJSF + StepperObjectField"]
  B --> G["Multi-step forms<br/>show all inline errors"]
  D --> G
  F --> G
Loading

Grey Divider

File Changes

1. workspaces/orchestrator/plugins/orchestrator-form-widgets/src/utils/safeSet.ts 🐞 Bug fix +8/-6

Fix safeSet to handle deep nested paths

• Changed from split('.', 2) to indexOf('.') to properly handle deeply nested paths
• Recursively processes full remaining path instead of truncating at second segment
• Correctly builds nested error trees for paths with 3+ segments like step.xParams.fieldA

workspaces/orchestrator/plugins/orchestrator-form-widgets/src/utils/safeSet.ts


2. workspaces/orchestrator/plugins/orchestrator-form-widgets/src/utils/safeSet.test.ts 🧪 Tests +67/-0

Add comprehensive safeSet path handling tests

• Test for deep paths with 4+ segments to verify recursive handling
• Test for single-segment paths to ensure basic functionality
• Test for sequential sibling paths under shared prefix to verify merging behavior

workspaces/orchestrator/plugins/orchestrator-form-widgets/src/utils/safeSet.test.ts


3. workspaces/orchestrator/plugins/orchestrator-form-widgets/src/utils/useGetExtraErrors.ts 🐞 Bug fix +6/-1

Sequential await for async validation errors

• Changed from Promise.all() to sequential for...await loop
• Prevents concurrent mutations on shared errors object causing lost updates
• Ensures all async validation errors are preserved for nested fields

workspaces/orchestrator/plugins/orchestrator-form-widgets/src/utils/useGetExtraErrors.ts


View more (5)
4. workspaces/orchestrator/plugins/orchestrator-form-widgets/src/utils/useGetExtraErrors.test.tsx 🧪 Tests +120/-0

Test async validation error aggregation

• Test aggregation of async validation errors for multiple nested fields
• Verifies correct error tree structure under shared path prefix
• Mocks fetch API to simulate backend validation responses

workspaces/orchestrator/plugins/orchestrator-form-widgets/src/utils/useGetExtraErrors.test.tsx


5. workspaces/orchestrator/plugins/orchestrator-form-react/src/utils/toRootExtraErrors.ts ✨ Enhancement +41/-0

New utility to wrap errors for RJSF root schema

• New utility function to wrap extraErrors with active step key
• Ensures RJSF receives errors matching root schema.properties shape
• Handles undefined activeKey and missing step slices gracefully

workspaces/orchestrator/plugins/orchestrator-form-react/src/utils/toRootExtraErrors.ts


6. workspaces/orchestrator/plugins/orchestrator-form-react/src/utils/toRootExtraErrors.test.ts 🧪 Tests +53/-0

Test toRootExtraErrors wrapping logic

• Test undefined extraErrors returns undefined
• Test missing activeKey returns extraErrors unchanged
• Test active step slice is wrapped correctly for RJSF
• Test missing step in tree returns undefined

workspaces/orchestrator/plugins/orchestrator-form-react/src/utils/toRootExtraErrors.test.ts


7. workspaces/orchestrator/plugins/orchestrator-form-react/src/components/OrchestratorFormWrapper.tsx ✨ Enhancement +2/-9

Use toRootExtraErrors utility in form wrapper

• Import new toRootExtraErrors utility function
• Replace inline error wrapping logic with toRootExtraErrors call
• Simplify conditional logic for setting extraErrors

workspaces/orchestrator/plugins/orchestrator-form-react/src/components/OrchestratorFormWrapper.tsx


8. workspaces/orchestrator/.changeset/sharp-dots-sing.md 📝 Documentation +10/-0

Changelog entry for nested errors fix

• Document patch version bumps for both form packages
• Summarize fixes for async validation errors and deep nested paths
• Detail changes in safeSet, getExtraErrors, and toRootExtraErrors

workspaces/orchestrator/.changeset/sharp-dots-sing.md


Grey Divider

Qodo Logo

@karthikjeeyar karthikjeeyar force-pushed the orchestrator/fix-nested-errors branch from ced3c7e to fc77b77 Compare April 17, 2026 16:54
@sonarqubecloud
Copy link
Copy Markdown

Copy link
Copy Markdown
Member

@lokanandaprabhu lokanandaprabhu left a comment

Choose a reason for hiding this comment

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

/lgtm

Image

@openshift-ci openshift-ci Bot added the lgtm label Apr 20, 2026
@lokanandaprabhu lokanandaprabhu merged commit 4aec634 into redhat-developer:main Apr 20, 2026
12 checks passed
lokanandaprabhu pushed a commit to lokanandaprabhu/rhdh-plugins that referenced this pull request Apr 22, 2026
karthikjeeyar added a commit that referenced this pull request Apr 22, 2026
* fix(orchestrator): preserve nested extraErrors and async validation errors (#2818)

* fix(orchestrator-form): Review step with empty form data (#2834)

Empty pruned form data (e.g. display-only ActiveText) made generateReviewTableData return undefined and NestedReviewTable throw. Return {} from generateReviewTableData and guard NestedReviewTable. Add regression test and changeset.

Made-with: Cursor

* fix(orchestrator-form-react): fix ui styling issues in workflow results page (#2808)

---------

Co-authored-by: Karthik Jeeyar <karthik@redhat.com>
karthikjeeyar added a commit that referenced this pull request Apr 22, 2026
* fix(orchestrator): preserve nested extraErrors and async validation errors (#2818)

* fix(orchestrator-form): Review step with empty form data (#2834)

Empty pruned form data (e.g. display-only ActiveText) made generateReviewTableData return undefined and NestedReviewTable throw. Return {} from generateReviewTableData and guard NestedReviewTable. Add regression test and changeset.

Made-with: Cursor

* fix(orchestrator-form-react): fix ui styling issues in workflow results page (#2808)

---------

Co-authored-by: Karthik Jeeyar <karthik@redhat.com>
karthikjeeyar added a commit that referenced this pull request Apr 22, 2026
* fix(orchestrator): preserve nested extraErrors and async validation errors (#2818)

* fix(orchestrator-form): Review step with empty form data (#2834)

Empty pruned form data (e.g. display-only ActiveText) made generateReviewTableData return undefined and NestedReviewTable throw. Return {} from generateReviewTableData and guard NestedReviewTable. Add regression test and changeset.

Made-with: Cursor

* fix(orchestrator-form-react): fix ui styling issues in workflow results page (#2808)

---------

Co-authored-by: Lokananda Prabhu <102503482+lokanandaprabhu@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants