[Synthetics] Apply faster backoff for non-test-now screenshot resul…

…ts (#152874)

## Summary

Resolves #152754.

It was noted in #152754 that we're getting some negative side effects
from the exponential backoff we added to the getter for journey
screenshots. When a screenshot is legitimately not available for an
older journey (as opposed to not yet fully indexed for a fresh journey),
the backoff will cause the screenshot never to render.

## Testing

Follow a flow similar to what's illustrated in the GIF below.

1. Create a monitor with some screenshots that show up.
2. Delete the screenshot data stream. A good way is to use the Index
Management > Data Streams view in Kibana.
3. Refresh your monitor's detail page when new photos are coming in. It
should give up on finding the images very fast, and as they aren't
there, you should see the missing image placeholder.
4. Start creating a new monitor. Make it have multiple steps.
5. Ensure that you aren't seeing any "infinite" loading states across
the screenshot UIs.


![20230308110103](https://user-images.githubusercontent.com/18429259/223766486-eaca4295-9e4f-4490-8717-c1b915382ec7.gif)

x-pack/plugins/synthetics/public/apps/synthetics/components/common/monitor_test_result/browser_steps_list.tsx

@@ -155,6 +155,7 @@ export const BrowserStepsList = ({
           allStepsLoaded={!loading}
           retryFetchOnRevisit={true}
           size={screenshotImageSize}
+          testNowMode={testNowMode}
           timestamp={timestamp}
         />
       ),

x-pack/plugins/synthetics/public/apps/synthetics/components/common/monitor_test_result/use_retrieve_step_image.ts

@@ -6,15 +6,13 @@
  */
 
 import { useEffect, useMemo, useState } from 'react';
-import { useFetcher } from '@kbn/observability-plugin/public';
-import moment from 'moment';
 import {
   ScreenshotImageBlob,
   ScreenshotRefImageData,
   isScreenshotRef,
 } from '../../../../../../common/runtime_types';
 import { useComposeImageFromRef } from '../../../hooks/use_composite_image';
-import { getJourneyScreenshot } from '../../../state';
+import { BackoffOptions, getJourneyScreenshot } from '../../../state';
 
 type ImageResponse = ScreenshotImageBlob | ScreenshotRefImageData | null;
 interface ImageDataResult {
@@ -34,6 +32,7 @@ export const useRetrieveStepImage = ({
   stepStatus,
   hasIntersected,
   imgPath,
+  testNowMode,
   retryFetchOnRevisit,
 }: {
   timestamp?: string;
@@ -42,6 +41,8 @@ export const useRetrieveStepImage = ({
   stepStatus?: string;
   hasIntersected: boolean;
 
+  testNowMode?: boolean;
+
   /**
    * Whether to retry screenshot image fetch on revisit (when intersection change triggers).
    * Will only re-fetch if an image fetch wasn't successful in previous attempts.
@@ -53,39 +54,54 @@ export const useRetrieveStepImage = ({
   const [imgState, setImgState] = useState<ImageDataResult>({});
   const skippedStep = stepStatus === 'skipped';
 
-  const dataResult = useGetStepScreenshotUrls(checkGroup, imgPath, imgState);
-  const isImageUrlAvailable = dataResult?.[imgPath]?.url ?? false;
-
-  useFetcher(() => {
-    const is5MinutesOld = timestamp
-      ? moment(timestamp).isBefore(moment().subtract(5, 'minutes'))
-      : false;
-    const retrieveAttemptedBefore = (imgState[imgPath]?.attempts ?? 0) > 0;
-    const shouldRetry = retryFetchOnRevisit || !retrieveAttemptedBefore;
+  const imageResult = useGetStepScreenshotUrls(checkGroup, imgPath, imgState);
+  const isImageUrlAvailable = imageResult?.[imgPath]?.url ?? false;
+
+  const shouldFetch = useMemo(() => {
+    const shouldRetry = retryFetchOnRevisit || !(imgState[imgPath]?.attempts ?? 0 > 0);
+    return !skippedStep && hasIntersected && !isImageUrlAvailable && shouldRetry && checkGroup;
+  }, [
+    checkGroup,
+    hasIntersected,
+    imgPath,
+    imgState,
+    isImageUrlAvailable,
+    retryFetchOnRevisit,
+    skippedStep,
+  ]);
 
-    if (!skippedStep && hasIntersected && !isImageUrlAvailable && shouldRetry && checkGroup) {
-      setImgState((prevState) => {
-        return getUpdatedState({ prevState, imgPath, increment: true, loading: true });
-      });
-      return getJourneyScreenshot(imgPath, !is5MinutesOld)
-        .then((data) => {
-          setImgState((prevState) => {
-            return getUpdatedState({ prevState, imgPath, increment: false, data, loading: false });
-          });
-
-          return data;
-        })
-        .catch(() => {
+  useEffect(() => {
+    async function run() {
+      if (shouldFetch) {
+        setImgState((prevState) => {
+          return getUpdatedState({ prevState, imgPath, increment: true, loading: true });
+        });
+        const backoffOptions: Partial<BackoffOptions> | undefined = !testNowMode
+          ? { shouldBackoff: false }
+          : undefined;
+        try {
+          getJourneyScreenshot(imgPath, backoffOptions).then((data) =>
+            setImgState((prevState) =>
+              getUpdatedState({
+                prevState,
+                imgPath,
+                increment: false,
+                data,
+                loading: false,
+              })
+            )
+          );
+        } catch (e: unknown) {
           setImgState((prevState) => {
             return getUpdatedState({ prevState, imgPath, increment: false, loading: false });
           });
-        });
-    } else {
-      return new Promise<ImageResponse>((resolve) => resolve(null));
+        }
+      }
     }
-  }, [skippedStep, hasIntersected, imgPath, retryFetchOnRevisit, timestamp]);
+    run();
+  }, [imgPath, shouldFetch, testNowMode]);
 
-  return dataResult;
+  return imageResult;
 };
 
 /**

x-pack/plugins/synthetics/public/apps/synthetics/components/common/screenshot/empty_thumbnail.tsx

@@ -11,8 +11,8 @@ import {
   useEuiTheme,
   useEuiBackgroundColor,
   EuiIcon,
-  EuiLoadingContent,
   EuiText,
+  EuiSkeletonRectangle,
 } from '@elastic/eui';
 
 import {
@@ -63,14 +63,23 @@ export const EmptyThumbnail = ({
         ...(borderRadius ? { borderRadius } : {}),
       }}
     >
-      {isLoading ? (
-        <EuiLoadingContent
+      {isLoading && animateLoading ? (
+        <EuiSkeletonRectangle
+          data-test-subj="stepScreenshotPlaceholderLoading"
+          isLoading={isLoading}
+          height={height}
+          width={width}
+        >
+          {/* `children` is required by this component, even though we'll replace it. */}
+          <span />
+        </EuiSkeletonRectangle>
+      ) : // when we're loading and `animateLoading` is false we simply render an un-animated div to fill the space
+      isLoading ? (
+        <div
           data-test-subj="stepScreenshotPlaceholderLoading"
           style={{
             width: '100%',
-            transform: `scale(1, ${animateLoading ? '100' : '0'})`, // To create a skeleton loading effect when animateLoading is true
           }}
-          lines={1}
         />
       ) : (
         <div

x-pack/plugins/synthetics/public/apps/synthetics/components/common/screenshot/journey_screenshot_dialog.tsx

@@ -123,7 +123,7 @@ export const JourneyScreenshotDialog = ({
           <ScreenshotImage
             label={stepCountLabel}
             imgSrc={imgSrc}
-            isLoading={loading ?? false}
+            isLoading={!!loading}
             animateLoading={false}
             hasBorder={false}
             size={'full'}
@@ -143,7 +143,7 @@ export const JourneyScreenshotDialog = ({
             // we don't want this to be captured by row click which leads to step list page
             evt.stopPropagation();
           }}
-          onKeyDown={(evt) => {
+          onKeyDown={(_evt) => {
             // Just to satisfy ESLint
           }}
         >

x-pack/plugins/synthetics/public/apps/synthetics/components/common/screenshot/journey_step_screenshot_container.test.tsx

@@ -9,7 +9,6 @@ import React from 'react';
 import { fireEvent, waitFor, waitForElementToBeRemoved } from '@testing-library/react';
 import { JourneyStepScreenshotContainer } from './journey_step_screenshot_container';
 import { render } from '../../../utils/testing';
-import * as observabilityPublic from '@kbn/observability-plugin/public';
 import * as retrieveHooks from '../monitor_test_result/use_retrieve_step_image';
 import { getScreenshotUrl } from './journey_screenshot_dialog';
 
@@ -39,8 +38,8 @@ const testImageDataResult = {
 };
 
 describe('JourneyStepScreenshotContainer', () => {
+  afterEach(() => jest.clearAllMocks());
   let checkGroup: string;
-  const { FETCH_STATUS } = observabilityPublic;
 
   beforeAll(() => {
     checkGroup = 'test-check-group';
@@ -50,21 +49,8 @@ describe('JourneyStepScreenshotContainer', () => {
     jest.clearAllMocks();
   });
 
-  it.each([[FETCH_STATUS.PENDING], [FETCH_STATUS.LOADING]])(
-    'displays spinner when loading step image',
-    (fetchStatus) => {
-      jest
-        .spyOn(observabilityPublic, 'useFetcher')
-        .mockReturnValue({ status: fetchStatus, data: null, refetch: () => null, loading: true });
-      const { getByTestId } = render(<JourneyStepScreenshotContainer checkGroup={checkGroup} />);
-      expect(getByTestId('stepScreenshotPlaceholderLoading')).toBeInTheDocument();
-    }
-  );
-
   it('displays no image available when img src is unavailable and fetch status is successful', () => {
-    jest
-      .spyOn(observabilityPublic, 'useFetcher')
-      .mockReturnValue({ status: FETCH_STATUS.SUCCESS, data: null, refetch: () => null });
+    jest.spyOn(retrieveHooks, 'useRetrieveStepImage').mockReturnValue(undefined);
     const { getByTestId } = render(
       <JourneyStepScreenshotContainer checkGroup={checkGroup} allStepsLoaded={true} />
     );

x-pack/plugins/synthetics/public/apps/synthetics/components/common/screenshot/journey_step_screenshot_container.tsx

@@ -23,17 +23,19 @@ interface Props {
   allStepsLoaded?: boolean;
   retryFetchOnRevisit?: boolean; // Set to `true` for "Run Once" / "Test Now" modes
   size?: ScreenshotImageSize;
+  testNowMode?: boolean;
   unavailableMessage?: string;
   borderRadius?: number | string;
 }
 
 export const JourneyStepScreenshotContainer = ({
+  allStepsLoaded,
   timestamp,
   checkGroup,
   stepStatus,
-  allStepsLoaded,
   initialStepNumber = 1,
   retryFetchOnRevisit = false,
+  testNowMode,
   size = THUMBNAIL_SCREENSHOT_SIZE,
   unavailableMessage,
   borderRadius,
@@ -58,6 +60,7 @@ export const JourneyStepScreenshotContainer = ({
     imgPath,
     retryFetchOnRevisit,
     checkGroup,
+    testNowMode,
     timestamp,
   });
 

x-pack/plugins/synthetics/public/apps/synthetics/state/browser_journey/api.test.ts

@@ -81,7 +81,7 @@ describe('getJourneyScreenshot', () => {
     const mockFetch = jest.fn().mockResolvedValue(mockResponse);
     global.fetch = mockFetch;
 
-    const result = await getJourneyScreenshot(url, false);
+    const result = await getJourneyScreenshot(url, { shouldBackoff: false });
     expect(result).toBeNull();
     expect(mockFetch).toHaveBeenCalledTimes(1);
   });
@@ -146,7 +146,11 @@ describe('getJourneyScreenshot', () => {
     const mockFetch = jest.fn().mockReturnValue(mockResponse);
     global.fetch = mockFetch;
 
-    const result = await getJourneyScreenshot(url, true, maxRetry, initialBackoff);
+    const result = await getJourneyScreenshot(url, {
+      shouldBackoff: true,
+      maxRetry,
+      initialBackoff,
+    });
     expect(result).toBeNull();
     expect(mockFetch).toBeCalledTimes(maxRetry + 1);
   });

x-pack/plugins/synthetics/public/apps/synthetics/state/browser_journey/api.ts

@@ -75,12 +75,24 @@ export async function fetchLastSuccessfulCheck({
   );
 }
 
+export interface BackoffOptions {
+  shouldBackoff?: boolean;
+  maxRetry?: number;
+  initialBackoff?: number;
+}
+
+const DEFAULT_SHOULD_BACKOFF = true;
+const DEFAULT_MAX_RETRY = 8;
+const DEFAULT_INITIAL_BACKOFF = 100;
+
 export async function getJourneyScreenshot(
   imgSrc: string,
-  shouldBackoff = true,
-  maxRetry = 15,
-  initialBackoff = 100
+  options?: Partial<BackoffOptions>
 ): Promise<ScreenshotImageBlob | ScreenshotRefImageData | null> {
+  const shouldBackoff = options?.shouldBackoff ?? DEFAULT_SHOULD_BACKOFF;
+  const maxRetry = options?.maxRetry ?? DEFAULT_MAX_RETRY;
+  const initialBackoff = options?.initialBackoff ?? DEFAULT_INITIAL_BACKOFF;
+
   try {
     let retryCount = 0;
 
@@ -89,8 +101,12 @@ export async function getJourneyScreenshot(
     while (response?.status !== 200) {
       const imgRequest = new Request(imgSrc);
 
+      if (retryCount > maxRetry) break;
+
       response = await fetch(imgRequest);
-      if (!shouldBackoff || retryCount >= maxRetry || response.status !== 404) break;
+
+      if (!shouldBackoff || response.status !== 404) break;
+
       await new Promise((r) => setTimeout(r, (backoff *= 2)));
       retryCount++;
     }