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

[Logs UI] Fix errors during navigation #78319

Merged
merged 9 commits into from
Nov 9, 2020
24 changes: 16 additions & 8 deletions x-pack/plugins/infra/public/containers/logs/log_entries/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
* you may not use this file except in compliance with the Elastic License.
*/
import { useEffect, useState, useReducer, useCallback } from 'react';
import { useMountedState } from 'react-use';
import createContainer from 'constate';
import { pick, throttle } from 'lodash';
import { TimeKey, timeKeyIsBetween } from '../../../../common/time';
Expand Down Expand Up @@ -146,15 +147,20 @@ const useFetchEntriesEffect = (
props: LogEntriesProps
) => {
const { services } = useKibanaContextForPlugin();
const isMounted = useMountedState();
const [prevParams, cachePrevParams] = useState<LogEntriesProps | undefined>();
const [startedStreaming, setStartedStreaming] = useState(false);
const dispatchIfMounted = useCallback((action) => (isMounted() ? dispatch(action) : undefined), [
dispatch,
isMounted,
]);

const runFetchNewEntriesRequest = async (overrides: Partial<LogEntriesProps> = {}) => {
if (!props.startTimestamp || !props.endTimestamp) {
return;
}

dispatch({ type: Action.FetchingNewEntries });
dispatchIfMounted({ type: Action.FetchingNewEntries });

try {
const commonFetchArgs: LogEntriesBaseRequest = {
Expand All @@ -175,13 +181,15 @@ const useFetchEntriesEffect = (
};

const { data: payload } = await fetchLogEntries(fetchArgs, services.http.fetch);
dispatch({ type: Action.ReceiveNewEntries, payload });
dispatchIfMounted({ type: Action.ReceiveNewEntries, payload });

// Move position to the bottom if it's the first load.
// Do it in the next tick to allow the `dispatch` to fire
if (!props.timeKey && payload.bottomCursor) {
setTimeout(() => {
props.jumpToTargetPosition(payload.bottomCursor!);
if (isMounted()) {
props.jumpToTargetPosition(payload.bottomCursor!);
}
});
} else if (
props.timeKey &&
Expand All @@ -192,7 +200,7 @@ const useFetchEntriesEffect = (
props.jumpToTargetPosition(payload.topCursor);
}
} catch (e) {
dispatch({ type: Action.ErrorOnNewEntries });
dispatchIfMounted({ type: Action.ErrorOnNewEntries });
}
};

Expand All @@ -210,7 +218,7 @@ const useFetchEntriesEffect = (
return;
}

dispatch({ type: Action.FetchingMoreEntries });
dispatchIfMounted({ type: Action.FetchingMoreEntries });

try {
const commonFetchArgs: LogEntriesBaseRequest = {
Expand All @@ -232,14 +240,14 @@ const useFetchEntriesEffect = (

const { data: payload } = await fetchLogEntries(fetchArgs, services.http.fetch);

dispatch({
dispatchIfMounted({
type: getEntriesBefore ? Action.ReceiveEntriesBefore : Action.ReceiveEntriesAfter,
payload,
});

return payload.bottomCursor;
} catch (e) {
dispatch({ type: Action.ErrorOnMoreEntries });
dispatchIfMounted({ type: Action.ErrorOnMoreEntries });
}
};

Expand Down Expand Up @@ -322,7 +330,7 @@ const useFetchEntriesEffect = (
after: props.endTimestamp > prevParams.endTimestamp,
};

dispatch({ type: Action.ExpandRange, payload: shouldExpand });
dispatchIfMounted({ type: Action.ExpandRange, payload: shouldExpand });
};

const expandRangeEffectDependencies = [
Expand Down
37 changes: 31 additions & 6 deletions x-pack/plugins/infra/public/utils/use_tracked_promise.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,15 @@

/* eslint-disable max-classes-per-file */

import { DependencyList, useEffect, useMemo, useRef, useState } from 'react';
import { DependencyList, useEffect, useMemo, useRef, useState, useCallback } from 'react';
import { useMountedState } from 'react-use';

interface UseTrackedPromiseArgs<Arguments extends any[], Result> {
createPromise: (...args: Arguments) => Promise<Result>;
onResolve?: (result: Result) => void;
onReject?: (value: unknown) => void;
cancelPreviousOn?: 'creation' | 'settlement' | 'resolution' | 'rejection' | 'never';
triggerOrThrow?: 'always' | 'whenMounted';
}

/**
Expand Down Expand Up @@ -64,16 +66,37 @@ interface UseTrackedPromiseArgs<Arguments extends any[], Result> {
* The last argument is a normal React hook dependency list that indicates
* under which conditions a new reference to the configuration object should be
* used.
*
* The `onResolve`, `onReject` and possible uncatched errors are only triggered
* if the underlying component is mounted. To ensure they always trigger (i.e.
* if the promise is called in a `useLayoutEffect`) use the `triggerOrThrow`
* attribute:
*
* 'whenMounted': (default) they are called only if the component is mounted.
*
* 'always': they always call. The consumer is then responsible of ensuring no
* side effects happen if the underlying component is not mounted.
*/
export const useTrackedPromise = <Arguments extends any[], Result>(
{
createPromise,
onResolve = noOp,
onReject = noOp,
cancelPreviousOn = 'never',
triggerOrThrow = 'whenMounted',
}: UseTrackedPromiseArgs<Arguments, Result>,
dependencies: DependencyList
) => {
const isComponentMounted = useMountedState();
const shouldTriggerOrThrow = useCallback(() => {
switch (triggerOrThrow) {
case 'always':
return true;
case 'whenMounted':
return isComponentMounted();
}
}, [isComponentMounted, triggerOrThrow]);

/**
* If a promise is currently pending, this holds a reference to it and its
* cancellation function.
Expand Down Expand Up @@ -144,7 +167,7 @@ export const useTrackedPromise = <Arguments extends any[], Result>(
(pendingPromise) => pendingPromise.promise !== newPendingPromise.promise
);

if (onResolve) {
if (onResolve && shouldTriggerOrThrow()) {
onResolve(value);
}

Expand Down Expand Up @@ -173,11 +196,13 @@ export const useTrackedPromise = <Arguments extends any[], Result>(
(pendingPromise) => pendingPromise.promise !== newPendingPromise.promise
);

if (onReject) {
onReject(value);
}
if (shouldTriggerOrThrow()) {
if (onReject) {
onReject(value);
}

throw value;
throw value;
}
}
),
};
Expand Down