Fixes #31822: Surface domain tree search failures instead of rejecting unhandled

[harsh-vador]

Describe your changes:

Fixes #31822

A failed domain search was indistinguishable from an empty one, and the rejection escaped the component.

onSearch in DomainSelectableTree.tsx and DomainSelectableTreeNew.tsx awaited searchDomains inside a try { ... } finally { setIsLoading(false) } with no catch. The callback is async and is invoked by lodash.debounce, so nothing awaits it — a rejected request became an unhandled promise rejection. The user-visible damage: the loader cleared, treeData kept its previous value, and the list fell through to the empty branch, so the dropdown said "No Domains available" for a request that had in fact failed. No error, no way to retry the term still sitting in the box. The sibling calls in the same component, fetchAPI and loadChildDomains, both already catch → showErrorToast, so the search path was the only outlier.

Reproducible with no code changes: type a AND OR b into any domain selector. escapeESReservedCharacters escapes reserved characters, not boolean keywords, so that reaches OpenSearch as q=*a AND OR b*, a query_string parse error — and the dropdown reports "No Domains available".

I also added the missing single-| entry to ES_RESERVED_CHARACTERS. The map is applied one character at a time, so its '||' key never matched and a lone pipe reached OpenSearch unescaped. || is Lucene's OR, so q=*a||b* searches *a OR b* instead of the literal string typed. '&' was already mapped individually alongside '&&'; '|' was simply missing. This is a correctness fix, not a crash fix — see the note at the bottom.

Type of change:

• Bug fix

High-level design:

N/A — small change.

• DomainSelectableTree.tsx / DomainSelectableTreeNew.tsx — onSearch now catches, sets a hasSearchError flag and clears treeData. The treeContent memo gains a branch between isLoading and the empty state, so a failure can never again render as "No Domains available". The branch uses each file's own existing empty-state markup (Box/Typography in the first, a plain div in New) plus a Try Again that re-runs the same term. The flag clears on the next search and in fetchAPI, which runs when the box is emptied.
• utils/StringUtils.ts — add '|': String.raw|``.

On the surface for the error (per review feedback from @chirag-madlani): the failure belongs in the list, not in a toast. The user is looking at the dropdown, so that is where both the error and the recovery go. showErrorToast stays on fetchAPI and loadChildDomains — those are the initial and child loads, where there is no search term to retry.

On the escaper change: Lucene's classic query parser accepts \ before any character and treats the pair as a literal (_ESCAPED_CHAR), so \| searches for a literal pipe. Same treatment '&' already gets. Only affects terms containing a pipe.

Tests:

Use cases covered

• A domain search that fails renders an inline error inside the dropdown, not a toast and not the empty state, and the tree stays usable.
• Try Again re-runs the same term and recovers.
• Clearing the search box clears the error and reloads the root list.
• A term containing pipes is escaped so it cannot form a Lucene OR.

Unit tests

• I added unit tests for the changed logic.
• Files updated:
  • components/common/DomainSelectableTree/DomainSelectableTree.test.tsx — three tests: inline error shown (and showErrorToast not called), retry recovers, clearing the box clears the error. Verified all three fail without the catch and pass with it.
  • utils/StringUtils.test.ts — new escapeESReservedCharacters block: all reserved characters, the single/consecutive-pipe cases, a plain term left untouched, undefined → ''.

yarn jest src/components/common/DomainSelectableTree/ src/utils/StringUtils.test.ts
  Test Suites: 2 passed, 2 total
  Tests:       89 passed, 89 total

escapeESReservedCharacters is shared, so I also ran every other suite that exercises it — TagClassBase, QuickLinkFormModal, useTestSuitesData, useTestCaseList, DomainTreeView, ExploreUtils:

  Test Suites: 12 passed, 12 total
  Tests:       196 passed, 196 total

eslint clean on all changed files (0 errors; pre-existing warnings in DomainSelectableTreeNew.tsx untouched). tsc --noEmit: identical error count before and after this branch.

Backend integration tests

• Not applicable (no backend changes).

Ingestion integration tests

• Not applicable (no ingestion changes).

Playwright (UI) tests

• Not applicable — the change is an error path that needs a failing search request; covered by the unit tests above.

Manual testing performed

1. Ran a local stack (server + Elasticsearch) and probed /api/v1/search/query and /api/v1/domains/hierarchy directly with curl to establish the real status codes — table at the bottom.
2. Confirmed q=*a AND OR b* on domain_search_index fails with a query_shard_exception, which is the reproduction path in the issue.
3. Verified the inline state, the retry, and the clear-the-box path in the browser by blocking */api/v1/search/query* in DevTools → Network against the patched build.

UI screen recording / screenshots:

No visual change on any success path. The changed surface is the failure branch: where the dropdown previously showed "No Domains available" for a failed search, it now shows Error while fetching Domains with a Try Again link, in the same position and using the same markup as the existing empty state.

Checklist:

• I have read the CONTRIBUTING document.
• My PR title is Fixes <issue-number>: <short explanation>
• My PR is linked to a GitHub issue via Fixes #<issue-number> above.
• I have commented on my code, particularly in hard-to-understand areas.
• For JSON Schema changes: not applicable.
• For UI changes: described above; no visual change on success paths.
• I have added tests and listed them above.

Bug fix

• I have added a test that covers the exact scenario we are fixing.

Correction to an earlier revision of this description

The first version of this PR body claimed the unescaped pipe caused an HTTP 400 on /search/query, and framed the escaper change as the fix for it. That was wrong. Measured against a running 1.13 stack:

Request	Status
q=*a||||b*, index=domain_search_index	200
q=*a AND OR b* (Lucene parse error)	500
q=*, from=9995 (past the 10k result window)	500
index=nope_search_index	500
/domains/hierarchy?fields=parent,bogus	400 Invalid field name bogus
/domains/hierarchy?offset=-1	400 must be greater than or equal to 0

/search/query surfaces its failures as 500, not 400. So the two changes here are independent: the missing catch is a real defect on any status code (500 parse errors, network drops, refresh races), and the missing | escape is a separate correctness gap that returns 200 today. I could not measure the pipe's effect on returned hits — the instance I tested had no indexed data.

Greptile Summary

This PR surfaces failed domain searches as an inline, retryable error state and adds escaping for individual pipe characters in Elasticsearch query terms.

• Adds search-error state, recovery controls, and stale-tree clearing to both domain selector implementations.
• Adds tests for error rendering, retry recovery, clearing the search, server messages, and reserved-character escaping.
• Extends ES_RESERVED_CHARACTERS with the missing single-pipe mapping.

Confidence Score: 5/5

The PR appears safe to merge.

No blocking failure remains.

Important Files Changed

Filename	Overview
openmetadata-ui/src/main/resources/ui/src/components/common/DomainSelectableTree/DomainSelectableTree.tsx	Catches failed searches and replaces the misleading empty state with an inline error and retry action.
openmetadata-ui/src/main/resources/ui/src/components/common/DomainSelectableTree/DomainSelectableTreeNew.tsx	Applies the same inline search-error and retry behavior to the newer domain selector implementation.
openmetadata-ui/src/main/resources/ui/src/components/common/DomainSelectableTree/DomainSelectableTree.test.tsx	Covers fallback and server error messages, retry recovery, and clearing the failed search.
openmetadata-ui/src/main/resources/ui/src/utils/StringUtils.ts	Escapes individual pipe characters so consecutive pipes cannot become a Lucene OR operator.
openmetadata-ui/src/main/resources/ui/src/utils/StringUtils.test.ts	Adds focused coverage for reserved characters, pipes, plain text, and undefined input.

_{Reviews (9): Last reviewed commit: "Merge branch 'main' into fix/domain-tree..." | Re-trigger Greptile}

[github-actions]

❌ PR checklist incomplete

This PR cannot be merged until the following are addressed on its linked issue:

• Linked issue Domain tree search failure is an unhandled promise rejection (no error toast); single '|' is never ES-escaped #31822 is not added to the Shipping project.

The fields live on the linked issue in the Shipping project (open the issue → right sidebar → Projects). After you set them, re-run this check (or push a commit) — issue/project changes do not re-trigger it automatically.

Maintainers can bypass this check by adding the skip-pr-checks label.

[github-actions]

Hi there 👋 Thanks for your contribution!

The OpenMetadata team will review the PR shortly! Once it has been labeled as safe to test, the CI workflows
will start executing and we'll be able to make sure everything is working as expected.

Let us know if you need any help!

[github-actions]

✅ Playwright Results — workflow succeeded

Validated commit 81602ce5a026166a1c1df614e73b1fd634028bf2 in Playwright run 32460545993, attempt 1.

✅ 553 passed · ❌ 0 failed · 🟡 0 flaky · ⏭️ 0 skipped · 🧰 0 lifecycle flaky

Performance

Blocking targets: ✅ met · Optimization targets: 🟡 in progress

Shard-job maxima below are not the full workflow wall time; the linked run includes build, fixture, planning, and reporting.

🕒 Full workflow signal wall (to summary) 46m 57s

⏱️ Max setup 4m 11s · max shard execution 14m 52s · max shard-job elapsed before upload 17m 54s · reporting 4s

🌐 215.67 requests/attempt · 2.83 app boots/UI scenario · 26.68% common-shard skew

Optimization targets still in progress:

• Common shard skew was 26.68% (convergence target: at most 15%).
• Browser traffic was 215.67 requests per attempt (convergence target: fewer than 200).
• Application boot ratio was 2.83 per UI scenario (1622 boots / 574 scenarios; convergence target: at most 1).

Shard	Passed	Failed	Flaky	Skipped	Lifecycle failed	Lifecycle flaky
✅ Shard chromium-01	103	0	0	0	0	0
✅ Shard chromium-02	103	0	0	0	0	0
✅ Shard chromium-03	91	0	0	0	0	0
✅ Shard chromium-04	102	0	0	0	0	0
✅ Shard data-asset-rules-01	61	0	0	0	0	0
✅ Shard domain-isolation-01	16	0	0	0	0	0
✅ Shard global-state-01	34	0	0	0	0	0
✅ Shard ingestion-01	1	0	0	0	0	0
✅ Shard reindex-01	2	0	0	0	0	0
✅ Shard search-01	11	0	0	0	0	0
✅ Shard search-rbac-01	29	0	0	0	0	0

📦 Download artifacts

How to debug locally

# Download playwright-test-results-<shard> artifact and unzip
npx playwright show-trace path/to/trace.zip    # view trace

[github-actions]

Jest test Coverage

UI tests summary

Lines	Statements	Branches	Functions
	67.04% (80391/119913)	51.47% (49244/95662)	52.44% (14700/28027)

[github-actions]

⚠️ UI Checkstyle passed — lint findings in changed files

🔍 ESLint findings in this PR's files — 0 error(s), 19 warning(s)

Errors block the build. Warnings do not yet — they are rules whose backlog is still
being worked down, listed so this PR does not add to it. See docs/ui-code-quality-gate.md.

0 error(s), 19 warning(s) across 2 changed file(s).

Count	Rule
13	react-hooks/exhaustive-deps
2	jsx-a11y/click-events-have-key-events
2	jsx-a11y/no-static-element-interactions
1	jsx-a11y/no-autofocus
1	sonarjs/no-duplicate-string

All findings

	Location	Rule	Message
🟡	src/components/common/DomainSelectableTree/DomainSelectableTree.tsx:405:5	react-hooks/exhaustive-deps	React Hook useCallback has an unnecessary dependency: 'initialDomains'. Either exclude it or remove the dependency array.
🟡	src/components/common/DomainSelectableTree/DomainSelectableTree.tsx:408:9	react-hooks/exhaustive-deps	The 'onSelect' function makes the dependencies of useMemo Hook (at line 626) change on every render. Move it inside the useMemo callback. Alternatively, wrap th
🟡	src/components/common/DomainSelectableTree/DomainSelectableTree.tsx:427:9	react-hooks/exhaustive-deps	The 'onCheck' function makes the dependencies of useMemo Hook (at line 626) change on every render. Move it inside the useMemo callback. Alternatively, wrap the
🟡	src/components/common/DomainSelectableTree/DomainSelectableTree.tsx:449:20	react-hooks/exhaustive-deps	React Hook useCallback received a function whose dependencies are unknown. Pass an inline function instead.
🟡	src/components/common/DomainSelectableTree/DomainSelectableTree.tsx:535:5	react-hooks/exhaustive-deps	React Hook useCallback has an unnecessary dependency: 'domains.length'. Either exclude it or remove the dependency array.
🟡	src/components/common/DomainSelectableTree/DomainSelectableTree.tsx:649:6	react-hooks/exhaustive-deps	React Hook useEffect has a missing dependency: 'fetchAPI'. Either include it or remove the dependency array.
🟡	src/components/common/DomainSelectableTree/DomainSelectableTree.tsx:673:6	react-hooks/exhaustive-deps	React Hook useEffect has missing dependencies: 'loadChildDomains' and 'loadingChildren'. Either include them or remove the dependency array.
🟡	src/components/common/DomainSelectableTree/DomainSelectableTree.tsx:686:9	jsx-a11y/no-autofocus	The autoFocus prop should not be used, as it can reduce usability and accessibility for users.
🟡	src/components/common/DomainSelectableTree/DomainSelectableTreeNew.tsx:249:13	jsx-a11y/click-events-have-key-events	Visible, non-interactive elements with click handlers must have at least one keyboard listener.
🟡	src/components/common/DomainSelectableTree/DomainSelectableTreeNew.tsx:249:13	jsx-a11y/no-static-element-interactions	Avoid non-native interactive elements. If using native HTML is not possible, add an appropriate role and support for tabbing, mouse, keyboard, and touch inputs
🟡	src/components/common/DomainSelectableTree/DomainSelectableTreeNew.tsx:303:6	react-hooks/exhaustive-deps	React Hook useEffect has a missing dependency: 'buildIndex'. Either include it or remove the dependency array.
🟡	src/components/common/DomainSelectableTree/DomainSelectableTreeNew.tsx:396:5	react-hooks/exhaustive-deps	React Hook useCallback has an unnecessary dependency: 'initialDomains'. Either exclude it or remove the dependency array.
🟡	src/components/common/DomainSelectableTree/DomainSelectableTreeNew.tsx:399:9	react-hooks/exhaustive-deps	The 'onSelect' function makes the dependencies of useMemo Hook (at line 596) change on every render. Move it inside the useMemo callback. Alternatively, wrap th
🟡	src/components/common/DomainSelectableTree/DomainSelectableTreeNew.tsx:417:9	react-hooks/exhaustive-deps	The 'onCheck' function makes the dependencies of useMemo Hook (at line 596) change on every render. Move it inside the useMemo callback. Alternatively, wrap the
🟡	src/components/common/DomainSelectableTree/DomainSelectableTreeNew.tsx:451:20	react-hooks/exhaustive-deps	React Hook useCallback received a function whose dependencies are unknown. Pass an inline function instead.
🟡	src/components/common/DomainSelectableTree/DomainSelectableTreeNew.tsx:542:27	sonarjs/no-duplicate-string	Define a constant instead of duplicating this literal 3 times.
🟡	src/components/common/DomainSelectableTree/DomainSelectableTreeNew.tsx:621:6	react-hooks/exhaustive-deps	React Hook useEffect has a missing dependency: 'fetchAPI'. Either include it or remove the dependency array.
🟡	src/components/common/DomainSelectableTree/DomainSelectableTreeNew.tsx:691:7	jsx-a11y/click-events-have-key-events	Visible, non-interactive elements with click handlers must have at least one keyboard listener.
🟡	src/components/common/DomainSelectableTree/DomainSelectableTreeNew.tsx:691:7	jsx-a11y/no-static-element-interactions	Avoid non-native interactive elements. If using native HTML is not possible, add an appropriate role and support for tabbing, mouse, keyboard, and touch inputs

---

Fix locally (fast - only checks files changed in this branch):

make ui-checkstyle-changed

[gitar-bot]

Code Review ✅ Approved 2 resolved / 2 findings

Surfaces domain tree search failures inside the dropdown with a retry option rather than producing an unhandled rejection, but consider sanitizing the raw server error message before displaying it to users.

✅ 2 resolved

✅ Quality: Search failures are now fully swallowed with no logging

📄 openmetadata-ui/src/main/resources/ui/src/components/common/DomainSelectableTree/DomainSelectableTree.tsx:481-485 📄 openmetadata-ui/src/main/resources/ui/src/components/common/DomainSelectableTree/DomainSelectableTreeNew.tsx:479-483
The onSearch catch block was changed from catch (error) { showErrorToast(error as AxiosError) } to an empty catch { setHasSearchError(true); setTreeData([]) } in both DomainSelectableTree.tsx and DomainSelectableTreeNew.tsx. This surfaces failures inline (good UX), but it now discards the error entirely — no toast, no console, no telemetry. Genuine backend failures (500s, network errors), not just the expected 400, become invisible to any browser error reporter. Consider logging the caught error (e.g. console.error/existing logger) inside the catch so real failures remain observable while keeping the inline UI.

✅ Security: Raw server error message surfaced directly in domain dropdown

📄 openmetadata-ui/src/main/resources/ui/src/components/common/DomainSelectableTree/DomainSelectableTree.tsx:553-558 📄 openmetadata-ui/src/main/resources/ui/src/components/common/DomainSelectableTree/DomainSelectableTreeNew.tsx:539-544
getErrorText(searchError, fallback) renders response.data.message straight into the dropdown. As the PR notes, /search/query failures come back as HTTP 500 (e.g. OpenSearch query_shard_exception), so the raw message can leak internal detail (index/field names, parser internals) into the UI for what is really a client-side parse issue. Consider showing the server message only for 4xx (client-actionable) responses and falling back to the generic server.entity-fetch-error string for 5xx/unknown errors.

Options

Display: compact → Showing less information.

Comment with these commands to change the behavior for this request:

Compact

gitar display:verbose

_{Was this helpful? React with 👍 / 👎 | Powered by Gitar — free for open source}

[sonarqubecloud]

Quality Gate passed for 'open-metadata-ui'

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
14.3% Duplication on New Code

See analysis details on SonarQube Cloud

[harsh-vador]

Filed the server-side half separately: #31878.

/search/query returns 500 for a query the user typed wrong — a AND OR b produces query_shard_exception: Failed to parse query, and nothing on the server actually failed. Same for an out-of-range from, or an unknown index. That should be a 400; 500 should mean the search backend is genuinely broken.

Keeping it out of this PR: it is a backend change in both search managers plus SearchException, on a surface shared by Explore and every entity picker. This PR stays on the client side — a failed search must not render as an empty result.

Worth noting it also settles the open review point here. The suggestion was to show the server's message only for 4xx and stay generic for 5xx. Today the one message a user can act on ("your search text is malformed") arrives as a 500, so that gate would hide exactly the useful case. Once #31878 lands and parse errors are 400s, the gate costs nothing and becomes the right call.