Fix: Correctly show error message in DQL and PPL query editor (opensearch-project#9586)

* Fix: Correctly show error message in DQL and PPL query editor

Signed-off-by: Joey Liu <jiyili@amazon.com>

* Changeset file for PR opensearch-project#9586 created/updated

* Update query status interface

Signed-off-by: Joey Liu <jiyili@amazon.com>

* Add unit test

Signed-off-by: Joey Liu <jiyili@amazon.com>

---------

Signed-off-by: Joey Liu <jiyili@amazon.com>
Co-authored-by: opensearch-changeset-bot[bot] <154024398+opensearch-changeset-bot[bot]@users.noreply.github.com>

Author: Maosaic

changelogs/fragments/9586.yml

@@ -0,0 +1,2 @@
+fix:
+- Correctly show error message in DQL and PPL query editor ([#9586](https://github.com/opensearch-project/OpenSearch-Dashboards/pull/9586))

src/plugins/data/public/query/query_string/language_service/lib/__snapshots__/query_result.test.tsx.snap

@@ -6,6 +6,7 @@
 import React from 'react';
 import { mountWithIntl, shallowWithIntl } from 'test_utils/enzyme_helpers';
 import { QueryResult } from './query_result';
+import { fireEvent, render, screen, waitFor } from '@testing-library/react';
 
 enum ResultStatus {
   UNINITIALIZED = 'uninitialized',
@@ -117,4 +118,79 @@ describe('Query Result', () => {
     const component = shallowWithIntl(<QueryResult {...props} />);
     expect(component).toEqual({});
   });
+
+  it('should render error message correctly with normal search strategy', async () => {
+    const props = {
+      queryStatus: {
+        status: ResultStatus.ERROR,
+        body: {
+          error: {
+            error: 'error',
+            statusCode: 400,
+            message: {
+              error: {
+                reason: 'error reason',
+                details: 'error details',
+                type: 'error type',
+              },
+              status: 400,
+            },
+          },
+        },
+      },
+    };
+
+    render(<QueryResult {...props} />);
+
+    await fireEvent.click(screen.getByText('Error'));
+
+    await waitFor(() => {
+      expect(screen.getByText('error details')).toBeInTheDocument();
+    });
+  });
+
+  it('should render error message correctly with async search strategy', async () => {
+    const props = {
+      queryStatus: {
+        status: ResultStatus.ERROR,
+        body: {
+          error: {
+            error: 'error',
+            statusCode: 400,
+            message: {
+              error: 'error message',
+              status: 400,
+            },
+          },
+        },
+      },
+    };
+
+    render(<QueryResult {...props} />);
+
+    await fireEvent.click(screen.getByText('Error'));
+
+    await waitFor(() => {
+      expect(screen.getByText('error message')).toBeInTheDocument();
+    });
+  });
+
+  it('should render error message with flexible error format', async () => {
+    const props = {
+      queryStatus: {
+        status: ResultStatus.ERROR,
+        body: {
+          error: 'error message',
+        },
+      },
+    };
+
+    render(<QueryResult {...props} />);
+
+    await fireEvent.click(screen.getByText('Error'));
+
+    await waitFor(() => {
+      expect(screen.getByText('error message')).toBeInTheDocument();
+    });
+  });
 });

src/plugins/data/public/query/query_string/language_service/lib/query_result.test.tsx

@@ -8,7 +8,7 @@ import { i18n } from '@osd/i18n';
 import './_recent_query.scss';
 import { EuiButtonEmpty, EuiPopover, EuiText, EuiPopoverTitle } from '@elastic/eui';
 
-import React, { useEffect, useState } from 'react';
+import React, { useEffect, useMemo, useState } from 'react';
 
 export enum ResultStatus {
   UNINITIALIZED = 'uninitialized',
@@ -22,8 +22,18 @@ export interface QueryStatus {
   status: ResultStatus;
   body?: {
     error?: {
+      error?: string;
+      message?: {
+        error?:
+          | string
+          | {
+              reason?: string;
+              details: string;
+              type?: string;
+            };
+        status?: number;
+      };
       statusCode?: number;
-      message?: string;
     };
   };
   elapsedMs?: number;
@@ -58,6 +68,43 @@ export function QueryResult(props: { queryStatus: QueryStatus }) {
     };
   }, [props.queryStatus.startTime]);
 
+  const displayErrorMessage = useMemo(() => {
+    const error = props.queryStatus.body?.error;
+    const message = error?.message;
+
+    if (message == null) {
+      if (typeof error === 'string') {
+        return error;
+      }
+
+      if (typeof error === 'object') {
+        return JSON.stringify(error);
+      }
+
+      return `Unknown Error: ${String(error)}`;
+    }
+
+    // For async search strategy, expecting message.error to be string
+    if (typeof message.error === 'string') {
+      return message.error;
+    }
+
+    // For normal search strategy, expecting message.error to be object
+    if (message.error?.details) {
+      return message.error.details;
+    }
+
+    if (typeof message === 'string') {
+      return message;
+    }
+
+    if (typeof message === 'object') {
+      return JSON.stringify(message);
+    }
+
+    return `Unknown Error: ${String(message)}`;
+  }, [props.queryStatus.body?.error]);
+
   if (props.queryStatus.status === ResultStatus.LOADING) {
     const time = Math.floor(elapsedTime / 1000);
     const loadingText =
@@ -166,7 +213,7 @@ export function QueryResult(props: { queryStatus: QueryStatus }) {
                 defaultMessage: `Message:`,
               })}
             </strong>{' '}
-            {props.queryStatus.body.error.message}
+            {displayErrorMessage}
           </p>
         </EuiText>
       </div>

src/plugins/data/public/query/query_string/language_service/lib/query_result.tsx

@@ -93,6 +93,10 @@ export const TopNav = ({ opts, showSaveQuery, isEnhancementsEnabled }: TopNavPro
       };
       setQueryStatus(result);
     });
+
+    return () => {
+      subscription.unsubscribe();
+    };
   }, [data$]);
 
   useEffect(() => {

src/plugins/discover/public/application/view_components/canvas/top_nav.tsx

@@ -12,7 +12,7 @@ import { Subject } from 'rxjs';
 import { createDataExplorerServicesMock } from '../../../../../data_explorer/public/utils/mocks';
 import { DiscoverViewServices } from '../../../build_services';
 import { discoverPluginMock } from '../../../mocks';
-import { ResultStatus, useSearch } from './use_search';
+import { ResultStatus, safeJSONParse, useSearch } from './use_search';
 import { Filter, ISearchSource, UI_SETTINGS } from '../../../../../data/common';
 import { opensearchFilters } from 'src/plugins/data/public';
 
@@ -418,3 +418,12 @@ describe('useSearch', () => {
     });
   });
 });
+
+describe('safeJSONParse', () => {
+  it('should covert string to JSON if possible', () => {
+    const validJSONString = '{"result":true, "count":42}';
+    const invalidJSONString = 'invalidJSON';
+    expect(safeJSONParse(validJSONString)).toEqual(JSON.parse(validJSONString));
+    expect(safeJSONParse(invalidJSONString)).toEqual(invalidJSONString);
+  });
+});

src/plugins/discover/public/application/view_components/utils/use_search.test.tsx

@@ -34,6 +34,14 @@ import { useSelector } from '../../utils/state_management';
 import { SEARCH_ON_PAGE_LOAD_SETTING } from '../../../../common';
 import { trackQueryMetric } from '../../../ui_metric';
 
+export function safeJSONParse(text: any) {
+  try {
+    return JSON.parse(text);
+  } catch (error) {
+    return text;
+  }
+}
+
 export enum ResultStatus {
   UNINITIALIZED = 'uninitialized',
   LOADING = 'loading', // initial data load
@@ -54,10 +62,19 @@ export interface SearchData {
   queryStatus?: {
     body?: {
       error?: {
-        reason?: string;
-        details: string;
+        error?: string;
+        message?: {
+          error?:
+            | string
+            | {
+                reason?: string;
+                details: string;
+                type?: string;
+              };
+          status?: number;
+        };
+        statusCode?: number;
       };
-      statusCode?: number;
     };
     elapsedMs?: number;
     startTime?: number;
@@ -318,17 +335,55 @@ export const useSearch = (services: DiscoverViewServices) => {
         data.search.showError(error as Error);
         return;
       }
+      // TODO: Create a unify error response at server side
       let errorBody;
       try {
+        // Normal search strategy failed query, return HttpFetchError
+        /*
+          @type {HttpFetchError}
+          {
+            body: {
+              error: string,
+              statusCode: number,
+              message: JSONstring,
+            },
+            ...
+          }
+        */
         errorBody = JSON.parse(error.body);
       } catch (e) {
         if (error.body) {
           errorBody = error.body;
         } else {
+          // Async search strategy failed query, return Error
+          /*
+            @type {Error}
+            {
+              message: string,
+              stack: string,
+            }
+          */
           errorBody = error;
         }
       }
 
+      // Error message can be sent as encoded JSON string, which requires extra parsing
+      /*
+        errorBody: {
+          error: string,
+          statusCode: number,
+          message: {
+            error: {
+              reason: string;
+              details: string;
+              type: string;
+            };
+            status: number;
+          }
+        }
+      */
+      errorBody.message = safeJSONParse(errorBody.message);
+
       data$.next({
         status: ResultStatus.ERROR,
         queryStatus: {

src/plugins/discover/public/application/view_components/utils/use_search.ts

@@ -83,8 +83,8 @@ export const fetch = (context: EnhancedFetchContext, query: Query, aggConfig?: Q
             // eslint-disable-next-line no-console
             console.error('Failed to cancel query:', cancelError);
           }
-          throw error;
         }
+        throw error;
       })
   );
 };