Skip to content

Commit

Permalink
[ML] Single Metric Viewer embeddable: fix job refetch on error (#199726)
Browse files Browse the repository at this point in the history
## 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: 
<img width="1308" alt="image"
src="https://github.com/user-attachments/assets/6d6d6972-ac1a-479c-a5dc-8d770026ca4e">



### 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 <[email protected]>
  • Loading branch information
alvarezmelissa87 and elasticmachine authored Nov 19, 2024
1 parent ee49986 commit ddf324a
Showing 1 changed file with 31 additions and 5 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand All @@ -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<SingleMetricViewerProps>;

/**
Expand All @@ -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;
}
Expand Down Expand Up @@ -112,6 +120,7 @@ const SingleMetricViewerWrapper: FC<SingleMetricViewerPropsWithDeps> = ({
const [selectedJobWrapper, setSelectedJobWrapper] = useState<
{ job: MlJob; stats: MlJobStats } | undefined
>();
const [errorEncountered, setErrorEncountered] = useState<number | undefined>();

const isMounted = useMountedState();
const { mlApi, mlTimeSeriesExplorerService, toastNotificationService } = mlServices;
Expand All @@ -125,6 +134,16 @@ const SingleMetricViewerWrapper: FC<SingleMetricViewerPropsWithDeps> = ({

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() {
Expand All @@ -136,19 +155,26 @@ const SingleMetricViewerWrapper: FC<SingleMetricViewerPropsWithDeps> = ({
]);
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 }) => {
Expand Down

0 comments on commit ddf324a

Please sign in to comment.