You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@superset.apache.org by ru...@apache.org on 2026/07/21 17:10:22 UTC

(superset) branch master updated: fix(dashboard): add cacheKey, dashboardId, and format context to screenshot download error logs (#42121)

This is an automated email from the ASF dual-hosted git repository.

rusackas pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git


The following commit(s) were added to refs/heads/master by this push:
     new 1a862476e1e fix(dashboard): add cacheKey, dashboardId, and format context to screenshot download error logs (#42121)
1a862476e1e is described below

commit 1a862476e1ea5b5dcbbb6df3d8a59c9310157dfc
Author: Elizabeth Thompson <es...@gmail.com>
AuthorDate: Tue Jul 21 10:10:14 2026 -0700

    fix(dashboard): add cacheKey, dashboardId, and format context to screenshot download error logs (#42121)
    
    Co-authored-by: Claude <no...@anthropic.com>
---
 .../dashboard/hooks/useDownloadScreenshot.test.ts  | 50 ++++++++++++++++++++++
 .../src/dashboard/hooks/useDownloadScreenshot.ts   | 12 +++++-
 2 files changed, 60 insertions(+), 2 deletions(-)

diff --git a/superset-frontend/src/dashboard/hooks/useDownloadScreenshot.test.ts b/superset-frontend/src/dashboard/hooks/useDownloadScreenshot.test.ts
index 27133b4e3c6..ce03dda3da5 100644
--- a/superset-frontend/src/dashboard/hooks/useDownloadScreenshot.test.ts
+++ b/superset-frontend/src/dashboard/hooks/useDownloadScreenshot.test.ts
@@ -18,6 +18,7 @@
  */
 import { renderHook, act } from '@testing-library/react';
 import { SupersetClient } from '@superset-ui/core';
+import { logging } from '@apache-superset/core/utils';
 import { useDownloadScreenshot } from './useDownloadScreenshot';
 import { DownloadScreenshotFormat } from '../components/menu/DownloadMenuItems/types';
 
@@ -35,6 +36,12 @@ jest.mock('@superset-ui/core', () => ({
   },
 }));
 
+jest.mock('@apache-superset/core/utils', () => ({
+  logging: {
+    error: jest.fn(),
+  },
+}));
+
 jest.mock('react-redux', () => ({
   useSelector: jest.fn(() => undefined),
 }));
@@ -171,3 +178,46 @@ test('triggers only one download when multiple successful responses race', async
   jest.clearAllTimers();
   jest.useRealTimers();
 });
+
+test('logs cacheKey, dashboardId, and format when retries are exhausted', async () => {
+  jest.useFakeTimers();
+  mockPostSuccess();
+  (SupersetClient.get as jest.Mock).mockRejectedValue(notReadyError());
+
+  await triggerDownload();
+
+  // Drive one retry interval at a time so each failed GET has a chance to
+  // resolve and increment the retry counter before the next interval fires.
+  for (let i = 0; i < 31; i += 1) {
+    // eslint-disable-next-line no-await-in-loop
+    await act(async () => {
+      jest.advanceTimersByTime(RETRY_INTERVAL);
+      await flushPromises();
+    });
+  }
+
+  expect(logging.error).toHaveBeenCalledWith('Max retries reached', {
+    cacheKey: CACHE_KEY,
+    dashboardId: DASHBOARD_ID,
+    format: DownloadScreenshotFormat.PNG,
+  });
+
+  jest.clearAllTimers();
+  jest.useRealTimers();
+});
+
+test('logs dashboardId, format, and the error when the initial screenshot request fails', async () => {
+  const error = new Error('network down');
+  (SupersetClient.post as jest.Mock).mockRejectedValue(error);
+
+  const { result } = renderHook(() => useDownloadScreenshot(DASHBOARD_ID));
+  await act(async () => {
+    result.current(DownloadScreenshotFormat.PDF);
+    await flushPromises();
+  });
+
+  expect(logging.error).toHaveBeenCalledWith(
+    'Failed to trigger dashboard screenshot',
+    { dashboardId: DASHBOARD_ID, format: DownloadScreenshotFormat.PDF, error },
+  );
+});
diff --git a/superset-frontend/src/dashboard/hooks/useDownloadScreenshot.ts b/superset-frontend/src/dashboard/hooks/useDownloadScreenshot.ts
index 8e286521be4..93e2198afe9 100644
--- a/superset-frontend/src/dashboard/hooks/useDownloadScreenshot.ts
+++ b/superset-frontend/src/dashboard/hooks/useDownloadScreenshot.ts
@@ -146,7 +146,11 @@ export const useDownloadScreenshot = (
         }
         if (retries >= MAX_RETRIES) {
           stopIntervals('failure');
-          logging.error('Max retries reached');
+          logging.error('Max retries reached', {
+            cacheKey,
+            dashboardId,
+            format,
+          });
           return;
         }
         isFetching = true;
@@ -180,7 +184,11 @@ export const useDownloadScreenshot = (
           fetchImageWithRetry(cacheKey);
         })
         .catch(error => {
-          logging.error(error);
+          logging.error('Failed to trigger dashboard screenshot', {
+            dashboardId,
+            format,
+            error,
+          });
           stopIntervals('failure');
         })
         .finally(() => {