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

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++;
     }