From c49863e1fd67fd9294cb1793fc5f1f2108a87fd4 Mon Sep 17 00:00:00 2001 From: Melissa Alvarez Date: Tue, 19 Nov 2024 08:57:56 -0700 Subject: [PATCH] [ML] Single Metric Viewer embeddable: fix job refetch on error (#199726) ## Summary This PR ensures that the job fetch is not reattempted continuously when errors are encountered. To test, add a Single Metric Viewer panel to a dashboard, then delete the job the panel depends on. You can then refresh the browser and see the error message about the job not being found. Error panel for generic error and for no known job error: image ### Checklist Delete any items that are not applicable to this PR. - [ ] Any text added follows [EUI's writing guidelines](https://elastic.github.io/eui/#/guidelines/writing), uses sentence case text and includes [i18n support](https://github.com/elastic/kibana/blob/main/packages/kbn-i18n/README.md) - [ ] [Documentation](https://www.elastic.co/guide/en/kibana/master/development-documentation.html) was added for features that require explanation or tutorials - [ ] [Unit or functional tests](https://www.elastic.co/guide/en/kibana/master/development-tests.html) were updated or added to match the most common scenarios - [ ] [Flaky Test Runner](https://ci-stats.kibana.dev/trigger_flaky_test_runner/1) was used on any tests changed - [ ] Any UI touched in this PR is usable by keyboard only (learn more about [keyboard accessibility](https://webaim.org/techniques/keyboard/)) - [ ] Any UI touched in this PR does not create any new axe failures (run axe in browser: [FF](https://addons.mozilla.org/en-US/firefox/addon/axe-devtools/), [Chrome](https://chrome.google.com/webstore/detail/axe-web-accessibility-tes/lhdoppojpmngadmnindnejefpokejbdd?hl=en-US)) - [ ] If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the [docker list](https://github.com/elastic/kibana/blob/main/src/dev/build/tasks/os_packages/docker_generator/resources/base/bin/kibana-docker) - [ ] This renders correctly on smaller devices using a responsive layout. (You can test this [in your browser](https://www.browserstack.com/guide/responsive-testing-on-local-server)) - [ ] This was checked for [cross-browser compatibility](https://www.elastic.co/support/matrix#matrix_browsers) --------- Co-authored-by: Elastic Machine (cherry picked from commit ddf324a9a055bf9ab5a6ba3e9a5f7c36d1e7f9ae) --- .../single_metric_viewer.tsx | 36 ++++++++++++++++--- 1 file changed, 31 insertions(+), 5 deletions(-) diff --git a/x-pack/plugins/ml/public/shared_components/single_metric_viewer/single_metric_viewer.tsx b/x-pack/plugins/ml/public/shared_components/single_metric_viewer/single_metric_viewer.tsx index 9d2ba88493774..39f22e2e988db 100644 --- a/x-pack/plugins/ml/public/shared_components/single_metric_viewer/single_metric_viewer.tsx +++ b/x-pack/plugins/ml/public/shared_components/single_metric_viewer/single_metric_viewer.tsx @@ -20,6 +20,7 @@ import type { MlJob, MlJobStats } from '@elastic/elasticsearch/lib/api/types'; import { DatePickerContextProvider, type DatePickerDependencies } from '@kbn/ml-date-picker'; import type { TimeRangeBounds } from '@kbn/ml-time-buckets'; import usePrevious from 'react-use/lib/usePrevious'; +import { extractErrorProperties } from '@kbn/ml-error-utils'; import { tz } from 'moment'; import { pick, throttle } from 'lodash'; import type { MlDependencies } from '../../application/app'; @@ -43,10 +44,17 @@ interface AppStateZoom { to?: string; } -const errorMessage = i18n.translate('xpack.ml.singleMetricViewerEmbeddable.errorMessage"', { +const basicErrorMessage = i18n.translate('xpack.ml.singleMetricViewerEmbeddable.errorMessage"', { defaultMessage: 'Unable to load the ML single metric viewer data', }); +const jobNotFoundErrorMessage = i18n.translate( + 'xpack.ml.singleMetricViewerEmbeddable.jobNotFoundErrorMessage"', + { + defaultMessage: 'No known job with the selected id', + } +); + export type SingleMetricViewerSharedComponent = FC; /** @@ -72,7 +80,7 @@ export interface SingleMetricViewerProps { */ lastRefresh?: number; onRenderComplete?: () => void; - onError?: (error: Error) => void; + onError?: (error?: Error) => void; onForecastIdChange?: (forecastId: string | undefined) => void; uuid: string; } @@ -112,6 +120,7 @@ const SingleMetricViewerWrapper: FC = ({ const [selectedJobWrapper, setSelectedJobWrapper] = useState< { job: MlJob; stats: MlJobStats } | undefined >(); + const [errorEncountered, setErrorEncountered] = useState(); const isMounted = useMountedState(); const { mlApi, mlTimeSeriesExplorerService, toastNotificationService } = mlServices; @@ -125,6 +134,16 @@ const SingleMetricViewerWrapper: FC = ({ const previousRefresh = usePrevious(lastRefresh ?? 0); + useEffect( + function resetErrorOnJobChange() { + // Calling onError to clear any previous error + setErrorEncountered(undefined); + onError?.(); + }, + // eslint-disable-next-line react-hooks/exhaustive-deps + [selectedJobId] + ); + useEffect( function setUpSelectedJob() { async function fetchSelectedJob() { @@ -136,19 +155,26 @@ const SingleMetricViewerWrapper: FC = ({ ]); setSelectedJobWrapper({ job: jobs[0], stats: jobStats[0] }); } catch (e) { + const error = extractErrorProperties(e); + // Could get 404 because job has been deleted and also avoid infinite refetches on any error + setErrorEncountered(error.statusCode); if (onError) { - onError(new Error(errorMessage)); + onError( + new Error(errorEncountered === 404 ? jobNotFoundErrorMessage : basicErrorMessage) + ); } } } } - if (isMounted() === false) { + if (isMounted() === false || errorEncountered !== undefined) { return; } fetchSelectedJob(); }, - [selectedJobId, mlApi, isMounted, onError] + // eslint-disable-next-line react-hooks/exhaustive-deps + [selectedJobId, isMounted, errorEncountered] ); + // eslint-disable-next-line react-hooks/exhaustive-deps const resizeHandler = useCallback( throttle((e: { width: number; height: number }) => {