From 3e6085d379b76f0d473faf2b3f61896c176f7061 Mon Sep 17 00:00:00 2001 From: Alex Simonok Date: Fri, 25 Oct 2024 13:00:47 +0300 Subject: [PATCH] Add Handling Data Source Request Errors (#530) * Add Handling Data Source Request Errors * Move useDatasourceRequest hook to package --------- Co-authored-by: Mikhail Volkov --- package-lock.json | 14 +- package.json | 2 +- src/__mocks__/@grafana/scenes.ts | 10 +- src/__mocks__/@volkovlabs/components.tsx | 3 + src/components/FormPanel/FormPanel.styles.ts | 4 + src/components/FormPanel/FormPanel.test.tsx | 26 +--- src/components/FormPanel/FormPanel.tsx | 19 ++- src/hooks/index.ts | 1 - src/hooks/useDatasourceRequest.test.ts | 129 ------------------- src/hooks/useDatasourceRequest.ts | 56 -------- src/utils/request.test.ts | 30 ++++- src/utils/request.ts | 24 ++++ 12 files changed, 92 insertions(+), 226 deletions(-) delete mode 100644 src/hooks/useDatasourceRequest.test.ts delete mode 100644 src/hooks/useDatasourceRequest.ts diff --git a/package-lock.json b/package-lock.json index 892d60fa..cd585167 100644 --- a/package-lock.json +++ b/package-lock.json @@ -15,7 +15,7 @@ "@grafana/scenes": "5.20.0", "@grafana/ui": "^11.2.0", "@hello-pangea/dnd": "^16.6.0", - "@volkovlabs/components": "^3.4.1", + "@volkovlabs/components": "^3.5.0", "react": "^18.3.1", "react-dom": "^18.3.1", "semver": "^7.6.3", @@ -4249,9 +4249,9 @@ "dev": true }, "node_modules/@volkovlabs/components": { - "version": "3.4.1", - "resolved": "https://registry.npmjs.org/@volkovlabs/components/-/components-3.4.1.tgz", - "integrity": "sha512-jp1jYlw15Y2bq2hKp0Fq3e/beBdAZeebWZfvpa7WeHQMA+/gsqieon6un/rxT5t87tXEaj8SRGef5ZUqDQ8dqw==", + "version": "3.5.0", + "resolved": "https://registry.npmjs.org/@volkovlabs/components/-/components-3.5.0.tgz", + "integrity": "sha512-WFHELd5hlBPmSb3vzOPWUAMYeax2GQnv9DjkXXtkBTgXex1RSfz0YMAR49ZuVd4klm9NPSoFUh0XbLqMgvzsDA==", "dependencies": { "@emotion/css": "^11.11.2", "@grafana/data": "^11.1.0", @@ -4283,9 +4283,9 @@ } }, "node_modules/@volkovlabs/components/node_modules/@grafana/scenes": { - "version": "5.20.2", - "resolved": "https://registry.npmjs.org/@grafana/scenes/-/scenes-5.20.2.tgz", - "integrity": "sha512-WTur5FhF4mYTUsi31l+vppQIL/BSf3umPVKa8labv/2udrGbtjXYElqv4dbjRnpUCaaOt780YsVCaRP+6JzV3A==", + "version": "5.20.4", + "resolved": "https://registry.npmjs.org/@grafana/scenes/-/scenes-5.20.4.tgz", + "integrity": "sha512-4ucIjm3jfWFz6U64WNBXQUzZAliCag9um8fMWrkHOKI2cLVJZOeL04ImYW+hropvkIuf8ENYgfCXo2IIKszbzQ==", "dependencies": { "@floating-ui/react": "0.26.16", "@grafana/e2e-selectors": "^11.0.0", diff --git a/package.json b/package.json index 984a1a45..a63d8f4c 100644 --- a/package.json +++ b/package.json @@ -7,7 +7,7 @@ "@grafana/scenes": "5.20.0", "@grafana/ui": "^11.2.0", "@hello-pangea/dnd": "^16.6.0", - "@volkovlabs/components": "^3.4.1", + "@volkovlabs/components": "^3.5.0", "react": "^18.3.1", "react-dom": "^18.3.1", "semver": "^7.6.3", diff --git a/src/__mocks__/@grafana/scenes.ts b/src/__mocks__/@grafana/scenes.ts index 43d43a08..2cfe3189 100644 --- a/src/__mocks__/@grafana/scenes.ts +++ b/src/__mocks__/@grafana/scenes.ts @@ -2,8 +2,12 @@ * Mock @grafana/scenes * mostly prevent IntersectionObserver is not defined */ -jest.mock('@grafana/scenes', () => ({ +const getVariables = jest.fn(); +const getTimeRange = jest.fn(); + +module.exports = { sceneGraph: { - getTimeRange: jest.fn(), + getVariables, + getTimeRange, }, -})); +}; diff --git a/src/__mocks__/@volkovlabs/components.tsx b/src/__mocks__/@volkovlabs/components.tsx index d4afab0d..f0254ca0 100644 --- a/src/__mocks__/@volkovlabs/components.tsx +++ b/src/__mocks__/@volkovlabs/components.tsx @@ -80,7 +80,10 @@ const NumberInput: React.FC = ({ value, onChange, min, max, step, ...rest return ; }; +const useDatasourceRequest = jest.fn(); + module.exports = { ...actual, NumberInput, + useDatasourceRequest, }; diff --git a/src/components/FormPanel/FormPanel.styles.ts b/src/components/FormPanel/FormPanel.styles.ts index d9520483..c9ae016d 100644 --- a/src/components/FormPanel/FormPanel.styles.ts +++ b/src/components/FormPanel/FormPanel.styles.ts @@ -50,5 +50,9 @@ export const getStyles = (theme: GrafanaTheme2) => { confirmTableTd: css` border: 1px solid; `, + errorMessage: css` + white-space: pre-wrap; + margin: ${theme.spacing(1, 0, 0)}; + `, }; }; diff --git a/src/components/FormPanel/FormPanel.test.tsx b/src/components/FormPanel/FormPanel.test.tsx index 7eca2c0e..c910c58a 100644 --- a/src/components/FormPanel/FormPanel.test.tsx +++ b/src/components/FormPanel/FormPanel.test.tsx @@ -3,6 +3,7 @@ import { getAppEvents, RefreshEvent } from '@grafana/runtime'; import { sceneGraph, SceneObject } from '@grafana/scenes'; import { PanelContextProvider } from '@grafana/ui'; import { act, fireEvent, render, screen, waitFor, within } from '@testing-library/react'; +import { useDatasourceRequest } from '@volkovlabs/components'; import React, { ReactElement } from 'react'; import { @@ -17,8 +18,7 @@ import { ResetActionMode, SectionVariant, TEST_IDS, -} from '../../constants'; -import { useDatasourceRequest } from '../../hooks'; +} from '@/constants'; import { ButtonOrientation, ButtonVariant, @@ -28,26 +28,17 @@ import { ModalColumnName, PanelOptions, UpdateEnabledMode, -} from '../../types'; +} from '@/types'; import { getFormElementsSectionSelectors, getFormElementsSelectors, getPanelSelectors, toLocalFormElement, -} from '../../utils'; +} from '@/utils'; + import { FormElements } from '../FormElements'; import { FormPanel } from './FormPanel'; -/** - * Mock @grafana/scenes - * mostly prevent IntersectionObserver is not defined - */ -jest.mock('@grafana/scenes', () => ({ - sceneGraph: { - getTimeRange: jest.fn(), - }, -})); - /** * Mock Form Elements */ @@ -77,11 +68,6 @@ const datasourceRequestMock = jest.fn(() => }) ); -jest.mock('../../hooks', () => ({ - ...jest.requireActual('../../hooks'), - useDatasourceRequest: jest.fn(() => datasourceRequestMock), -})); - /** * Panel */ @@ -499,7 +485,7 @@ describe('Panel', () => { * Check if http error message shown */ expect(selectors.errorMessage()).toBeInTheDocument(); - expect(within(selectors.errorMessage()).getByText('Initial error: message')).toBeInTheDocument(); + expect(selectors.errorMessage()).toHaveTextContent('Initial Error: message'); }); it('Should show error if error while execution', async () => { diff --git a/src/components/FormPanel/FormPanel.tsx b/src/components/FormPanel/FormPanel.tsx index 4a776990..d350ba53 100644 --- a/src/components/FormPanel/FormPanel.tsx +++ b/src/components/FormPanel/FormPanel.tsx @@ -21,7 +21,7 @@ import { toDataQueryResponse, } from '@grafana/runtime'; import { Alert, Button, ConfirmModal, LoadingBar, usePanelContext, useStyles2, useTheme2 } from '@grafana/ui'; -import { useDashboardRefresh } from '@volkovlabs/components'; +import { useDashboardRefresh, useDatasourceRequest } from '@volkovlabs/components'; import { CustomButtonsRow } from 'components/CustomButtonsRow'; import { isEqual } from 'lodash'; import React, { useCallback, useEffect, useMemo, useState } from 'react'; @@ -37,7 +37,7 @@ import { ResetActionMode, TEST_IDS, } from '@/constants'; -import { useDatasourceRequest, useFormElements, useMutableState } from '@/hooks'; +import { useFormElements, useMutableState } from '@/hooks'; import { ButtonVariant, FormElement, @@ -410,7 +410,7 @@ export const FormPanel: React.FC = ({ replaceVariables, payload, }).catch((error: DataQueryError) => { - const errorMessage = `Initial datasource error: ${error.message ? error.message : JSON.stringify(error)}`; + const errorMessage = `Initial Datasource Error: ${error.message ? error.message : JSON.stringify(error)}`; setError(errorMessage); return null; }); @@ -477,7 +477,7 @@ export const FormPanel: React.FC = ({ method: options.initial.method, headers, }).catch((error: Error) => { - const errorMessage = `Initial error: ${error.message ? error.message : error.toString()}`; + const errorMessage = `Initial Error: ${error.message ? error.message : error.toString()}`; setError(errorMessage); return null; }); @@ -618,7 +618,7 @@ export const FormPanel: React.FC = ({ replaceVariables, payload, }).catch((error: DataQueryError) => { - const errorMessage = `Reset datasource error: ${error.message ? error.message : JSON.stringify(error)}`; + const errorMessage = `Reset Datasource Error: ${error.message ? error.message : JSON.stringify(error)}`; setError(errorMessage); return null; }); @@ -709,7 +709,7 @@ export const FormPanel: React.FC = ({ replaceVariables, payload, }).catch((error: DataQueryError) => { - const errorMessage = `Update datasource error: ${error.message ? error.message : JSON.stringify(error)}`; + const errorMessage = `Update Datasource Error: ${error.message ? error.message : JSON.stringify(error)}`; setError(errorMessage); return null; }); @@ -1066,7 +1066,12 @@ export const FormPanel: React.FC = ({ {error && ( - + {error} )} diff --git a/src/hooks/index.ts b/src/hooks/index.ts index c963a120..f4b41831 100644 --- a/src/hooks/index.ts +++ b/src/hooks/index.ts @@ -1,5 +1,4 @@ export * from './useAutoSave'; -export * from './useDatasourceRequest'; export * from './useDatasources'; export * from './useFormElements'; export * from './useMutableState'; diff --git a/src/hooks/useDatasourceRequest.test.ts b/src/hooks/useDatasourceRequest.test.ts deleted file mode 100644 index f4b4705b..00000000 --- a/src/hooks/useDatasourceRequest.test.ts +++ /dev/null @@ -1,129 +0,0 @@ -import { getDataSourceSrv } from '@grafana/runtime'; -import { renderHook } from '@testing-library/react'; -import { Observable } from 'rxjs'; - -import { useDatasourceRequest } from './useDatasourceRequest'; - -/** - * Response - * - * @param response - */ -export const getResponse = (response: any) => - new Observable((subscriber) => { - subscriber.next(response); - subscriber.complete(); - }); - -/** - * Mock @grafana/runtime - */ -jest.mock('@grafana/runtime', () => ({ - getDataSourceSrv: jest.fn(), -})); - -describe('Use Datasource Request', () => { - it('Should run query', async () => { - const dataSourceSrv = { - query: jest.fn(() => - getResponse({ - data: { - message: 'hello', - }, - }) - ), - }; - const getDataSourceSrvMock = jest.fn(() => dataSourceSrv); - - jest.mocked(getDataSourceSrv).mockImplementationOnce( - () => - ({ - get: getDataSourceSrvMock, - }) as any - ); - const { result } = renderHook(() => useDatasourceRequest()); - - const response = await result.current({ - query: { - key1: 'value1', - key2: 'value2', - }, - datasource: 'abc', - replaceVariables: jest.fn((str) => str), - payload: {}, - }); - - /** - * Should get datasource - */ - expect(getDataSourceSrvMock).toHaveBeenCalledWith('abc'); - - /** - * Should pass query - */ - expect(dataSourceSrv.query).toHaveBeenCalledWith({ - targets: [{ key1: 'value1', key2: 'value2' }], - }); - - /** - * Should return result - */ - expect(response).toEqual({ - data: { - message: 'hello', - }, - }); - }); - - it('Should handle promise result query', async () => { - const dataSourceSrv = { - query: jest.fn(() => - Promise.resolve({ - data: { - message: 'hello', - }, - }) - ), - }; - const getDataSourceSrvMock = jest.fn(() => dataSourceSrv); - - jest.mocked(getDataSourceSrv).mockImplementationOnce( - () => - ({ - get: getDataSourceSrvMock, - }) as any - ); - const { result } = renderHook(() => useDatasourceRequest()); - - const response = await result.current({ - query: { - key1: 'value1', - key2: 'value2', - }, - datasource: 'abc', - replaceVariables: jest.fn((str) => str), - payload: {}, - }); - - /** - * Should get datasource - */ - expect(getDataSourceSrvMock).toHaveBeenCalledWith('abc'); - - /** - * Should pass query - */ - expect(dataSourceSrv.query).toHaveBeenCalledWith({ - targets: [{ key1: 'value1', key2: 'value2' }], - }); - - /** - * Should return result - */ - expect(response).toEqual({ - data: { - message: 'hello', - }, - }); - }); -}); diff --git a/src/hooks/useDatasourceRequest.ts b/src/hooks/useDatasourceRequest.ts deleted file mode 100644 index 0b6fb3cf..00000000 --- a/src/hooks/useDatasourceRequest.ts +++ /dev/null @@ -1,56 +0,0 @@ -import { DataQueryResponse, InterpolateFunction } from '@grafana/data'; -import { getDataSourceSrv } from '@grafana/runtime'; -import { useCallback } from 'react'; -import { lastValueFrom } from 'rxjs'; - -/** - * Use Data Source Request - */ -export const useDatasourceRequest = () => { - return useCallback( - async ({ - query, - datasource, - replaceVariables, - payload, - }: { - query: unknown; - datasource: string; - replaceVariables: InterpolateFunction; - payload: unknown; - }): Promise => { - const ds = await getDataSourceSrv().get(datasource); - - /** - * Replace Variables - */ - const target = JSON.parse( - replaceVariables(JSON.stringify(query), { - payload: { - value: payload, - }, - }) - ); - - /** - * Response - */ - const response = ds.query({ - targets: [target], - } as never); - - /** - * Handle as promise - */ - if (response instanceof Promise) { - return await response; - } - - /** - * Handle as observable - */ - return await lastValueFrom(response); - }, - [] - ); -}; diff --git a/src/utils/request.test.ts b/src/utils/request.test.ts index e6bcbb39..c5b56cac 100644 --- a/src/utils/request.test.ts +++ b/src/utils/request.test.ts @@ -1,5 +1,6 @@ -import { FormElementType, PayloadMode } from '../constants'; -import { getPayloadForRequest, toFormData, toJson } from './request'; +import { FormElementType, PayloadMode } from '@/constants'; + +import { DatasourceResponseError, getPayloadForRequest, toFormData, toJson } from './request'; describe('Request Utils', () => { const replaceVariablesMock = (str: string) => str; @@ -179,4 +180,29 @@ describe('Request Utils', () => { expect(result.get('file[1]')).toEqual(payload.file[1]); }); }); + + describe('DatasourceResponseError', () => { + it('Should format error', () => { + const error = new Error('123'); + + expect(new DatasourceResponseError(error, '111')).toEqual({ + error, + message: '123\nRequest: 111', + }); + }); + + it('Should format object', () => { + expect(new DatasourceResponseError({ name: 'hello' }, '111')).toEqual({ + error: { name: 'hello' }, + message: `${JSON.stringify({ name: 'hello' }, null, 2)}\nRequest: 111`, + }); + }); + + it('Should format unknown', () => { + expect(new DatasourceResponseError('hello', '111')).toEqual({ + error: 'hello', + message: 'Unknown Error\nRequest: 111', + }); + }); + }); }); diff --git a/src/utils/request.ts b/src/utils/request.ts index 3d8dc6da..f361200e 100644 --- a/src/utils/request.ts +++ b/src/utils/request.ts @@ -159,3 +159,27 @@ export const toFormData = (payload: object, replaceVariables: InterpolateFunctio return formData; }; + +/** + * Data Source Response Error + */ +export class DatasourceResponseError { + public readonly message: string; + + constructor( + public readonly error: unknown, + target: string + ) { + if (error && typeof error === 'object') { + if ('message' in error && typeof error.message === 'string') { + this.message = error.message; + } else { + this.message = JSON.stringify(error, null, 2); + } + } else { + this.message = 'Unknown Error'; + } + + this.message += `\nRequest: ${target}`; + } +}