(superset) branch enrich-screenshot-retry-log-context created (now a1d58fbb38d)

[el...@apache.org]

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

eschutho pushed a change to branch enrich-screenshot-retry-log-context
in repository https://gitbox.apache.org/repos/asf/superset.git


      at a1d58fbb38d fix(dashboard): add cacheKey, dashboardId, and format context to screenshot download error logs

This branch includes the following new commits:

     new a1d58fbb38d fix(dashboard): add cacheKey, dashboardId, and format context to screenshot download error logs

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.

(superset) 01/01: fix(dashboard): add cacheKey, dashboardId, and format context to screenshot download error logs

[el...@apache.org]

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

eschutho pushed a commit to branch enrich-screenshot-retry-log-context
in repository https://gitbox.apache.org/repos/asf/superset.git

commit a1d58fbb38de69eae94e0d5fa3999333249df40c
Author: Elizabeth Thompson <es...@gmail.com>
AuthorDate: Fri Jul 17 00:29:16 2026 +0000

    fix(dashboard): add cacheKey, dashboardId, and format context to screenshot download error logs
    
    Enriches the error logs emitted by useDownloadScreenshot's retry-exhausted and
    initial-request-failure paths with structured identifying context, so client-side
    download failures can be correlated with server-side logs for the same attempt.
    
    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(() => {