Skip to content

Commit 6110ef8

Browse files
Alejandro Fernández Gómezkibanamachine
andauthored
[Logs UI] Fix errors during navigation (#78319)
Co-authored-by: Kibana Machine <42973632+kibanamachine@users.noreply.github.com>
1 parent 202dec7 commit 6110ef8

File tree

2 files changed

+47
-14
lines changed

2 files changed

+47
-14
lines changed

x-pack/plugins/infra/public/containers/logs/log_entries/index.ts

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
* you may not use this file except in compliance with the Elastic License.
55
*/
66
import { useEffect, useState, useReducer, useCallback } from 'react';
7+
import { useMountedState } from 'react-use';
78
import createContainer from 'constate';
89
import { pick, throttle } from 'lodash';
910
import { TimeKey, timeKeyIsBetween } from '../../../../common/time';
@@ -146,15 +147,20 @@ const useFetchEntriesEffect = (
146147
props: LogEntriesProps
147148
) => {
148149
const { services } = useKibanaContextForPlugin();
150+
const isMounted = useMountedState();
149151
const [prevParams, cachePrevParams] = useState<LogEntriesProps | undefined>();
150152
const [startedStreaming, setStartedStreaming] = useState(false);
153+
const dispatchIfMounted = useCallback((action) => (isMounted() ? dispatch(action) : undefined), [
154+
dispatch,
155+
isMounted,
156+
]);
151157

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

157-
dispatch({ type: Action.FetchingNewEntries });
163+
dispatchIfMounted({ type: Action.FetchingNewEntries });
158164

159165
try {
160166
const commonFetchArgs: LogEntriesBaseRequest = {
@@ -175,13 +181,15 @@ const useFetchEntriesEffect = (
175181
};
176182

177183
const { data: payload } = await fetchLogEntries(fetchArgs, services.http.fetch);
178-
dispatch({ type: Action.ReceiveNewEntries, payload });
184+
dispatchIfMounted({ type: Action.ReceiveNewEntries, payload });
179185

180186
// Move position to the bottom if it's the first load.
181187
// Do it in the next tick to allow the `dispatch` to fire
182188
if (!props.timeKey && payload.bottomCursor) {
183189
setTimeout(() => {
184-
props.jumpToTargetPosition(payload.bottomCursor!);
190+
if (isMounted()) {
191+
props.jumpToTargetPosition(payload.bottomCursor!);
192+
}
185193
});
186194
} else if (
187195
props.timeKey &&
@@ -192,7 +200,7 @@ const useFetchEntriesEffect = (
192200
props.jumpToTargetPosition(payload.topCursor);
193201
}
194202
} catch (e) {
195-
dispatch({ type: Action.ErrorOnNewEntries });
203+
dispatchIfMounted({ type: Action.ErrorOnNewEntries });
196204
}
197205
};
198206

@@ -210,7 +218,7 @@ const useFetchEntriesEffect = (
210218
return;
211219
}
212220

213-
dispatch({ type: Action.FetchingMoreEntries });
221+
dispatchIfMounted({ type: Action.FetchingMoreEntries });
214222

215223
try {
216224
const commonFetchArgs: LogEntriesBaseRequest = {
@@ -232,14 +240,14 @@ const useFetchEntriesEffect = (
232240

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

235-
dispatch({
243+
dispatchIfMounted({
236244
type: getEntriesBefore ? Action.ReceiveEntriesBefore : Action.ReceiveEntriesAfter,
237245
payload,
238246
});
239247

240248
return payload.bottomCursor;
241249
} catch (e) {
242-
dispatch({ type: Action.ErrorOnMoreEntries });
250+
dispatchIfMounted({ type: Action.ErrorOnMoreEntries });
243251
}
244252
};
245253

@@ -322,7 +330,7 @@ const useFetchEntriesEffect = (
322330
after: props.endTimestamp > prevParams.endTimestamp,
323331
};
324332

325-
dispatch({ type: Action.ExpandRange, payload: shouldExpand });
333+
dispatchIfMounted({ type: Action.ExpandRange, payload: shouldExpand });
326334
};
327335

328336
const expandRangeEffectDependencies = [

x-pack/plugins/infra/public/utils/use_tracked_promise.ts

Lines changed: 31 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -6,13 +6,15 @@
66

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

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

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

1820
/**
@@ -64,16 +66,37 @@ interface UseTrackedPromiseArgs<Arguments extends any[], Result> {
6466
* The last argument is a normal React hook dependency list that indicates
6567
* under which conditions a new reference to the configuration object should be
6668
* used.
69+
*
70+
* The `onResolve`, `onReject` and possible uncatched errors are only triggered
71+
* if the underlying component is mounted. To ensure they always trigger (i.e.
72+
* if the promise is called in a `useLayoutEffect`) use the `triggerOrThrow`
73+
* attribute:
74+
*
75+
* 'whenMounted': (default) they are called only if the component is mounted.
76+
*
77+
* 'always': they always call. The consumer is then responsible of ensuring no
78+
* side effects happen if the underlying component is not mounted.
6779
*/
6880
export const useTrackedPromise = <Arguments extends any[], Result>(
6981
{
7082
createPromise,
7183
onResolve = noOp,
7284
onReject = noOp,
7385
cancelPreviousOn = 'never',
86+
triggerOrThrow = 'whenMounted',
7487
}: UseTrackedPromiseArgs<Arguments, Result>,
7588
dependencies: DependencyList
7689
) => {
90+
const isComponentMounted = useMountedState();
91+
const shouldTriggerOrThrow = useCallback(() => {
92+
switch (triggerOrThrow) {
93+
case 'always':
94+
return true;
95+
case 'whenMounted':
96+
return isComponentMounted();
97+
}
98+
}, [isComponentMounted, triggerOrThrow]);
99+
77100
/**
78101
* If a promise is currently pending, this holds a reference to it and its
79102
* cancellation function.
@@ -144,7 +167,7 @@ export const useTrackedPromise = <Arguments extends any[], Result>(
144167
(pendingPromise) => pendingPromise.promise !== newPendingPromise.promise
145168
);
146169

147-
if (onResolve) {
170+
if (onResolve && shouldTriggerOrThrow()) {
148171
onResolve(value);
149172
}
150173

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

176-
if (onReject) {
177-
onReject(value);
178-
}
199+
if (shouldTriggerOrThrow()) {
200+
if (onReject) {
201+
onReject(value);
202+
}
179203

180-
throw value;
204+
throw value;
205+
}
181206
}
182207
),
183208
};

0 commit comments

Comments
 (0)