Skip to content

Commit

Permalink
[APM] Surface http errors to users (#42160) (#42336)
Browse files Browse the repository at this point in the history
* [APM] Surface http errors to users

* Fix tests

* Add markup to error message

* Remove transaction type from ui filters
  • Loading branch information
sorenlouv authored Jul 31, 2019
1 parent 9f83008 commit 30e403e
Show file tree
Hide file tree
Showing 7 changed files with 96 additions and 35 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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 <LoadingStatePrompt />;
}

if (status === 'failure') {
return <ErrorStatePrompt />;
}

if (historicalDataFound) {
return (
<EuiEmptyPrompt
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,19 +7,20 @@
import { shallow } from 'enzyme';
import React from 'react';
import { NoServicesMessage } from '../NoServicesMessage';
import { FETCH_STATUS } from '../../../../hooks/useFetcher';

describe('NoServicesMessage', () => {
it('should show only a "not found" message when historical data is found', () => {
const wrapper = shallow(
<NoServicesMessage isLoading={false} historicalDataFound={true} />
);
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(
<NoServicesMessage isLoading={false} historicalDataFound={false} />
);
expect(wrapper).toMatchSnapshot();
Object.values(FETCH_STATUS).forEach(status => {
[true, false].forEach(historicalDataFound => {
it(`status: ${status} and historicalDataFound: ${historicalDataFound}`, () => {
const wrapper = shallow(
<NoServicesMessage
status={status}
historicalDataFound={historicalDataFound}
/>
);
expect(wrapper).toMatchSnapshot();
});
});
});
});

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ export function ServiceOverview() {
noItemsMessage={
<NoServicesMessage
historicalDataFound={data.hasHistoricalData}
isLoading={status === 'loading'}
status={status}
/>
}
/>
Expand Down
Original file line number Diff line number Diff line change
@@ -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 (
<EuiEmptyPrompt
title={
<div>
{i18n.translate('xpack.apm.error.prompt.title', {
defaultMessage: `Sorry, an error occured :(`
})}
</div>
}
body={i18n.translate('xpack.apm.error.prompt.body', {
defaultMessage: `Please inspect your browser's developer console for details.`
})}
titleSize="s"
/>
);
}
Original file line number Diff line number Diff line change
Expand Up @@ -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) => {};
Expand Down
41 changes: 34 additions & 7 deletions x-pack/legacy/plugins/apm/public/hooks/useFetcher.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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',
Expand All @@ -16,7 +20,7 @@ export enum FETCH_STATUS {

export function useFetcher<Response>(
fn: () => Promise<Response> | undefined,
effectKey: any[],
fnDeps: any[],
options: { preservePreviousResponse?: boolean } = {}
) {
const { preservePreviousResponse = true } = options;
Expand All @@ -40,11 +44,11 @@ export function useFetcher<Response>(

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;
Expand All @@ -57,7 +61,30 @@ export function useFetcher<Response>(
});
}
} 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: (
<div>
<h5>
{i18n.translate('xpack.apm.fetcher.error.status', {
defaultMessage: `Error`
})}
</h5>
{idx(err.res, r => r.statusText)} ({idx(err.res, r => r.status)}
)
<h5>
{i18n.translate('xpack.apm.fetcher.error.url', {
defaultMessage: `URL`
})}
</h5>
{idx(err.res, r => r.url)}
</div>
)
});
dispatchStatus({ id, isLoading: false });
setResult({
data: undefined,
Expand All @@ -80,15 +107,15 @@ export function useFetcher<Response>(
id,
preservePreviousResponse,
dispatchStatus,
...effectKey
...fnDeps
/* eslint-enable react-hooks/exhaustive-deps */
]);

return useMemo(
() => ({
...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);
}
}),
Expand Down

0 comments on commit 30e403e

Please sign in to comment.