NotificationsYou must be signed in to change notification settings
Fork715
Star17.5k

fix: fixed traces error handling#8236

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Jump to bottom

Merged

omkarK06 merged 2 commits intomainfromfix/traces-undefined-error

Sep 3, 2025

Merged

fix: fixed traces error handling#8236

omkarK06 merged 2 commits intomainfromfix/traces-undefined-error

Sep 3, 2025

Conversation

Copy link

Contributor

omkarK06 commentedSep 1, 2025•
edited by github-actionsbot
Loading

PR Type

Bug fix

Description

Prevent undefined error messages in traces UI
Safely build HTML for error details
Reset error detail before each query
Robust parsing of backend error responses

Diagram Walkthrough

flowchart LR  Query["User runs trace query"] -- "API error" --> Catch["Catch error"]  Catch -- "extract error/message/code" --> Parse["Normalize error fields"]  Parse -- "set errorMsg/errorCode/errorDetail" --> State["UI state updated"]  State -- "render" --> Renderer["SanitizedHtmlRenderer shows details"]

File Walkthrough

Relevant files

Bug fix

Index.vue `Harden traces error handling and rendering` web/src/plugins/traces/Index.vue Build error HTML with safe conditional template string. Reset`errorDetail` before each query run. Handle error sources:`error`,`message`, and`code`. Populate`errorDetail` from`error_detail` when available.	+25/-12

github-actionsbot added the ☢️ BugSomething isn't working label

Sep 1, 2025

greptile-appsbot reviewed

Sep 1, 2025

View reviewed changes

Copy link

Contributor

greptile-appsbot left a comment

Choose a reason for hiding this comment

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

Greptile Summary

This PR fixes error handling issues in the traces page (web/src/plugins/traces/Index.vue) by addressing template interpolation problems and improving error message extraction from API responses. The changes focus on three main areas:

Template Fix: Added conditional rendering in the HTML template to prevent "undefined" from appearing in error messages by checking iferrorDetail exists before interpolating it
State Initialization: Properly initializes botherrorMsg anderrorDetail to empty strings to ensure they have defined values
Robust Error Parsing: Enhanced the error handling logic to accommodate different API response structures that the backend might return, checking forerror,message, anderror_detail fields in various response formats

This change aligns with the existing error handling patterns seen in the logs composable (from the provided context), where similar defensive programming techniques are used to extract trace IDs and error messages from different response structures. The traces page needed similar robustness to handle the variety of error response formats that can come from the backend services.

Confidence score: 4/5

This PR is safe to merge with minimal risk as it only improves error handling without changing core functionality
Score reflects straightforward defensive programming improvements that prevent UI display issues
No files require special attention as the changes are localized and follow established patterns

_{1 file reviewed, no comments}

_{Edit Code Review Bot Settings |Greptile}

github-actionsbot added the Review effort 2/5 label

Sep 1, 2025

Copy link

Contributor

github-actionsbot commentedSep 1, 2025

PR Reviewer Guide 🔍

Here are some key observations to aid the review process:

⏱️ Estimated effort to review: 2 🔵🔵⚪⚪⚪
🧪 No relevant tests
🔒 Security concerns Cross-site scripting (XSS) risk: Interpolating backend-provided strings into an HTML template string can be dangerous if SanitizedHtmlRenderer does not fully sanitize nested HTML. Ensure renderer escapes or sanitizes errorMsg and errorDetail, or pass them as plain text and construct safe HTML within the renderer.
⚡ Recommended focus areas for review Possible XSS via HTML injection Although a SanitizedHtmlRenderer is used, the template string now interpolates raw backend-provided errorMsg and errorDetail into HTML. Verify that SanitizedHtmlRenderer robustly sanitizes both text and nested HTML, especially the conditional wrapper, to prevent script/style/event-handler injection. :htmlContent="`${searchObj.data.errorMsg} ${searchObj.data.errorDetail ? `<h6style='font-size:14px;margin:0;'>${searchObj.data.errorDetail}</h6>` : ''}`"/> Error precedence The catch block sets errorMsg from multiple sources with overlapping conditions; later branches can overwrite earlier choices (e.g., custom mapped message replaced by backend message). Confirm desired precedence/order and deduplicate to avoid confusing user messaging. if (err.response.data.error) { searchObj.data.errorMsg = err.response.data.error; } else if (err.response.data.message) { searchObj.data.errorMsg = err.response.data.message; }} else if (err.message) { searchObj.data.errorMsg = err.message;}if (err.response?.data?.code) { const customMessage = logsErrorMessage(err.response.data.code); searchObj.data.errorCode = err.response.data.code; if (customMessage != "") { searchObj.data.errorMsg = t(customMessage); }}if (err.response?.data?.code && err.response?.data?.message) { searchObj.data.errorMsg = err.response.data.message; searchObj.data.errorCode = err.response.data.code;}if (err.response?.data?.code && err.response?.data?.error_detail) { searchObj.data.errorDetail = err.response.data.error_detail; searchObj.data.errorCode = err.response.data.code;} Optional chaining fallback Access to err.message occurs only in an else-if tied to err.response; if both are undefined or unexpected shapes, errorMsg may remain empty. Consider a final fallback ensuring a generic error message is always shown. if (err.response != undefined) { if (err.response.data.error) { searchObj.data.errorMsg = err.response.data.error; } else if (err.response.data.message) { searchObj.data.errorMsg = err.response.data.message; }} else if (err.message) { searchObj.data.errorMsg = err.message;}

Copy link

Contributor

github-actionsbot commentedSep 1, 2025

PR Code Suggestions ✨

Explore these optional code suggestions:

Category	Suggestion	Impact
Possible issue	Fix message precedence logic Avoid overwriting the localized/custom message with the raw backend message when both are present. Prefer a single precedence order and only fall back to the raw message if no custom translation exists. This prevents confusing or inconsistent messaging. web/src/plugins/traces/Index.vue [700-711] if (err.response?.data?.code) {- const customMessage = logsErrorMessage(err.response.data.code);- searchObj.data.errorCode = err.response.data.code;- if (customMessage != "") {+ const code = err.response.data.code;+ const customMessage = logsErrorMessage(code);+ searchObj.data.errorCode = code;++ if (customMessage) { searchObj.data.errorMsg = t(customMessage);+ } else if (err.response.data.message) {+ searchObj.data.errorMsg = err.response.data.message; } }-if (err.response?.data?.code && err.response?.data?.message) {- searchObj.data.errorMsg = err.response.data.message;- searchObj.data.errorCode = err.response.data.code;-}- Suggestion importance[1-10]: 8 __ Why: This accurately identifies that the code can overwrite a localized/custom message with a raw backend message and proposes a clear precedence order, improving user-facing consistency without altering behavior elsewhere.	Medium
Security	Prevent XSS in error rendering Avoid rendering raw`errorMsg`/`errorDetail` directly into HTML to prevent XSS if these values can contain user-controlled content. Pass plain text to the renderer and let it handle escaping, or supply them as separate props. If`SanitizedHtmlRenderer` expects HTML, ensure you sanitize both fields before interpolation. web/src/plugins/traces/Index.vue [93-97] <SanitizedHtmlRenderer data-test="logs-search-error-message"- :htmlContent="`${searchObj.data.errorMsg}- ${searchObj.data.errorDetail ? `<h6 style='font-size: 14px; margin: 0;'>${searchObj.data.errorDetail}</h6>` : ''}`"+ :htmlContent="+ [+ searchObj.data.errorMsg,+ searchObj.data.errorDetail ? `<h6 style='font-size: 14px; margin: 0;'>${$sanitize(searchObj.data.errorDetail)}</h6>` : ''+ ].map($sanitize).join('')+ " /> Suggestion importance[1-10]: 7 __ Why: Highlighting XSS risk in interpolating`errorMsg`/`errorDetail` into HTML is important, but the proposal references`$sanitize` which may not exist in this context and mixes array`.map($sanitize)` with already-sanitized component responsibility, reducing certainty.	Medium
General	Safely coerce error message Guard against non-string`err.message` to avoid rendering`[object Object]` or throwing. Coerce to string safely with fallback. This ensures consistent user-facing messages. web/src/plugins/traces/Index.vue [696-698] } else if (err.message) {- searchObj.data.errorMsg = err.message;+ searchObj.data.errorMsg = typeof err.message === 'string' ? err.message : JSON.stringify(err.message); } Suggestion importance[1-10]: 5 __ Why: Guarding against non-string`err.message` is reasonable and low-risk, but likely marginal since`Error.message` is typically a string; impact is small.	Low