fix: XML preview crash when response contains <error> root element

[abhishek-bruno]

Description

• Fixes crash in XML preview when the response XML has an root element (e.g. OData error responses)
• The parsed XML object {_xmlns, code, message} was colliding with the internal error sentinel check (parsedData.error), causing it to be passed as a React child to ErrorBanner
• Fix: tighten the check from parsedData.error to typeof parsedData.error === 'string' so only actual parse errors are caught

JIRA

Test plan

Send a request that returns an XML error response like:

<error xmlns="http://docs.oasis-open.org/odata/ns/metadata">
    <code>Unauthorized</code>
    <message>The credentials provided are incorrect</message>
</error>

1. Toggle XML preview — should render as a tree instead of crashing
2. Verify that invalid XML still shows the parse error banner correctly

Fixes #7735

Contribution Checklist:

• I've used AI significantly to create this pull request
• The pull request only addresses one issue or adds one feature.
• The pull request does not introduce any breaking changes
• I have added screenshots or gifs to help explain the change if applicable.
• I have read the contribution guidelines.
• Create an issue and link to the pull request.

Note: Keeping the PR small and focused helps make it easier to review and merge. If you have multiple changes you want to make, please consider submitting them as separate pull requests.

Publishing to New Package Managers

Please see here for more information.

[coderabbitai]

Walkthrough

Fixed XML preview error detection in XmlPreview component by explicitly checking if parsedData.error is a string type, preventing false positives when XML content contains an "error" element that gets parsed as an object.

Changes

Cohort / File(s)	Summary
XML Preview Error Handling packages/bruno-app/src/components/ResponsePane/QueryResult/QueryResultPreview/XmlPreview/index.js	Changed error condition from truthiness check to explicit string type check (typeof parsedData.error === 'string'), preventing erroneous error banners when XML has an "error" element.

Estimated code review effort

🎯 1 (Trivial) | ⏱️ ~3 minutes

Suggested reviewers

• helloanoop
• lohit-bruno
• naman-bruno
• bijin-bruno

Poem

🎯 When XML met an <error> tag,
The parser took a stumble, dragged and lagged,
But now it's sharp—a type-check true,
Distinguishing real errors from legitimate XML too! ✨

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Title check	✅ Passed	The title directly addresses the main issue: fixing a crash when XML preview encounters an root element.
Linked Issues check	✅ Passed	The code change properly implements the fix for issue #7735 by adding a string type check to distinguish parsing errors from valid XML structures containing an element.
Out of Scope Changes check	✅ Passed	Changes are narrowly scoped to the XML preview error detection logic and directly address the linked issue with no extraneous modifications.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create PR with unit tests

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In
`@packages/bruno-app/src/components/ResponsePane/QueryResult/QueryResultPreview/XmlPreview/index.js`:
- Around line 20-21: The current check treats any parsed object with an error
key as a parse failure and misclassifies valid XML like <error>...</error>;
change the parser to attach a dedicated parse-error field (e.g., __parseError or
parseError) to the parse result when parsing fails, and update the detection
logic to check that dedicated field (replace checks on parsedData.error with
parsedData.__parseError or parsedData.parseError). Locate where parsedData is
produced and where the condition is evaluated (references: parsedData) and
ensure only the dedicated parse-error field is used to detect parse failures so
legitimate <error> elements are preserved.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 3258f0ce-8f9c-4834-916a-2cf195a4787a

📥 Commits

Reviewing files that changed from the base of the PR and between cd06f28 and df93537.

📒 Files selected for processing (1)

• packages/bruno-app/src/components/ResponsePane/QueryResult/QueryResultPreview/XmlPreview/index.js

[coderabbitai]

⚠️ Potential issue | 🟡 Minor

Use a dedicated parse-error field instead of error key collision

Line 21 still misclassifies valid XML like <error>Unauthorized</error> as a parse failure, because parsed XML can legitimately produce { error: '...' }.

💡 Suggested fix

-  const parsedData = useMemo(() => {
+  const parsedResult = useMemo(() => {
     if (typeof data !== 'string') {
-      return { error: 'Invalid input. Expected an XML string.' };
+      return { data: null, parseError: 'Invalid input. Expected an XML string.' };
     }

     const parsed = parseXMLString(data);
     if (parsed === null) {
-      return { error: 'Failed to parse XML string. Invalid XML format.' };
+      return { data: null, parseError: 'Failed to parse XML string. Invalid XML format.' };
     }
-    return parsed;
+    return { data: parsed, parseError: null };
   }, [data]);

-  if (parsedData && typeof parsedData === 'object' && typeof parsedData.error === 'string') {
+  if (parsedResult.parseError) {
     return (
       <div className="px-2">
-        <ErrorBanner errors={[{ title: 'Cannot preview as XML', message: parsedData.error }]} />
+        <ErrorBanner errors={[{ title: 'Cannot preview as XML', message: parsedResult.parseError }]} />
       </div>
     );
   }
+
+  const parsedData = parsedResult.data;

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In
`@packages/bruno-app/src/components/ResponsePane/QueryResult/QueryResultPreview/XmlPreview/index.js`
around lines 20 - 21, The current check treats any parsed object with an error
key as a parse failure and misclassifies valid XML like <error>...</error>;
change the parser to attach a dedicated parse-error field (e.g., __parseError or
parseError) to the parse result when parsing fails, and update the detection
logic to check that dedicated field (replace checks on parsedData.error with
parsedData.__parseError or parsedData.parseError). Locate where parsedData is
produced and where the condition is evaluated (references: parsedData) and
ensure only the dedicated parse-error field is used to detect parse failures so
legitimate <error> elements are preserved.

[sanish-bruno]

@abhishek-bruno is there anyway we can test this behaviour?

[sanish-bruno]

@abhishek-bruno How are we representing the xml error now in the UI? what is the typeof error we receive.