From ae62b3817bad4d8ca2299e78f3868a5562944fcc Mon Sep 17 00:00:00 2001 From: Dominik Prokop Date: Mon, 27 Jan 2025 15:54:10 +0100 Subject: [PATCH] Dashboards: Change the way dashboard not found error is handled (#98950) * Get rid of _dashboardLoadFailed * Get rid of dashboardNotFound meta * Update public dashboards tests * Fix DashboardPage tests * DashboardPageProxy tests * DashboardScenePageStateManager test fix * Beterer * Fix merge * Nits * Fix test * remove debugger * Update get folder to throw * translate error title * Update public/app/features/apiserver/types.ts Co-authored-by: Ivan Ortega Alba * Update public/app/features/dashboard/services/DashboardLoaderSrv.ts Co-authored-by: Ivan Ortega Alba * Update public/app/features/dashboard/services/DashboardLoaderSrv.ts Co-authored-by: Ivan Ortega Alba * Update public/app/features/dashboard/services/DashboardLoaderSrv.ts Co-authored-by: Haris Rozajac <58232930+harisrozajac@users.noreply.github.com> * Betterer * Update test cases * More test updates * More translations --------- Co-authored-by: Ivan Ortega Alba Co-authored-by: Haris Rozajac <58232930+harisrozajac@users.noreply.github.com> --- .betterer.results | 9 +- public/app/core/utils/errors.test.ts | 15 ++ public/app/core/utils/errors.ts | 41 ++++ public/app/features/apiserver/types.ts | 1 - .../embedding/EmbeddedDashboard.tsx | 6 +- .../pages/DashboardScenePage.test.tsx | 60 ++++++ .../pages/DashboardScenePage.tsx | 25 +-- .../DashboardScenePageStateManager.test.ts | 38 +++- .../pages/DashboardScenePageStateManager.ts | 57 +++++- .../pages/PublicDashboardScenePage.test.tsx | 47 ++++- .../pages/PublicDashboardScenePage.tsx | 50 +++-- .../dashboard-scene/scene/DashboardScene.tsx | 4 - .../scene/DashboardSceneRenderer.test.tsx | 41 ---- .../scene/DashboardSceneRenderer.tsx | 19 +- .../scene/NavToolbarActions.test.tsx | 8 - .../scene/NavToolbarActions.tsx | 7 +- .../transformSaveModelSchemaV2ToScene.ts | 2 - .../dashboard-scene/solo/SoloPanelPage.tsx | 15 +- .../dashboard-scene/utils/test-utils.ts | 20 ++ .../app/features/dashboard/api/legacy.test.ts | 7 +- public/app/features/dashboard/api/legacy.ts | 9 +- public/app/features/dashboard/api/v0.test.ts | 7 + public/app/features/dashboard/api/v0.ts | 61 +++--- public/app/features/dashboard/api/v2.test.ts | 10 +- public/app/features/dashboard/api/v2.ts | 55 ++++-- .../dashboard/containers/DashboardPage.tsx | 32 ++-- .../containers/DashboardPageError.tsx | 30 +++ .../containers/DashboardPageProxy.test.tsx | 81 +++++++- .../containers/DashboardPageProxy.tsx | 11 +- .../containers/PublicDashboardPage.test.tsx | 47 ++++- .../containers/PublicDashboardPage.tsx | 43 ++++- .../dashboard/services/DashboardLoaderSrv.ts | 175 +++--------------- .../dashboard/services/SnapshotSrv.ts | 10 +- public/app/types/dashboard.ts | 1 - public/locales/en-US/grafana.json | 3 + public/locales/pseudo-LOCALE/grafana.json | 3 + 36 files changed, 684 insertions(+), 366 deletions(-) create mode 100644 public/app/features/dashboard/containers/DashboardPageError.tsx diff --git a/.betterer.results b/.betterer.results index fc3779c97bf..2c72e385439 100644 --- a/.betterer.results +++ b/.betterer.results @@ -3236,9 +3236,6 @@ exports[`better eslint`] = { [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "0"], [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "1"] ], - "public/app/features/dashboard-scene/embedding/EmbeddedDashboard.tsx:5381": [ - [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "0"] - ], "public/app/features/dashboard-scene/embedding/EmbeddedDashboardTestPage.tsx:5381": [ [0, 0, 0, "No untranslated strings. Wrap text with ", "0"] ], @@ -3281,8 +3278,7 @@ exports[`better eslint`] = { "public/app/features/dashboard-scene/pages/DashboardScenePage.tsx:5381": [ [0, 0, 0, "Do not use any type assertions.", "0"], [0, 0, 0, "Do not use any type assertions.", "1"], - [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "2"], - [0, 0, 0, "Unexpected any. Specify a different type.", "3"] + [0, 0, 0, "Unexpected any. Specify a different type.", "2"] ], "public/app/features/dashboard-scene/panel-edit/LibraryVizPanelInfo.tsx:5381": [ [0, 0, 0, "No untranslated strings. Wrap text with ", "0"] @@ -4213,6 +4209,9 @@ exports[`better eslint`] = { [0, 0, 0, "No untranslated strings. Wrap text with ", "0"], [0, 0, 0, "No untranslated strings. Wrap text with ", "1"] ], + "public/app/features/dashboard/containers/PublicDashboardPage.tsx:5381": [ + [0, 0, 0, "Do not use any type assertions.", "0"] + ], "public/app/features/dashboard/containers/SoloPanelPage.tsx:5381": [ [0, 0, 0, "No untranslated strings in text props. Wrap text with or use t()", "0"], [0, 0, 0, "No untranslated strings. Wrap text with ", "1"] diff --git a/public/app/core/utils/errors.test.ts b/public/app/core/utils/errors.test.ts index 262f4d1481d..e01887acb16 100644 --- a/public/app/core/utils/errors.test.ts +++ b/public/app/core/utils/errors.test.ts @@ -1,5 +1,6 @@ import { FetchError } from '@grafana/runtime'; import { getMessageFromError } from 'app/core/utils/errors'; +import { LoadError } from 'app/features/dashboard-scene/pages/DashboardScenePageStateManager'; describe('errors functions', () => { let message: string | null; @@ -53,4 +54,18 @@ describe('errors functions', () => { expect(message).toBe('{"customError":"error string"}'); }); }); + + describe('when getMessageFromError gets an LoadError object', () => { + beforeEach(() => { + const error: LoadError = { + message: 'error string', + status: 500, + }; + message = getMessageFromError(error); + }); + + it('should return the stringified error', () => { + expect(message).toBe('error string'); + }); + }); }); diff --git a/public/app/core/utils/errors.ts b/public/app/core/utils/errors.ts index 42992b6975b..5446e7d475f 100644 --- a/public/app/core/utils/errors.ts +++ b/public/app/core/utils/errors.ts @@ -14,8 +14,49 @@ export function getMessageFromError(err: unknown): string { } else if (err.statusText) { return err.statusText; } + } else if (err.hasOwnProperty('message')) { + // @ts-expect-error + return err.message; } } return JSON.stringify(err); } + +export function getStatusFromError(err: unknown): number | undefined { + if (typeof err === 'string') { + return undefined; + } + + if (err) { + if (err instanceof Error) { + return undefined; + } else if (isFetchError(err)) { + return err.status; + } else if (err.hasOwnProperty('status')) { + // @ts-expect-error + return err.status; + } + } + + return undefined; +} + +export function getMessageIdFromError(err: unknown): string | undefined { + if (typeof err === 'string') { + return undefined; + } + + if (err) { + if (err instanceof Error) { + return undefined; + } else if (isFetchError(err)) { + return err.data?.messageId; + } else if (err.hasOwnProperty('messageId')) { + // @ts-expect-error + return err.messageId; + } + } + + return undefined; +} diff --git a/public/app/features/apiserver/types.ts b/public/app/features/apiserver/types.ts index c335ee9f745..95953d209bc 100644 --- a/public/app/features/apiserver/types.ts +++ b/public/app/features/apiserver/types.ts @@ -82,7 +82,6 @@ type GrafanaClientAnnotations = { [AnnoKeyFolderId]?: number; [AnnoKeyFolderId]?: number; [AnnoKeySavedFromUI]?: string; - [AnnoKeyDashboardNotFound]?: boolean; [AnnoKeyDashboardIsSnapshot]?: boolean; [AnnoKeyDashboardSnapshotOriginalUrl]?: string; diff --git a/public/app/features/dashboard-scene/embedding/EmbeddedDashboard.tsx b/public/app/features/dashboard-scene/embedding/EmbeddedDashboard.tsx index a10c8eae6b9..c416ca0dc91 100644 --- a/public/app/features/dashboard-scene/embedding/EmbeddedDashboard.tsx +++ b/public/app/features/dashboard-scene/embedding/EmbeddedDashboard.tsx @@ -5,6 +5,8 @@ import { GrafanaTheme2, urlUtil } from '@grafana/data'; import { EmbeddedDashboardProps } from '@grafana/runtime'; import { SceneObjectStateChangedEvent, sceneUtils } from '@grafana/scenes'; import { Spinner, Alert, useStyles2 } from '@grafana/ui'; +import { t } from 'app/core/internationalization'; +import { getMessageFromError } from 'app/core/utils/errors'; import { DashboardRoutes } from 'app/types'; import { getDashboardScenePageStateManager } from '../pages/DashboardScenePageStateManager'; @@ -23,8 +25,8 @@ export function EmbeddedDashboard(props: EmbeddedDashboardProps) { if (loadError) { return ( - - {loadError} + + {getMessageFromError(loadError)} ); } diff --git a/public/app/features/dashboard-scene/pages/DashboardScenePage.test.tsx b/public/app/features/dashboard-scene/pages/DashboardScenePage.test.tsx index 2de08282a92..77551f16579 100644 --- a/public/app/features/dashboard-scene/pages/DashboardScenePage.test.tsx +++ b/public/app/features/dashboard-scene/pages/DashboardScenePage.test.tsx @@ -7,6 +7,7 @@ import { getGrafanaContextMock } from 'test/mocks/getGrafanaContextMock'; import { PanelProps } from '@grafana/data'; import { getPanelPlugin } from '@grafana/data/test/__mocks__/pluginMocks'; +import { selectors } from '@grafana/e2e-selectors'; import { LocationServiceProvider, config, @@ -23,6 +24,7 @@ import { DashboardLoaderSrv, setDashboardLoaderSrv } from 'app/features/dashboar import { DASHBOARD_FROM_LS_KEY, DashboardRoutes } from 'app/types'; import { dashboardSceneGraph } from '../utils/dashboardSceneGraph'; +import { setupLoadDashboardMockReject, setupLoadDashboardRuntimeErrorMock } from '../utils/test-utils'; import { DashboardScenePage, Props } from './DashboardScenePage'; import { getDashboardScenePageStateManager } from './DashboardScenePageStateManager'; @@ -298,6 +300,64 @@ describe('DashboardScenePage', () => { await waitFor(() => expect(screen.queryByText('Last 6 hours')).toBeInTheDocument()); }); }); + + describe('errors rendering', () => { + it('should render dashboard not found notice when dashboard... not found', async () => { + setupLoadDashboardMockReject({ + status: 404, + statusText: 'Not Found', + data: { + message: 'Dashboard not found', + }, + config: { + method: 'GET', + url: 'api/dashboards/uid/adfjq9edwm0hsdsa', + retry: 0, + headers: { + 'X-Grafana-Org-Id': 1, + }, + hideFromInspector: true, + }, + isHandled: true, + }); + + setup(); + + expect(await screen.findByTestId(selectors.components.EntityNotFound.container)).toBeInTheDocument(); + }); + it('should render error alert for backend errors', async () => { + setupLoadDashboardMockReject({ + status: 500, + statusText: 'internal server error', + data: { + message: 'Internal server error', + }, + config: { + method: 'GET', + url: 'api/dashboards/uid/adfjq9edwm0hsdsa', + retry: 0, + headers: { + 'X-Grafana-Org-Id': 1, + }, + hideFromInspector: true, + }, + isHandled: true, + }); + + setup(); + + expect(await screen.findByTestId('dashboard-page-error')).toBeInTheDocument(); + expect(await screen.findByTestId('dashboard-page-error')).toHaveTextContent('Internal server error'); + }); + it('should render error alert for runtime errors', async () => { + setupLoadDashboardRuntimeErrorMock(); + + setup(); + + expect(await screen.findByTestId('dashboard-page-error')).toBeInTheDocument(); + expect(await screen.findByTestId('dashboard-page-error')).toHaveTextContent('Runtime error'); + }); + }); }); interface VizOptions { diff --git a/public/app/features/dashboard-scene/pages/DashboardScenePage.tsx b/public/app/features/dashboard-scene/pages/DashboardScenePage.tsx index 23d1f70305b..6ac66d3d312 100644 --- a/public/app/features/dashboard-scene/pages/DashboardScenePage.tsx +++ b/public/app/features/dashboard-scene/pages/DashboardScenePage.tsx @@ -6,10 +6,11 @@ import { usePrevious } from 'react-use'; import { PageLayoutType } from '@grafana/data'; import { config } from '@grafana/runtime'; import { UrlSyncContextProvider } from '@grafana/scenes'; -import { Alert, Box } from '@grafana/ui'; +import { Box } from '@grafana/ui'; import { Page } from 'app/core/components/Page/Page'; import PageLoader from 'app/core/components/PageLoader/PageLoader'; import { GrafanaRouteComponentProps } from 'app/core/navigation/types'; +import { DashboardPageError } from 'app/features/dashboard/containers/DashboardPageError'; import { DashboardPageRouteParams, DashboardPageRouteSearchParams } from 'app/features/dashboard/containers/types'; import { DashboardRoutes } from 'app/types'; @@ -48,17 +49,19 @@ export function DashboardScenePage({ route, queryParams, location }: Props) { }, [stateManager, uid, route.routeName, queryParams.folderUid, routeReloadCounter, slug, type]); if (!dashboard) { + let errorElement; + if (loadError) { + errorElement = ; + } + return ( - - - {isLoading && } - {loadError && ( - - {loadError} - - )} - - + errorElement || ( + + + {isLoading && } + + + ) ); } diff --git a/public/app/features/dashboard-scene/pages/DashboardScenePageStateManager.test.ts b/public/app/features/dashboard-scene/pages/DashboardScenePageStateManager.test.ts index 38bf7a0fcb5..64d850e9047 100644 --- a/public/app/features/dashboard-scene/pages/DashboardScenePageStateManager.test.ts +++ b/public/app/features/dashboard-scene/pages/DashboardScenePageStateManager.test.ts @@ -9,7 +9,7 @@ import { getDashboardSnapshotSrv } from 'app/features/dashboard/services/Snapsho import { DASHBOARD_FROM_LS_KEY, DashboardRoutes } from 'app/types'; import { DashboardScene } from '../scene/DashboardScene'; -import { setupLoadDashboardMock } from '../utils/test-utils'; +import { setupLoadDashboardMock, setupLoadDashboardMockReject } from '../utils/test-utils'; import { DashboardScenePageStateManager, @@ -45,14 +45,34 @@ describe('DashboardScenePageStateManager v1', () => { }); it("should error when the dashboard doesn't exist", async () => { - setupLoadDashboardMock({ dashboard: undefined, meta: {} }); + setupLoadDashboardMockReject({ + status: 404, + statusText: 'Not Found', + data: { + message: 'Dashboard not found', + }, + config: { + method: 'GET', + url: 'api/dashboards/uid/adfjq9edwm0hsdsa', + retry: 0, + headers: { + 'X-Grafana-Org-Id': 1, + }, + hideFromInspector: true, + }, + isHandled: true, + }); const loader = new DashboardScenePageStateManager({}); await loader.loadDashboard({ uid: 'fake-dash', route: DashboardRoutes.Normal }); expect(loader.state.dashboard).toBeUndefined(); expect(loader.state.isLoading).toBe(false); - expect(loader.state.loadError).toBe('Dashboard not found'); + expect(loader.state.loadError).toEqual({ + status: 404, + messageId: undefined, + message: 'Dashboard not found', + }); }); it('should clear current dashboard while loading next', async () => { @@ -128,7 +148,11 @@ describe('DashboardScenePageStateManager v1', () => { await loader.loadDashboard({ uid: '', route: DashboardRoutes.Home }); expect(loader.state.dashboard).toBeUndefined(); - expect(loader.state.loadError).toEqual('Failed to load home dashboard'); + expect(loader.state.loadError).toEqual({ + message: 'Failed to load home dashboard', + messageId: undefined, + status: 500, + }); }); }); @@ -425,7 +449,11 @@ describe('DashboardScenePageStateManager v2', () => { await loader.loadDashboard({ uid: '', route: DashboardRoutes.Home }); expect(loader.state.dashboard).toBeUndefined(); - expect(loader.state.loadError).toEqual('Failed to load home dashboard'); + expect(loader.state.loadError).toEqual({ + message: 'Failed to load home dashboard', + messageId: undefined, + status: 500, + }); }); }); diff --git a/public/app/features/dashboard-scene/pages/DashboardScenePageStateManager.ts b/public/app/features/dashboard-scene/pages/DashboardScenePageStateManager.ts index 80d9666945f..d5dcf3d533b 100644 --- a/public/app/features/dashboard-scene/pages/DashboardScenePageStateManager.ts +++ b/public/app/features/dashboard-scene/pages/DashboardScenePageStateManager.ts @@ -4,7 +4,7 @@ import { locationUtil, UrlQueryMap } from '@grafana/data'; import { config, getBackendSrv, isFetchError, locationService } from '@grafana/runtime'; import { DashboardV2Spec } from '@grafana/schema/dist/esm/schema/dashboard/v2alpha0'; import { StateManagerBase } from 'app/core/services/StateManagerBase'; -import { getMessageFromError } from 'app/core/utils/errors'; +import { getMessageFromError, getMessageIdFromError, getStatusFromError } from 'app/core/utils/errors'; import { startMeasure, stopMeasure } from 'app/core/utils/metrics'; import { AnnoKeyFolder } from 'app/features/apiserver/types'; import { ResponseTransformers } from 'app/features/dashboard/api/ResponseTransformers'; @@ -24,12 +24,18 @@ import { restoreDashboardStateFromLocalStorage } from '../utils/dashboardSession import { updateNavModel } from './utils'; +export interface LoadError { + status?: number; + messageId?: string; + message: string; +} + export interface DashboardScenePageState { dashboard?: DashboardScene; options?: LoadDashboardOptions; panelEditor?: PanelEditor; isLoading?: boolean; - loadError?: string; + loadError?: LoadError; } export const DASHBOARD_CACHE_TTL = 500; @@ -48,6 +54,7 @@ interface DashboardCacheEntry { export interface LoadDashboardOptions { uid: string; route: DashboardRoutes; + type?: string; urlFolderUid?: string; params?: { version: number; @@ -99,7 +106,18 @@ abstract class DashboardScenePageStateManagerBase this.setState({ dashboard: dashboard, isLoading: false }); } catch (err) { - this.setState({ isLoading: false, loadError: String(err) }); + const status = getStatusFromError(err); + const message = getMessageFromError(err); + const messageId = getMessageIdFromError(err); + + this.setState({ + isLoading: false, + loadError: { + status, + message, + messageId, + }, + }); } } @@ -128,8 +146,17 @@ abstract class DashboardScenePageStateManagerBase }); } } catch (err) { - const msg = getMessageFromError(err); - this.setState({ isLoading: false, loadError: msg }); + const status = getStatusFromError(err); + const message = getMessageFromError(err); + const messageId = getMessageIdFromError(err); + this.setState({ + isLoading: false, + loadError: { + status, + message, + messageId, + }, + }); } } @@ -351,7 +378,13 @@ export class DashboardScenePageStateManager extends DashboardScenePageStateManag } if (!rsp?.dashboard) { - this.setState({ isLoading: false, loadError: 'Dashboard not found' }); + this.setState({ + isLoading: false, + loadError: { + status: 404, + message: 'Dashboard not found', + }, + }); return; } @@ -361,8 +394,15 @@ export class DashboardScenePageStateManager extends DashboardScenePageStateManag this.setState({ dashboard: scene, isLoading: false, options }); } catch (err) { - const msg = getMessageFromError(err); - this.setState({ isLoading: false, loadError: msg }); + const status = getStatusFromError(err); + const message = getMessageFromError(err); + this.setState({ + isLoading: false, + loadError: { + message, + status, + }, + }); } } } @@ -429,7 +469,6 @@ export class DashboardScenePageStateManagerV2 extends DashboardScenePageStateMan urlFolderUid, params, }: LoadDashboardOptions): Promise | null> { - // throw new Error('Method not implemented.'); const cacheKey = route === DashboardRoutes.Home ? HOME_DASHBOARD_CACHE_KEY : uid; if (!params) { const cachedDashboard = this.getDashboardFromCache(cacheKey); diff --git a/public/app/features/dashboard-scene/pages/PublicDashboardScenePage.test.tsx b/public/app/features/dashboard-scene/pages/PublicDashboardScenePage.test.tsx index 44688f0aa2e..a8c109d261b 100644 --- a/public/app/features/dashboard-scene/pages/PublicDashboardScenePage.test.tsx +++ b/public/app/features/dashboard-scene/pages/PublicDashboardScenePage.test.tsx @@ -11,7 +11,7 @@ import { Dashboard } from '@grafana/schema'; import { getRouteComponentProps } from 'app/core/navigation/__mocks__/routeProps'; import { DashboardRoutes } from 'app/types/dashboard'; -import { setupLoadDashboardMock } from '../utils/test-utils'; +import { setupLoadDashboardMock, setupLoadDashboardMockReject } from '../utils/test-utils'; import { getDashboardScenePageStateManager } from './DashboardScenePageStateManager'; import { PublicDashboardScenePage, Props as PublicDashboardSceneProps } from './PublicDashboardScenePage'; @@ -192,10 +192,27 @@ describe('given unavailable public dashboard', () => { it('renders public dashboard paused screen when it is paused', async () => { const accessToken = 'paused-pubdash-access-token'; config.publicDashboardAccessToken = accessToken; - setupLoadDashboardMock({ - dashboard: simpleDashboard, - meta: { publicDashboardEnabled: false, dashboardNotFound: false }, + + setupLoadDashboardMockReject({ + status: 403, + statusText: 'Forbidden', + data: { + statusCode: 403, + messageId: 'publicdashboards.notEnabled', + message: 'Dashboard paused', + }, + config: { + method: 'GET', + url: 'api/public/dashboards/ce159fe139fc4d238a7d9c3ae33fb82b', + retry: 0, + headers: { + 'X-Grafana-Org-Id': 1, + 'X-Grafana-Device-Id': 'da48fad0e58ba327fd7d1e6bd17e9c63', + }, + hideFromInspector: true, + }, }); + setup(accessToken); await waitForElementToBeRemoved(screen.getByTestId(publicDashboardSceneSelector.loadingPage)); @@ -208,10 +225,26 @@ describe('given unavailable public dashboard', () => { it('renders public dashboard not available screen when it is deleted', async () => { const accessToken = 'deleted-pubdash-access-token'; config.publicDashboardAccessToken = accessToken; - setupLoadDashboardMock({ - dashboard: simpleDashboard, - meta: { dashboardNotFound: true }, + + setupLoadDashboardMockReject({ + status: 404, + statusText: 'Not Found', + data: { + statusCode: 404, + messageId: 'publicdashboards.notFound', + message: 'Dashboard not found', + }, + config: { + method: 'GET', + url: 'api/public/dashboards/ce159fe139fc4d238a7d9c3ae33fb82b', + retry: 0, + hideFromInspector: true, + headers: { + 'X-Grafana-Device-Id': 'da48fad0e58ba327fd7d1e6bd17e9c63', + }, + }, }); + setup(accessToken); await waitForElementToBeRemoved(screen.getByTestId(publicDashboardSceneSelector.loadingPage)); diff --git a/public/app/features/dashboard-scene/pages/PublicDashboardScenePage.tsx b/public/app/features/dashboard-scene/pages/PublicDashboardScenePage.tsx index c55d4bbcce9..169dd74d82d 100644 --- a/public/app/features/dashboard-scene/pages/PublicDashboardScenePage.tsx +++ b/public/app/features/dashboard-scene/pages/PublicDashboardScenePage.tsx @@ -5,7 +5,7 @@ import { useParams } from 'react-router-dom-v5-compat'; import { GrafanaTheme2, PageLayoutType } from '@grafana/data'; import { selectors as e2eSelectors } from '@grafana/e2e-selectors'; import { SceneComponentProps, UrlSyncContextProvider } from '@grafana/scenes'; -import { Icon, Stack, useStyles2 } from '@grafana/ui'; +import { Alert, Box, Icon, Stack, useStyles2 } from '@grafana/ui'; import { Page } from 'app/core/components/Page/Page'; import PageLoader from 'app/core/components/PageLoader/PageLoader'; import { GrafanaRouteComponentProps } from 'app/core/navigation/types'; @@ -15,11 +15,12 @@ import { PublicDashboardPageRouteParams, PublicDashboardPageRouteSearchParams, } from 'app/features/dashboard/containers/types'; +import { AppNotificationSeverity } from 'app/types'; import { DashboardRoutes } from 'app/types/dashboard'; import { DashboardScene } from '../scene/DashboardScene'; -import { getDashboardScenePageStateManager } from './DashboardScenePageStateManager'; +import { getDashboardScenePageStateManager, LoadError } from './DashboardScenePageStateManager'; const selectors = e2eSelectors.pages.PublicDashboardScene; @@ -42,23 +43,18 @@ export function PublicDashboardScenePage({ route }: Props) { }; }, [stateManager, accessToken, route.routeName]); + if (loadError) { + return ; + } + if (!dashboard) { return ( {isLoading && } - {loadError &&

{loadError}

} ); } - if (dashboard.state.meta.publicDashboardEnabled === false) { - return ; - } - - if (dashboard.state.meta.dashboardNotFound) { - return ; - } - // if no time picker render without url sync if (dashboard.state.controls?.state.hideTimeControls) { return ; @@ -162,3 +158,35 @@ function getStyles(theme: GrafanaTheme2) { }), }; } + +function PublicDashboardScenePageError({ error }: { error: LoadError }) { + const styles = useStyles2(getStyles); + const statusCode = error.status; + const messageId = error.messageId; + const message = error.message; + + const isPublicDashboardPaused = statusCode === 403 && messageId === 'publicdashboards.notEnabled'; + const isPublicDashboardNotFound = statusCode === 404 && messageId === 'publicdashboards.notFound'; + const isDashboardNotFound = statusCode === 404 && messageId === 'publicdashboards.dashboardNotFound'; + + const publicDashboardEnabled = isPublicDashboardNotFound ? undefined : !isPublicDashboardPaused; + const dashboardNotFound = isPublicDashboardNotFound || isDashboardNotFound; + + if (publicDashboardEnabled === false) { + return ; + } + + if (dashboardNotFound) { + return ; + } + + return ( + + + + {message} + + + + ); +} diff --git a/public/app/features/dashboard-scene/scene/DashboardScene.tsx b/public/app/features/dashboard-scene/scene/DashboardScene.tsx index e609e489632..e7f9a6e7f07 100644 --- a/public/app/features/dashboard-scene/scene/DashboardScene.tsx +++ b/public/app/features/dashboard-scene/scene/DashboardScene.tsx @@ -421,10 +421,6 @@ export class DashboardScene extends SceneObjectBase { const { meta, viewPanelScene, editPanel, title, uid } = this.state; const isNew = !Boolean(uid); - if (meta.dashboardNotFound) { - return { text: 'Not found' }; - } - let pageNav: NavModelItem = { text: title, url: getDashboardUrl({ diff --git a/public/app/features/dashboard-scene/scene/DashboardSceneRenderer.test.tsx b/public/app/features/dashboard-scene/scene/DashboardSceneRenderer.test.tsx index d4ec59ed2cf..3f456078a7e 100644 --- a/public/app/features/dashboard-scene/scene/DashboardSceneRenderer.test.tsx +++ b/public/app/features/dashboard-scene/scene/DashboardSceneRenderer.test.tsx @@ -2,7 +2,6 @@ import { screen } from '@testing-library/react'; import { render } from 'test/test-utils'; import { getPanelPlugin } from '@grafana/data/test/__mocks__/pluginMocks'; -import { selectors } from '@grafana/e2e-selectors'; import { config, setPluginImportUtils } from '@grafana/runtime'; import { transformSaveModelToScene } from '../serialization/transformSaveModelToScene'; @@ -34,46 +33,6 @@ jest.mock('@grafana/runtime', () => ({ })); describe('DashboardSceneRenderer', () => { - it('should render Not Found notice when dashboard is not found', async () => { - const scene = transformSaveModelToScene({ - meta: { - isSnapshot: true, - dashboardNotFound: true, - canStar: false, - canDelete: false, - canSave: false, - canEdit: false, - canShare: false, - }, - dashboard: { - title: 'Not found', - uid: 'uid', - schemaVersion: 0, - // Disabling build in annotations to avoid mocking Grafana data source - annotations: { - list: [ - { - builtIn: 1, - datasource: { - type: 'grafana', - uid: '-- Grafana --', - }, - enable: false, - hide: true, - iconColor: 'rgba(0, 211, 255, 1)', - name: 'Annotations & Alerts', - type: 'dashboard', - }, - ], - }, - }, - }); - - render(); - - expect(await screen.findByTestId(selectors.components.EntityNotFound.container)).toBeInTheDocument(); - }); - it('should render angular deprecation notice when dashboard contains angular components', async () => { const noticeText = /This dashboard depends on Angular/i; //enable feature flag angularDeprecationUI diff --git a/public/app/features/dashboard-scene/scene/DashboardSceneRenderer.tsx b/public/app/features/dashboard-scene/scene/DashboardSceneRenderer.tsx index c108170eae9..794cbe2b2d8 100644 --- a/public/app/features/dashboard-scene/scene/DashboardSceneRenderer.tsx +++ b/public/app/features/dashboard-scene/scene/DashboardSceneRenderer.tsx @@ -4,7 +4,6 @@ import { useLocation, useParams } from 'react-router-dom-v5-compat'; import { PageLayoutType } from '@grafana/data'; import { SceneComponentProps } from '@grafana/scenes'; import { Page } from 'app/core/components/Page/Page'; -import { EntityNotFound } from 'app/core/components/PageNotFound/EntityNotFound'; import { getNavModel } from 'app/core/selectors/navModel'; import DashboardEmpty from 'app/features/dashboard/dashgrid/DashboardEmpty'; import { useSelector } from 'app/types'; @@ -16,18 +15,8 @@ import { PanelSearchLayout } from './PanelSearchLayout'; import { DashboardAngularDeprecationBanner } from './angular/DashboardAngularDeprecationBanner'; export function DashboardSceneRenderer({ model }: SceneComponentProps) { - const { - controls, - overlay, - editview, - editPanel, - isEmpty, - meta, - viewPanelScene, - panelSearch, - panelsPerRow, - isEditing, - } = model.useState(); + const { controls, overlay, editview, editPanel, isEmpty, viewPanelScene, panelSearch, panelsPerRow, isEditing } = + model.useState(); const { type } = useParams(); const location = useLocation(); const navIndex = useSelector((state) => state.navIndex); @@ -60,10 +49,6 @@ export function DashboardSceneRenderer({ model }: SceneComponentProps; - } - if (panelSearch || panelsPerRow) { return ; } diff --git a/public/app/features/dashboard-scene/scene/NavToolbarActions.test.tsx b/public/app/features/dashboard-scene/scene/NavToolbarActions.test.tsx index 468bcc5adf9..abca5175425 100644 --- a/public/app/features/dashboard-scene/scene/NavToolbarActions.test.tsx +++ b/public/app/features/dashboard-scene/scene/NavToolbarActions.test.tsx @@ -178,14 +178,6 @@ describe('NavToolbarActions', () => { expect(screen.queryByTestId('button-snapshot')).toBeInTheDocument(); }); - it('should not show link button when is not found dashboard', () => { - setup({ - isSnapshot: true, - dashboardNotFound: true, - }); - - expect(screen.queryByTestId('button-snapshot')).not.toBeInTheDocument(); - }); }); }); diff --git a/public/app/features/dashboard-scene/scene/NavToolbarActions.tsx b/public/app/features/dashboard-scene/scene/NavToolbarActions.tsx index 69666caa007..341052c3567 100644 --- a/public/app/features/dashboard-scene/scene/NavToolbarActions.tsx +++ b/public/app/features/dashboard-scene/scene/NavToolbarActions.tsx @@ -70,7 +70,6 @@ export function ToolbarActions({ dashboard }: Props) { const isViewingPanel = Boolean(viewPanelScene); const isEditedPanelDirty = usePanelEditDirty(editPanel); const isEditingLibraryPanel = editPanel && isLibraryPanel(editPanel.state.panelRef.resolve()); - const isNotFound = Boolean(meta.dashboardNotFound); const isNew = !Boolean(uid); const hasCopiedPanel = store.exists(LS_PANEL_COPY_KEY); @@ -80,10 +79,6 @@ export function ToolbarActions({ dashboard }: Props) { const showScopesSelector = config.featureToggles.scopeFilters && !isEditing; const dashboardNewLayouts = config.featureToggles.dashboardNewLayouts; - if (isNotFound) { - return null; - } - if (!isEditingPanel) { // This adds the precence indicators in enterprise addDynamicActions(toolbarActions, dynamicDashNavActions.left, 'left-actions'); @@ -150,7 +145,7 @@ export function ToolbarActions({ dashboard }: Props) { toolbarActions.push({ group: 'icon-actions', - condition: meta.isSnapshot && !meta.dashboardNotFound && !isEditing, + condition: meta.isSnapshot && !isEditing, render: () => ( ), diff --git a/public/app/features/dashboard-scene/serialization/transformSaveModelSchemaV2ToScene.ts b/public/app/features/dashboard-scene/serialization/transformSaveModelSchemaV2ToScene.ts index 0ed452b3b01..a78771340ff 100644 --- a/public/app/features/dashboard-scene/serialization/transformSaveModelSchemaV2ToScene.ts +++ b/public/app/features/dashboard-scene/serialization/transformSaveModelSchemaV2ToScene.ts @@ -55,7 +55,6 @@ import { import { contextSrv } from 'app/core/core'; import { AnnoKeyCreatedBy, - AnnoKeyDashboardNotFound, AnnoKeyFolder, AnnoKeyUpdatedBy, AnnoKeyUpdatedTimestamp, @@ -148,7 +147,6 @@ export function transformSaveModelSchemaV2ToScene(dto: DashboardWithAccessInfo { @@ -36,6 +37,16 @@ export function SoloPanelPage({ queryParams }: Props) { return ; } + if (loadError) { + return ( + + + {loadError.message} + + + ); + } + if (!dashboard) { return ; } diff --git a/public/app/features/dashboard-scene/utils/test-utils.ts b/public/app/features/dashboard-scene/utils/test-utils.ts index 5415ace6e28..ebcf45d0c5d 100644 --- a/public/app/features/dashboard-scene/utils/test-utils.ts +++ b/public/app/features/dashboard-scene/utils/test-utils.ts @@ -1,4 +1,5 @@ import { VariableRefresh } from '@grafana/data'; +import { FetchError } from '@grafana/runtime'; import { DeepPartial, EmbeddedScene, @@ -31,6 +32,25 @@ export function setupLoadDashboardMock(rsp: DeepPartial, spy?: jes } as unknown as DashboardLoaderSrv); return loadDashboardMock; } +export function setupLoadDashboardMockReject(rsp: DeepPartial, spy?: jest.Mock) { + const loadDashboardMock = (spy || jest.fn()).mockRejectedValue(rsp); + // disabling type checks since this is a test util + // eslint-disable-next-line @typescript-eslint/consistent-type-assertions + setDashboardLoaderSrv({ + loadDashboard: loadDashboardMock, + } as unknown as DashboardLoaderSrv); + return loadDashboardMock; +} + +export function setupLoadDashboardRuntimeErrorMock() { + // disabling type checks since this is a test util + // eslint-disable-next-line @typescript-eslint/consistent-type-assertions + setDashboardLoaderSrv({ + loadDashboard: () => { + throw new Error('Runtime error'); + }, + } as unknown as DashboardLoaderSrv); +} export function mockResizeObserver() { window.ResizeObserver = class ResizeObserver { diff --git a/public/app/features/dashboard/api/legacy.test.ts b/public/app/features/dashboard/api/legacy.test.ts index 54ceb4d1bf4..b62450fd0ea 100644 --- a/public/app/features/dashboard/api/legacy.test.ts +++ b/public/app/features/dashboard/api/legacy.test.ts @@ -40,7 +40,12 @@ jest.mock('app/features/live/dashboard/dashboardWatcher', () => ({ describe('Legacy dashboard API', () => { it('should throw an error if requesting a folder', async () => { const api = new LegacyDashboardAPI(); - expect(async () => await api.getDashboardDTO('folderUid')).rejects.toThrowError('Dashboard not found'); + + await expect(api.getDashboardDTO('folderUid')).rejects.toMatchObject({ + status: 404, + config: { url: `/api/dashboards/uid/folderUid` }, + data: { message: 'Dashboard not found' }, + }); }); it('should return a valid dashboard', async () => { diff --git a/public/app/features/dashboard/api/legacy.ts b/public/app/features/dashboard/api/legacy.ts index 08270fcc27b..1a4ff247307 100644 --- a/public/app/features/dashboard/api/legacy.ts +++ b/public/app/features/dashboard/api/legacy.ts @@ -1,5 +1,5 @@ import { AppEvents, UrlQueryMap } from '@grafana/data'; -import { getBackendSrv } from '@grafana/runtime'; +import { FetchError, getBackendSrv } from '@grafana/runtime'; import { Dashboard } from '@grafana/schema'; import appEvents from 'app/core/app_events'; import { dashboardWatcher } from 'app/features/live/dashboard/dashboardWatcher'; @@ -33,7 +33,12 @@ export class LegacyDashboardAPI implements DashboardAPI if (result.meta.isFolder) { appEvents.emit(AppEvents.alertError, ['Dashboard not found']); - throw new Error('Dashboard not found'); + const fetchError: FetchError = { + status: 404, + config: { url: `/api/dashboards/uid/${uid}` }, + data: { message: 'Dashboard not found' }, + }; + throw fetchError; } return result; diff --git a/public/app/features/dashboard/api/v0.test.ts b/public/app/features/dashboard/api/v0.test.ts index 82d82ecc0d3..67ca83adc8e 100644 --- a/public/app/features/dashboard/api/v0.test.ts +++ b/public/app/features/dashboard/api/v0.test.ts @@ -133,6 +133,13 @@ describe('v0 dashboard API', () => { expect(result.meta.folderUid).toBe('new-folder'); }); + it('throws an error if folder is not found', async () => { + jest.spyOn(backendSrv, 'getFolderByUid').mockRejectedValue({ message: 'folder not found', status: 'not-found' }); + + const api = new K8sDashboardAPI(); + await expect(api.getDashboardDTO('test')).rejects.toThrow('Failed to load folder'); + }); + describe('saveDashboard', () => { beforeEach(() => { locationUtil.initialize({ diff --git a/public/app/features/dashboard/api/v0.ts b/public/app/features/dashboard/api/v0.ts index 811fc46a5b6..e9a72fa52be 100644 --- a/public/app/features/dashboard/api/v0.ts +++ b/public/app/features/dashboard/api/v0.ts @@ -1,6 +1,7 @@ import { locationUtil } from '@grafana/data'; import { Dashboard } from '@grafana/schema'; import { backendSrv } from 'app/core/services/backend_srv'; +import { getMessageFromError, getStatusFromError } from 'app/core/utils/errors'; import kbn from 'app/core/utils/kbn'; import { ScopedResourceClient } from 'app/features/apiserver/client'; import { @@ -91,32 +92,46 @@ export class K8sDashboardAPI implements DashboardAPI { } async getDashboardDTO(uid: string) { - const dash = await this.client.subresource>(uid, 'dto'); + try { + const dash = await this.client.subresource>(uid, 'dto'); - const result: DashboardDTO = { - meta: { - ...dash.access, - isNew: false, - isFolder: false, - uid: dash.metadata.name, - k8s: dash.metadata, - version: parseInt(dash.metadata.resourceVersion, 10), - }, - dashboard: dash.spec, - }; + const result: DashboardDTO = { + meta: { + ...dash.access, + isNew: false, + isFolder: false, + uid: dash.metadata.name, + k8s: dash.metadata, + version: parseInt(dash.metadata.resourceVersion, 10), + }, + dashboard: dash.spec, + }; - if (dash.metadata.annotations?.[AnnoKeyFolder]) { - try { - const folder = await backendSrv.getFolderByUid(dash.metadata.annotations[AnnoKeyFolder]); - result.meta.folderTitle = folder.title; - result.meta.folderUrl = folder.url; - result.meta.folderUid = folder.uid; - result.meta.folderId = folder.id; - } catch (e) { - console.error('Failed to load a folder', e); + if (dash.metadata.annotations?.[AnnoKeyFolder]) { + try { + const folder = await backendSrv.getFolderByUid(dash.metadata.annotations[AnnoKeyFolder]); + result.meta.folderTitle = folder.title; + result.meta.folderUrl = folder.url; + result.meta.folderUid = folder.uid; + result.meta.folderId = folder.id; + } catch (e) { + throw new Error('Failed to load folder'); + } } - } - return result; + return result; + } catch (e) { + const status = getStatusFromError(e); + const message = getMessageFromError(e); + // Hacking around a bug in k8s api server that returns 500 for not found resources + if (message.includes('not found') && status !== 404) { + // @ts-expect-error + e.status = 404; + // @ts-expect-error + e.data.message = 'Dashboard not found'; + } + + throw e; + } } } diff --git a/public/app/features/dashboard/api/v2.test.ts b/public/app/features/dashboard/api/v2.test.ts index 07705c56657..2bfb83ad9a4 100644 --- a/public/app/features/dashboard/api/v2.test.ts +++ b/public/app/features/dashboard/api/v2.test.ts @@ -75,8 +75,7 @@ describe('v2 dashboard API', () => { updatedBy: '', }); - const convertToV1 = false; - const api = new K8sDashboardV2API(convertToV1); + const api = new K8sDashboardV2API(false); // because the API can currently return both DashboardDTO and DashboardWithAccessInfo based on the // parameter convertToV1, we need to cast the result to DashboardWithAccessInfo to be able to // access @@ -86,6 +85,13 @@ describe('v2 dashboard API', () => { expect(result.metadata.annotations![AnnoKeyFolderUrl]).toBe('/folder/url'); expect(result.metadata.annotations![AnnoKeyFolder]).toBe('new-folder'); }); + + it('throws an error if folder is not found', async () => { + jest.spyOn(backendSrv, 'getFolderByUid').mockRejectedValue({ message: 'folder not found', status: 'not-found' }); + + const api = new K8sDashboardV2API(false); + await expect(api.getDashboardDTO('test')).rejects.toThrow('Failed to load folder'); + }); }); describe('v2 dashboard API - Save', () => { diff --git a/public/app/features/dashboard/api/v2.ts b/public/app/features/dashboard/api/v2.ts index a1bc8d83d6a..3a283071091 100644 --- a/public/app/features/dashboard/api/v2.ts +++ b/public/app/features/dashboard/api/v2.ts @@ -1,6 +1,7 @@ import { locationUtil, UrlQueryMap } from '@grafana/data'; import { DashboardV2Spec } from '@grafana/schema/dist/esm/schema/dashboard/v2alpha0'; import { backendSrv } from 'app/core/services/backend_srv'; +import { getMessageFromError, getStatusFromError } from 'app/core/utils/errors'; import kbn from 'app/core/utils/kbn'; import { ScopedResourceClient } from 'app/features/apiserver/client'; import { @@ -37,33 +38,47 @@ export class K8sDashboardV2API } async getDashboardDTO(uid: string, params?: UrlQueryMap) { - const dashboard = await this.client.subresource>(uid, 'dto'); + try { + const dashboard = await this.client.subresource>(uid, 'dto'); - let result: DashboardWithAccessInfo | DashboardDTO | undefined; + let result: DashboardWithAccessInfo | DashboardDTO | undefined; - // TODO: For dev purposes only, the conversion should and will happen in the API. This is just to stub v2 api responses. - result = ResponseTransformers.ensureV2Response(dashboard); + // TODO: For dev purposes only, the conversion should and will happen in the API. This is just to stub v2 api responses. + result = ResponseTransformers.ensureV2Response(dashboard); - // load folder info if available - if (result.metadata.annotations && result.metadata.annotations[AnnoKeyFolder]) { - try { - const folder = await backendSrv.getFolderByUid(result.metadata.annotations[AnnoKeyFolder]); - result.metadata.annotations[AnnoKeyFolderTitle] = folder.title; - result.metadata.annotations[AnnoKeyFolderUrl] = folder.url; - result.metadata.annotations[AnnoKeyFolderId] = folder.id; - } catch (e) { - console.error('Failed to load a folder', e); + // load folder info if available + if (result.metadata.annotations && result.metadata.annotations[AnnoKeyFolder]) { + try { + const folder = await backendSrv.getFolderByUid(result.metadata.annotations[AnnoKeyFolder]); + result.metadata.annotations[AnnoKeyFolderTitle] = folder.title; + result.metadata.annotations[AnnoKeyFolderUrl] = folder.url; + result.metadata.annotations[AnnoKeyFolderId] = folder.id; + } catch (e) { + throw new Error('Failed to load folder'); + } } - } - // Depending on the ui components readiness, we might need to convert the response to v1 - if (this.convertToV1) { - // Always return V1 format - result = ResponseTransformers.ensureV1Response(result); + // Depending on the ui components readiness, we might need to convert the response to v1 + if (this.convertToV1) { + // Always return V1 format + result = ResponseTransformers.ensureV1Response(result); + return result; + } + // return the v2 response return result; + } catch (e) { + const status = getStatusFromError(e); + const message = getMessageFromError(e); + // Hacking around a bug in k8s api server that returns 500 for not found resources + if (message.includes('not found') && status !== 404) { + // @ts-expect-error + e.status = 404; + // @ts-expect-error + e.data.message = 'Dashboard not found'; + } + + throw e; } - // return the v2 response - return result; } deleteDashboard(uid: string, showSuccessAlert: boolean): Promise { diff --git a/public/app/features/dashboard/containers/DashboardPage.tsx b/public/app/features/dashboard/containers/DashboardPage.tsx index 2f825a3ffad..2f9a6f874fb 100644 --- a/public/app/features/dashboard/containers/DashboardPage.tsx +++ b/public/app/features/dashboard/containers/DashboardPage.tsx @@ -9,7 +9,6 @@ import { Themeable2, withTheme2 } from '@grafana/ui'; import { notifyApp } from 'app/core/actions'; import { ScrollRefElement } from 'app/core/components/NativeScrollbar'; import { Page } from 'app/core/components/Page/Page'; -import { EntityNotFound } from 'app/core/components/PageNotFound/EntityNotFound'; import { GrafanaContext, GrafanaContextType } from 'app/core/context/GrafanaContext'; import { createErrorNotification } from 'app/core/copy/appNotification'; import { getKioskMode } from 'app/core/navigation/kiosk'; @@ -26,7 +25,6 @@ import { PanelEditEnteredEvent, PanelEditExitedEvent } from 'app/types/events'; import { cancelVariables, templateVarsChangedInUrl } from '../../variables/state/actions'; import { findTemplateVarChanges } from '../../variables/utils'; import { DashNav } from '../components/DashNav'; -import { DashboardFailed } from '../components/DashboardLoading/DashboardFailed'; import { DashboardLoading } from '../components/DashboardLoading/DashboardLoading'; import { DashboardPrompt } from '../components/DashboardPrompt/DashboardPrompt'; import { DashboardSettings } from '../components/DashboardSettings'; @@ -41,6 +39,7 @@ import { explicitlyControlledMigrationPanels, autoMigrateAngular } from '../stat import { cleanUpDashboardAndVariables } from '../state/actions'; import { initDashboard } from '../state/initDashboard'; +import { DashboardPageError } from './DashboardPageError'; import { DashboardPageRouteParams, DashboardPageRouteSearchParams } from './types'; import 'react-grid-layout/css/styles.css'; @@ -355,7 +354,8 @@ export class UnthemedDashboardPage extends PureComponent { }; render() { - const { dashboard, initError, queryParams, theme } = this.props; + const { dashboard, initError, queryParams, theme, params } = this.props; + const { editPanel, viewPanel, pageNav, sectionNav } = this.state; const kioskMode = getKioskMode(this.props.queryParams); const styles = getStyles(theme); @@ -367,21 +367,13 @@ export class UnthemedDashboardPage extends PureComponent { const inspectPanel = this.getInspectPanel(); const showSubMenu = !editPanel && !kioskMode && !this.props.queryParams.editview && dashboard.isSubMenuVisible(); - const showToolbar = kioskMode !== KioskMode.Full && !queryParams.editview; + const showToolbar = kioskMode !== KioskMode.Full && !queryParams.editview && !initError; const pageClassName = cx({ [styles.fullScreenPanel]: Boolean(viewPanel), 'page-hidden': Boolean(queryParams.editview || editPanel), }); - if (dashboard.meta.dashboardNotFound) { - return ( - - - - ); - } - const migrationFeatureFlags = new Set([ 'autoMigrateOldPanels', 'autoMigrateGraphPanel', @@ -448,7 +440,7 @@ export class UnthemedDashboardPage extends PureComponent { )} - {initError && } + {initError && } {showSubMenu && (
@@ -463,12 +455,14 @@ export class UnthemedDashboardPage extends PureComponent { /> )} {showDashboardMigrationNotice && } - + {!initError && ( + + )} {inspectPanel && } {queryParams.shareView && ( diff --git a/public/app/features/dashboard/containers/DashboardPageError.tsx b/public/app/features/dashboard/containers/DashboardPageError.tsx new file mode 100644 index 00000000000..1e856a7ca26 --- /dev/null +++ b/public/app/features/dashboard/containers/DashboardPageError.tsx @@ -0,0 +1,30 @@ +import { PageLayoutType } from '@grafana/data'; +import { Alert, Box } from '@grafana/ui'; +import { Page } from 'app/core/components/Page/Page'; +import { EntityNotFound } from 'app/core/components/PageNotFound/EntityNotFound'; +import { t } from 'app/core/internationalization'; +import { getMessageFromError, getStatusFromError } from 'app/core/utils/errors'; + +export function DashboardPageError({ error, type }: { error: unknown; type?: string }) { + const status = getStatusFromError(error); + const message = getMessageFromError(error); + const entity = type === 'snapshot' ? 'Snapshot' : 'Dashboard'; + + return ( + + + {status === 404 ? ( + + ) : ( + + {message} + + )} + + + ); +} diff --git a/public/app/features/dashboard/containers/DashboardPageProxy.test.tsx b/public/app/features/dashboard/containers/DashboardPageProxy.test.tsx index 23eac600472..8fe6faeac8d 100644 --- a/public/app/features/dashboard/containers/DashboardPageProxy.test.tsx +++ b/public/app/features/dashboard/containers/DashboardPageProxy.test.tsx @@ -3,13 +3,20 @@ import { useParams } from 'react-router-dom-v5-compat'; import { Props } from 'react-virtualized-auto-sizer'; import { render } from 'test/test-utils'; +import { selectors } from '@grafana/e2e-selectors'; import { config, locationService } from '@grafana/runtime'; import { HOME_DASHBOARD_CACHE_KEY, getDashboardScenePageStateManager, } from 'app/features/dashboard-scene/pages/DashboardScenePageStateManager'; +import { + setupLoadDashboardMockReject, + setupLoadDashboardRuntimeErrorMock, +} from 'app/features/dashboard-scene/utils/test-utils'; import { DashboardDTO, DashboardRoutes } from 'app/types'; +import { DashboardLoaderSrv, setDashboardLoaderSrv } from '../services/DashboardLoaderSrv'; + import DashboardPageProxy, { DashboardPageProxyProps } from './DashboardPageProxy'; const dashMock: DashboardDTO = { @@ -63,6 +70,11 @@ jest.mock('@grafana/runtime', () => ({ get: jest.fn().mockResolvedValue({}), }), useChromeHeaderHeight: jest.fn(), + getBackendSrv: () => { + return { + get: jest.fn().mockResolvedValue({ dashboard: {}, meta: { url: '' } }), + }; + }, })); jest.mock('react-virtualized-auto-sizer', () => { @@ -75,11 +87,11 @@ jest.mock('react-virtualized-auto-sizer', () => { }); }); -jest.mock('app/features/dashboard/api/dashboard_api', () => ({ - getDashboardAPI: () => ({ - getDashboardDTO: jest.fn().mockResolvedValue(dashMock), - }), -})); +setDashboardLoaderSrv({ + loadDashboard: jest.fn().mockResolvedValue(dashMock), + // disabling type checks since this is a test util + // eslint-disable-next-line @typescript-eslint/consistent-type-assertions +} as unknown as DashboardLoaderSrv); jest.mock('react-router-dom-v5-compat', () => ({ ...jest.requireActual('react-router-dom-v5-compat'), @@ -209,4 +221,63 @@ describe('DashboardPageProxy', () => { }); }); }); + + describe('errors rendering', () => { + it('should render dashboard not found notice when dashboard... not found', async () => { + setupLoadDashboardMockReject({ + status: 404, + statusText: 'Not Found', + data: { + message: 'Dashboard not found', + }, + config: { + method: 'GET', + url: 'api/dashboards/uid/adfjq9edwm0hsdsa', + retry: 0, + headers: { + 'X-Grafana-Org-Id': 1, + }, + hideFromInspector: true, + }, + isHandled: true, + }); + + setup({ route: { routeName: DashboardRoutes.Normal, component: () => null, path: '/' }, uid: 'abc' }); + + expect(await screen.findByTestId(selectors.components.EntityNotFound.container)).toBeInTheDocument(); + }); + + it('should render error alert for backend errors', async () => { + setupLoadDashboardMockReject({ + status: 500, + statusText: 'internal server error', + data: { + message: 'Internal server error', + }, + config: { + method: 'GET', + url: 'api/dashboards/uid/adfjq9edwm0hsdsa', + retry: 0, + headers: { + 'X-Grafana-Org-Id': 1, + }, + hideFromInspector: true, + }, + isHandled: true, + }); + + setup({ route: { routeName: DashboardRoutes.Normal, component: () => null, path: '/' }, uid: 'abc' }); + + expect(await screen.findByTestId('dashboard-page-error')).toBeInTheDocument(); + expect(await screen.findByTestId('dashboard-page-error')).toHaveTextContent('Internal server error'); + }); + it('should render error alert for runtime errors', async () => { + setupLoadDashboardRuntimeErrorMock(); + + setup({ route: { routeName: DashboardRoutes.Normal, component: () => null, path: '/' }, uid: 'abc' }); + + expect(await screen.findByTestId('dashboard-page-error')).toBeInTheDocument(); + expect(await screen.findByTestId('dashboard-page-error')).toHaveTextContent('Runtime error'); + }); + }); }); diff --git a/public/app/features/dashboard/containers/DashboardPageProxy.tsx b/public/app/features/dashboard/containers/DashboardPageProxy.tsx index 39b13676fa9..52d91d5d956 100644 --- a/public/app/features/dashboard/containers/DashboardPageProxy.tsx +++ b/public/app/features/dashboard/containers/DashboardPageProxy.tsx @@ -8,6 +8,7 @@ import { getDashboardScenePageStateManager } from 'app/features/dashboard-scene/ import { DashboardRoutes } from 'app/types'; import DashboardPage, { DashboardPageParams } from './DashboardPage'; +import { DashboardPageError } from './DashboardPageError'; import { DashboardPageRouteParams, DashboardPageRouteSearchParams } from './types'; export type DashboardPageProxyProps = Omit< @@ -46,9 +47,17 @@ function DashboardPageProxy(props: DashboardPageProxyProps) { return null; } - return stateManager.fetchDashboard({ route: props.route.routeName as DashboardRoutes, uid: params.uid ?? '' }); + return stateManager.fetchDashboard({ + route: props.route.routeName as DashboardRoutes, + uid: params.uid ?? '', + type: params.type, + }); }, [params.uid, props.route.routeName]); + if (dashboard.error) { + return ; + } + if (dashboard.loading) { return null; } diff --git a/public/app/features/dashboard/containers/PublicDashboardPage.test.tsx b/public/app/features/dashboard/containers/PublicDashboardPage.test.tsx index 0de0da4426b..915dd50d69a 100644 --- a/public/app/features/dashboard/containers/PublicDashboardPage.test.tsx +++ b/public/app/features/dashboard/containers/PublicDashboardPage.test.tsx @@ -260,7 +260,29 @@ describe('PublicDashboardPage', () => { setup(undefined, { dashboard: { ...dashboardBase, - getModel: () => getTestDashboard(undefined, { publicDashboardEnabled: false, dashboardNotFound: false }), + initError: { + message: 'Failed to fetch dashboard', + error: { + status: 403, + statusText: 'Forbidden', + data: { + statusCode: 403, + messageId: 'publicdashboards.notEnabled', + message: 'Dashboard paused', + }, + config: { + method: 'GET', + url: 'api/public/dashboards/4615c835a4e441f09c94fb1b073e6d2e', + retry: 0, + headers: { + 'X-Grafana-Org-Id': 1, + 'X-Grafana-Device-Id': 'da48fad0e58ba327fd7d1e6bd17e9c63', + }, + hideFromInspector: true, + }, + }, + }, + getModel: () => getTestDashboard(undefined, { publicDashboardEnabled: false }), }, }); @@ -277,7 +299,28 @@ describe('PublicDashboardPage', () => { setup(undefined, { dashboard: { ...dashboardBase, - getModel: () => getTestDashboard(undefined, { dashboardNotFound: true }), + initError: { + message: 'Failed to fetch dashboard', + error: { + status: 404, + statusText: 'Not Found', + data: { + statusCode: 404, + messageId: 'publicdashboards.notFound', + message: 'Dashboard not found', + }, + config: { + method: 'GET', + url: 'api/public/dashboards/ce159fe139fc4d238a7d9c3ae33fb82b', + retry: 0, + hideFromInspector: true, + headers: { + 'X-Grafana-Device-Id': 'da48fad0e58ba327fd7d1e6bd17e9c63', + }, + }, + }, + }, + getModel: () => getTestDashboard(undefined, {}), }, }); diff --git a/public/app/features/dashboard/containers/PublicDashboardPage.tsx b/public/app/features/dashboard/containers/PublicDashboardPage.tsx index 1588d464e1c..4c3957431d8 100644 --- a/public/app/features/dashboard/containers/PublicDashboardPage.tsx +++ b/public/app/features/dashboard/containers/PublicDashboardPage.tsx @@ -14,7 +14,7 @@ import { PublicDashboardPageRouteSearchParams, } from 'app/features/dashboard/containers/types'; import { updateTimeZoneForSession } from 'app/features/profile/state/reducers'; -import { useSelector, useDispatch } from 'app/types'; +import { useSelector, useDispatch, DashboardInitError } from 'app/types'; import { DashNavTimeControls } from '../components/DashNav/DashNavTimeControls'; import { DashboardFailed } from '../components/DashboardLoading/DashboardFailed'; @@ -64,6 +64,7 @@ const PublicDashboardPage = (props: Props) => { const prevProps = usePrevious({ ...props, location }); const styles = useStyles2(getStyles); const dashboardState = useSelector((store) => store.dashboard); + const loadError = dashboardState.initError; const dashboard = dashboardState.getModel(); useEffect(() => { @@ -96,18 +97,14 @@ const PublicDashboardPage = (props: Props) => { } }, [prevProps, location.search, props.queryParams, dashboard?.timepicker.hidden, accessToken]); + if (loadError) { + return ; + } + if (!dashboard) { return ; } - if (dashboard.meta.publicDashboardEnabled === false) { - return ; - } - - if (dashboard.meta.dashboardNotFound) { - return ; - } - return ( @@ -134,3 +131,31 @@ const getStyles = (theme: GrafanaTheme2) => ({ }); export default PublicDashboardPage; + +function PublicDashboardPageError({ error }: { error: DashboardInitError }) { + let statusCode: number | undefined; + let messageId: string | undefined; + + if (typeof error.error === 'object' && error.error !== null && 'data' in error.error) { + const typedError = error.error as { data: { statusCode: number; messageId: string } }; + statusCode = typedError.data.statusCode; + messageId = typedError.data.messageId; + } + + const isPublicDashboardPaused = statusCode === 403 && messageId === 'publicdashboards.notEnabled'; + const isPublicDashboardNotFound = statusCode === 404 && messageId === 'publicdashboards.notFound'; + const isDashboardNotFound = statusCode === 404 && messageId === 'publicdashboards.dashboardNotFound'; + + const publicDashboardEnabled = isPublicDashboardNotFound ? undefined : !isPublicDashboardPaused; + const dashboardNotFound = isPublicDashboardNotFound || isDashboardNotFound; + + if (publicDashboardEnabled === false) { + return ; + } + + if (dashboardNotFound) { + return ; + } + + return ; +} diff --git a/public/app/features/dashboard/services/DashboardLoaderSrv.ts b/public/app/features/dashboard/services/DashboardLoaderSrv.ts index 771446bfac1..f0ddc2a32ab 100644 --- a/public/app/features/dashboard/services/DashboardLoaderSrv.ts +++ b/public/app/features/dashboard/services/DashboardLoaderSrv.ts @@ -4,12 +4,10 @@ import moment from 'moment'; // eslint-disable-line no-restricted-imports import { AppEvents, dateMath, UrlQueryMap, UrlQueryValue } from '@grafana/data'; import { getBackendSrv, isFetchError, locationService } from '@grafana/runtime'; -import { DashboardV2Spec, defaultDashboardV2Spec } from '@grafana/schema/dist/esm/schema/dashboard/v2alpha0'; +import { DashboardV2Spec } from '@grafana/schema/dist/esm/schema/dashboard/v2alpha0'; import { backendSrv } from 'app/core/services/backend_srv'; import impressionSrv from 'app/core/services/impression_srv'; -import { getMessageFromError } from 'app/core/utils/errors'; import kbn from 'app/core/utils/kbn'; -import { AnnoKeyDashboardIsSnapshot, AnnoKeyDashboardNotFound } from 'app/features/apiserver/types'; import { getDashboardScenePageStateManager } from 'app/features/dashboard-scene/pages/DashboardScenePageStateManager'; import { getDatasourceSrv } from 'app/features/plugins/datasource_srv'; import { DashboardDTO } from 'app/types'; @@ -23,7 +21,6 @@ import { getDashboardSrv } from './DashboardSrv'; import { getDashboardSnapshotSrv } from './SnapshotSrv'; interface DashboardLoaderSrvLike { - _dashboardLoadFailed(title: string, snapshot?: boolean): T; loadDashboard( type: UrlQueryValue, slug: string | undefined, @@ -33,7 +30,6 @@ interface DashboardLoaderSrvLike { } abstract class DashboardLoaderSrvBase implements DashboardLoaderSrvLike { - abstract _dashboardLoadFailed(title: string, snapshot?: boolean): T; abstract loadDashboard( type: UrlQueryValue, slug: string | undefined, @@ -66,7 +62,7 @@ abstract class DashboardLoaderSrvBase implements DashboardLoaderSrvLike { 'Script Error', 'Please make sure it exists and returns a valid dashboard', ]); - return this._dashboardLoadFailed('Scripted dashboard'); + throw err; } ); } @@ -116,22 +112,6 @@ abstract class DashboardLoaderSrvBase implements DashboardLoaderSrvLike { } export class DashboardLoaderSrv extends DashboardLoaderSrvBase { - _dashboardLoadFailed(title: string, snapshot?: boolean) { - snapshot = snapshot || false; - return { - meta: { - canStar: false, - isSnapshot: snapshot, - canDelete: false, - canSave: false, - canEdit: false, - canShare: false, - dashboardNotFound: true, - }, - dashboard: { title, uid: title, schemaVersion: 0 }, - }; - } - loadDashboard( type: UrlQueryValue, slug: string | undefined, @@ -146,38 +126,11 @@ export class DashboardLoaderSrv extends DashboardLoaderSrvBase { // needed for the old architecture // in scenes this is handled through loadSnapshot method } else if (type === 'snapshot' && slug) { - promise = getDashboardSnapshotSrv() - .getSnapshot(slug) - .catch(() => { - return this._dashboardLoadFailed('Snapshot not found', true); - }); + promise = getDashboardSnapshotSrv().getSnapshot(slug); } else if (type === 'public' && uid) { - promise = backendSrv - .getPublicDashboardByUid(uid) - .then((result) => { - return result; - }) - .catch((e) => { - const isPublicDashboardPaused = - e.data.statusCode === 403 && e.data.messageId === 'publicdashboards.notEnabled'; - const isPublicDashboardNotFound = - e.data.statusCode === 404 && e.data.messageId === 'publicdashboards.notFound'; - const isDashboardNotFound = - e.data.statusCode === 404 && e.data.messageId === 'publicdashboards.dashboardNotFound'; - - const dashboardModel = this._dashboardLoadFailed( - isPublicDashboardPaused ? 'Public Dashboard paused' : 'Public Dashboard Not found', - true - ); - return { - ...dashboardModel, - meta: { - ...dashboardModel.meta, - publicDashboardEnabled: isPublicDashboardNotFound ? undefined : !isPublicDashboardPaused, - dashboardNotFound: isPublicDashboardNotFound || isDashboardNotFound, - }, - }; - }); + promise = backendSrv.getPublicDashboardByUid(uid).then((result) => { + return result; + }); } else if (uid) { if (!params) { const cachedDashboard = stateManager.getDashboardFromCache(uid); @@ -188,26 +141,23 @@ export class DashboardLoaderSrv extends DashboardLoaderSrvBase { promise = getDashboardAPI() .getDashboardDTO(uid, params) - .then((result) => { - if (result.meta.isFolder) { - appEvents.emit(AppEvents.alertError, ['Dashboard not found']); - throw new Error('Dashboard not found'); + .catch((e) => { + console.error('Failed to load dashboard', e); + if (isFetchError(e)) { + e.isHandled = true; + if (e.status === 404) { + appEvents.emit(AppEvents.alertError, ['Dashboard not found']); + } } - return result; - }) - .catch(() => { - const dash = this._dashboardLoadFailed('Not found', true); - dash.dashboard.uid = ''; - return dash; + + throw e; }); } else { throw new Error('Dashboard uid or slug required'); } promise.then((result: DashboardDTO) => { - if (result.meta.dashboardNotFound !== true) { - impressionSrv.addDashboardImpression(result.dashboard.uid); - } + impressionSrv.addDashboardImpression(result.dashboard.uid); return result; }); @@ -216,16 +166,10 @@ export class DashboardLoaderSrv extends DashboardLoaderSrvBase { } loadSnapshot(slug: string): Promise { - const promise = getDashboardSnapshotSrv() - .getSnapshot(slug) - .catch(() => { - return this._dashboardLoadFailed('Snapshot not found', true); - }); + const promise = getDashboardSnapshotSrv().getSnapshot(slug); promise.then((result: DashboardDTO) => { - if (result.meta.dashboardNotFound !== true) { - impressionSrv.addDashboardImpression(result.dashboard.uid); - } + impressionSrv.addDashboardImpression(result.dashboard.uid); return result; }); @@ -235,36 +179,6 @@ export class DashboardLoaderSrv extends DashboardLoaderSrvBase { } export class DashboardLoaderSrvV2 extends DashboardLoaderSrvBase> { - _dashboardLoadFailed(title: string, snapshot?: boolean) { - const dashboard: DashboardWithAccessInfo = { - kind: 'DashboardWithAccessInfo', - spec: { - ...defaultDashboardV2Spec(), - title, - }, - access: { - canSave: false, - canEdit: false, - canAdmin: false, - canStar: false, - canShare: false, - canDelete: false, - }, - apiVersion: 'v2alpha1', - metadata: { - creationTimestamp: '', - name: title, - namespace: '', - resourceVersion: '', - annotations: { - [AnnoKeyDashboardNotFound]: true, - [AnnoKeyDashboardIsSnapshot]: Boolean(snapshot), - }, - }, - }; - return dashboard; - } - loadDashboard( type: UrlQueryValue, slug: string | undefined, @@ -277,34 +191,9 @@ export class DashboardLoaderSrvV2 extends DashboardLoaderSrvBase ResponseTransformers.ensureV2Response(r)); } else if (type === 'public' && uid) { - promise = backendSrv - .getPublicDashboardByUid(uid) - .then((result) => { - return ResponseTransformers.ensureV2Response(result); - }) - .catch((e) => { - const isPublicDashboardPaused = - e.data.statusCode === 403 && e.data.messageId === 'publicdashboards.notEnabled'; - // const isPublicDashboardNotFound = - // e.data.statusCode === 404 && e.data.messageId === 'publicdashboards.notFound'; - // const isDashboardNotFound = - // e.data.statusCode === 404 && e.data.messageId === 'publicdashboards.dashboardNotFound'; - const dashboardModel = this._dashboardLoadFailed( - isPublicDashboardPaused ? 'Public Dashboard paused' : 'Public Dashboard Not found', - true - ); - - return dashboardModel; - // TODO[schema v2]: - // return { - // ...dashboardModel, - // meta: { - // ...dashboardModel.meta, - // publicDashboardEnabled: isPublicDashboardNotFound ? undefined : !isPublicDashboardPaused, - // dashboardNotFound: isPublicDashboardNotFound || isDashboardNotFound, - // }, - // }; - }); + promise = backendSrv.getPublicDashboardByUid(uid).then((result) => { + return ResponseTransformers.ensureV2Response(result); + }); } else if (uid) { if (!params) { const cachedDashboard = stateManager.getDashboardFromCache(uid); @@ -319,21 +208,19 @@ export class DashboardLoaderSrvV2 extends DashboardLoaderSrvBase) => { - if (result.metadata.annotations?.[AnnoKeyDashboardNotFound] !== true) { - impressionSrv.addDashboardImpression(result.metadata.name); - } - + impressionSrv.addDashboardImpression(result.metadata.name); return result; }); @@ -343,16 +230,10 @@ export class DashboardLoaderSrvV2 extends DashboardLoaderSrvBase> { const promise = getDashboardSnapshotSrv() .getSnapshot(slug) - .then((r) => ResponseTransformers.ensureV2Response(r)) - .catch((e) => { - const msg = getMessageFromError(e); - throw new Error(`Failed to load snapshot: ${msg}`); - }); + .then((r) => ResponseTransformers.ensureV2Response(r)); promise.then((result: DashboardWithAccessInfo) => { - if (result.metadata.annotations?.[AnnoKeyDashboardNotFound] !== true) { - impressionSrv.addDashboardImpression(result.metadata.name); - } + impressionSrv.addDashboardImpression(result.metadata.name); return result; }); diff --git a/public/app/features/dashboard/services/SnapshotSrv.ts b/public/app/features/dashboard/services/SnapshotSrv.ts index e3e0e9db637..5467542ab2b 100644 --- a/public/app/features/dashboard/services/SnapshotSrv.ts +++ b/public/app/features/dashboard/services/SnapshotSrv.ts @@ -47,9 +47,13 @@ const legacyDashboardSnapshotSrv: DashboardSnapshotSrv = { getSharingOptions: () => getBackendSrv().get('/api/snapshot/shared-options'), deleteSnapshot: (key: string) => getBackendSrv().delete('/api/snapshots/' + key), getSnapshot: async (key: string) => { - const dto = await getBackendSrv().get('/api/snapshots/' + key); - dto.meta.canShare = false; - return dto; + try { + const dto = await getBackendSrv().get('/api/snapshots/' + key); + dto.meta.canShare = false; + return dto; + } catch (e) { + throw e; + } }, }; diff --git a/public/app/types/dashboard.ts b/public/app/types/dashboard.ts index d79e7a5a382..fb4eb3fd962 100644 --- a/public/app/types/dashboard.ts +++ b/public/app/types/dashboard.ts @@ -66,7 +66,6 @@ export interface DashboardMeta { hasUnsavedFolderChange?: boolean; annotationsPermissions?: AnnotationsPermissions; publicDashboardEnabled?: boolean; - dashboardNotFound?: boolean; isEmbedded?: boolean; isNew?: boolean; version?: number; diff --git a/public/locales/en-US/grafana.json b/public/locales/en-US/grafana.json index 6181dc87999..979924df896 100644 --- a/public/locales/en-US/grafana.json +++ b/public/locales/en-US/grafana.json @@ -893,6 +893,9 @@ "import-a-dashboard-header": "Import a dashboard", "import-dashboard-button": "Import dashboard" }, + "errors": { + "failed-to-load": "Failed to load dashboard" + }, "inspect": { "data-tab": "Data", "error-tab": "Error", diff --git a/public/locales/pseudo-LOCALE/grafana.json b/public/locales/pseudo-LOCALE/grafana.json index e5fff220ff6..d155653d0ed 100644 --- a/public/locales/pseudo-LOCALE/grafana.json +++ b/public/locales/pseudo-LOCALE/grafana.json @@ -893,6 +893,9 @@ "import-a-dashboard-header": "Ĩmpőřŧ ä đäşĥþőäřđ", "import-dashboard-button": "Ĩmpőřŧ đäşĥþőäřđ" }, + "errors": { + "failed-to-load": "Fäįľęđ ŧő ľőäđ đäşĥþőäřđ" + }, "inspect": { "data-tab": "Đäŧä", "error-tab": "Ēřřőř",