From 30e403ebd421809aed4a27b1f32cc30608ff5a71 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?S=C3=B8ren=20Louv-Jansen?= Date: Wed, 31 Jul 2019 14:28:47 +0200 Subject: [PATCH] [APM] Surface http errors to users (#42160) (#42336) * [APM] Surface http errors to users * Fix tests * Add markup to error message * Remove transaction type from ui filters --- .../app/ServiceOverview/NoServicesMessage.tsx | 12 ++++-- .../__test__/NoServicesMessage.test.tsx | 25 +++++------ .../NoServicesMessage.test.tsx.snap | 12 +++++- .../components/app/ServiceOverview/index.tsx | 2 +- .../components/shared/ErrorStatePrompt.tsx | 27 ++++++++++++ .../public/context/UrlParamsContext/index.tsx | 12 +----- .../plugins/apm/public/hooks/useFetcher.tsx | 41 +++++++++++++++---- 7 files changed, 96 insertions(+), 35 deletions(-) create mode 100644 x-pack/legacy/plugins/apm/public/components/shared/ErrorStatePrompt.tsx diff --git a/x-pack/legacy/plugins/apm/public/components/app/ServiceOverview/NoServicesMessage.tsx b/x-pack/legacy/plugins/apm/public/components/app/ServiceOverview/NoServicesMessage.tsx index 0b87590576342..de058d6ef973a 100644 --- a/x-pack/legacy/plugins/apm/public/components/app/ServiceOverview/NoServicesMessage.tsx +++ b/x-pack/legacy/plugins/apm/public/components/app/ServiceOverview/NoServicesMessage.tsx @@ -10,18 +10,24 @@ import React from 'react'; import { KibanaLink } from '../../shared/Links/KibanaLink'; import { SetupInstructionsLink } from '../../shared/Links/SetupInstructionsLink'; import { LoadingStatePrompt } from '../../shared/LoadingStatePrompt'; +import { FETCH_STATUS } from '../../../hooks/useFetcher'; +import { ErrorStatePrompt } from '../../shared/ErrorStatePrompt'; interface Props { // any data submitted from APM agents found (not just in the given time range) historicalDataFound: boolean; - isLoading: boolean; + status: FETCH_STATUS | undefined; } -export function NoServicesMessage({ historicalDataFound, isLoading }: Props) { - if (isLoading) { +export function NoServicesMessage({ historicalDataFound, status }: Props) { + if (status === 'loading') { return ; } + if (status === 'failure') { + return ; + } + if (historicalDataFound) { return ( { - it('should show only a "not found" message when historical data is found', () => { - const wrapper = shallow( - - ); - expect(wrapper).toMatchSnapshot(); - }); - - it('should show a "no services installed" message, a link to the set up instructions page, a message about upgrading APM server, and a link to the upgrade assistant when NO historical data is found', () => { - const wrapper = shallow( - - ); - expect(wrapper).toMatchSnapshot(); + Object.values(FETCH_STATUS).forEach(status => { + [true, false].forEach(historicalDataFound => { + it(`status: ${status} and historicalDataFound: ${historicalDataFound}`, () => { + const wrapper = shallow( + + ); + expect(wrapper).toMatchSnapshot(); + }); + }); }); }); diff --git a/x-pack/legacy/plugins/apm/public/components/app/ServiceOverview/__test__/__snapshots__/NoServicesMessage.test.tsx.snap b/x-pack/legacy/plugins/apm/public/components/app/ServiceOverview/__test__/__snapshots__/NoServicesMessage.test.tsx.snap index 02dbe22aeb19d..42e2b59da2e10 100644 --- a/x-pack/legacy/plugins/apm/public/components/app/ServiceOverview/__test__/__snapshots__/NoServicesMessage.test.tsx.snap +++ b/x-pack/legacy/plugins/apm/public/components/app/ServiceOverview/__test__/__snapshots__/NoServicesMessage.test.tsx.snap @@ -1,6 +1,14 @@ // Jest Snapshot v1, https://goo.gl/fbAQLP -exports[`NoServicesMessage should show a "no services installed" message, a link to the set up instructions page, a message about upgrading APM server, and a link to the upgrade assistant when NO historical data is found 1`] = ` +exports[`NoServicesMessage status: failure and historicalDataFound: false 1`] = ``; + +exports[`NoServicesMessage status: failure and historicalDataFound: true 1`] = ``; + +exports[`NoServicesMessage status: loading and historicalDataFound: false 1`] = ``; + +exports[`NoServicesMessage status: loading and historicalDataFound: true 1`] = ``; + +exports[`NoServicesMessage status: success and historicalDataFound: false 1`] = ` `; -exports[`NoServicesMessage should show only a "not found" message when historical data is found 1`] = ` +exports[`NoServicesMessage status: success and historicalDataFound: true 1`] = ` } /> diff --git a/x-pack/legacy/plugins/apm/public/components/shared/ErrorStatePrompt.tsx b/x-pack/legacy/plugins/apm/public/components/shared/ErrorStatePrompt.tsx new file mode 100644 index 0000000000000..7154d006fcaae --- /dev/null +++ b/x-pack/legacy/plugins/apm/public/components/shared/ErrorStatePrompt.tsx @@ -0,0 +1,27 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ + +import React from 'react'; +import { i18n } from '@kbn/i18n'; +import { EuiEmptyPrompt } from '@elastic/eui'; + +export function ErrorStatePrompt() { + return ( + + {i18n.translate('xpack.apm.error.prompt.title', { + defaultMessage: `Sorry, an error occured :(` + })} + + } + body={i18n.translate('xpack.apm.error.prompt.body', { + defaultMessage: `Please inspect your browser's developer console for details.` + })} + titleSize="s" + /> + ); +} diff --git a/x-pack/legacy/plugins/apm/public/context/UrlParamsContext/index.tsx b/x-pack/legacy/plugins/apm/public/context/UrlParamsContext/index.tsx index 9252abb599f30..8cf326d6cfd6a 100644 --- a/x-pack/legacy/plugins/apm/public/context/UrlParamsContext/index.tsx +++ b/x-pack/legacy/plugins/apm/public/context/UrlParamsContext/index.tsx @@ -23,16 +23,8 @@ interface TimeRange { rangeTo: string; } -function useUiFilters({ - kuery, - environment, - transactionType -}: IUrlParams): UIFilters { - return useMemo(() => ({ kuery, environment, transactionType }), [ - kuery, - environment, - transactionType - ]); +function useUiFilters({ kuery, environment }: IUrlParams): UIFilters { + return useMemo(() => ({ kuery, environment }), [kuery, environment]); } const defaultRefresh = (time: TimeRange) => {}; diff --git a/x-pack/legacy/plugins/apm/public/hooks/useFetcher.tsx b/x-pack/legacy/plugins/apm/public/hooks/useFetcher.tsx index a540c261def48..6927d3359cd84 100644 --- a/x-pack/legacy/plugins/apm/public/hooks/useFetcher.tsx +++ b/x-pack/legacy/plugins/apm/public/hooks/useFetcher.tsx @@ -4,9 +4,13 @@ * you may not use this file except in compliance with the Elastic License. */ -import { useContext, useEffect, useState, useMemo } from 'react'; +import React, { useContext, useEffect, useState, useMemo } from 'react'; +import { toastNotifications } from 'ui/notify'; +import { idx } from '@kbn/elastic-idx/target'; +import { i18n } from '@kbn/i18n'; import { LoadingIndicatorContext } from '../context/LoadingIndicatorContext'; import { useComponentId } from './useComponentId'; +import { KFetchError } from '../../../../../../src/legacy/ui/public/kfetch/kfetch_error'; export enum FETCH_STATUS { LOADING = 'loading', @@ -16,7 +20,7 @@ export enum FETCH_STATUS { export function useFetcher( fn: () => Promise | undefined, - effectKey: any[], + fnDeps: any[], options: { preservePreviousResponse?: boolean } = {} ) { const { preservePreviousResponse = true } = options; @@ -40,11 +44,11 @@ export function useFetcher( dispatchStatus({ id, isLoading: true }); - setResult({ - data: preservePreviousResponse ? result.data : undefined, // preserve data from previous state while loading next state + setResult(prevResult => ({ + data: preservePreviousResponse ? prevResult.data : undefined, // preserve data from previous state while loading next state status: FETCH_STATUS.LOADING, error: undefined - }); + })); try { const data = await promise; @@ -57,7 +61,30 @@ export function useFetcher( }); } } catch (e) { + const err = e as KFetchError; if (!didCancel) { + toastNotifications.addWarning({ + title: i18n.translate('xpack.apm.fetcher.error.title', { + defaultMessage: `Error while fetching resource` + }), + text: ( +
+
+ {i18n.translate('xpack.apm.fetcher.error.status', { + defaultMessage: `Error` + })} +
+ {idx(err.res, r => r.statusText)} ({idx(err.res, r => r.status)} + ) +
+ {i18n.translate('xpack.apm.fetcher.error.url', { + defaultMessage: `URL` + })} +
+ {idx(err.res, r => r.url)} +
+ ) + }); dispatchStatus({ id, isLoading: false }); setResult({ data: undefined, @@ -80,7 +107,7 @@ export function useFetcher( id, preservePreviousResponse, dispatchStatus, - ...effectKey + ...fnDeps /* eslint-enable react-hooks/exhaustive-deps */ ]); @@ -88,7 +115,7 @@ export function useFetcher( () => ({ ...result, refresh: () => { - // this will invalidate the effectKey and will result in a new request + // this will invalidate the deps to `useEffect` and will result in a new request setCounter(count => count + 1); } }),