Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[7.x] [APM] Surface http errors to users (#42160) #42336

Merged
merged 1 commit into from
Jul 31, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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