From a67add52399e1ad7914a3d0d23a1f198c18f80b9 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Piotr=20Jamr=C3=B3z?= <pm.jamroz@gmail.com>
Date: Wed, 25 May 2022 12:21:23 +0200
Subject: [PATCH] Query History: Track query history migration failures
 (#49560)

---
 public/app/core/utils/richHistory.test.ts    | 17 ++++++++++++++---
 public/app/core/utils/richHistory.ts         | 13 +++++++++----
 public/app/features/explore/state/history.ts |  7 ++++---
 3 files changed, 27 insertions(+), 10 deletions(-)

diff --git a/public/app/core/utils/richHistory.test.ts b/public/app/core/utils/richHistory.test.ts
index cff9ebe501e..9fbfe9b8a72 100644
--- a/public/app/core/utils/richHistory.test.ts
+++ b/public/app/core/utils/richHistory.test.ts
@@ -15,6 +15,7 @@ import {
   deleteQueryInRichHistory,
   migrateQueryHistoryFromLocalStorage,
   SortOrder,
+  LocalStorageMigrationStatus,
 } from './richHistory';
 
 const richHistoryStorageMock: RichHistoryStorage = {} as RichHistoryStorage;
@@ -187,13 +188,23 @@ describe('richHistory', () => {
       const history = { richHistory: [{ id: 'test' }, { id: 'test2' }], total: 2 };
 
       richHistoryLocalStorageMock.getRichHistory.mockReturnValue(history);
-      await migrateQueryHistoryFromLocalStorage();
+      const migrationResult = await migrateQueryHistoryFromLocalStorage();
       expect(richHistoryRemoteStorageMock.migrate).toBeCalledWith(history.richHistory);
+      expect(migrationResult.status).toBe(LocalStorageMigrationStatus.Successful);
+      expect(migrationResult.error).toBeUndefined();
     });
     it('does not migrate if there are no entries', async () => {
-      richHistoryLocalStorageMock.getRichHistory.mockReturnValue([]);
-      await migrateQueryHistoryFromLocalStorage();
+      richHistoryLocalStorageMock.getRichHistory.mockReturnValue({ richHistory: [] });
+      const migrationResult = await migrateQueryHistoryFromLocalStorage();
       expect(richHistoryRemoteStorageMock.migrate).not.toBeCalled();
+      expect(migrationResult.status).toBe(LocalStorageMigrationStatus.NotNeeded);
+      expect(migrationResult.error).toBeUndefined();
+    });
+    it('propagates thrown errors', async () => {
+      richHistoryLocalStorageMock.getRichHistory.mockRejectedValue(new Error('migration failed'));
+      const migrationResult = await migrateQueryHistoryFromLocalStorage();
+      expect(migrationResult.status).toBe(LocalStorageMigrationStatus.Failed);
+      expect(migrationResult.error?.message).toBe('migration failed');
     });
   });
 
diff --git a/public/app/core/utils/richHistory.ts b/public/app/core/utils/richHistory.ts
index 2b86d4305fd..560aa4a95a5 100644
--- a/public/app/core/utils/richHistory.ts
+++ b/public/app/core/utils/richHistory.ts
@@ -131,7 +131,12 @@ export enum LocalStorageMigrationStatus {
   NotNeeded = 'not-needed',
 }
 
-export async function migrateQueryHistoryFromLocalStorage(): Promise<LocalStorageMigrationStatus> {
+export interface LocalStorageMigrationResult {
+  status: LocalStorageMigrationStatus;
+  error?: Error;
+}
+
+export async function migrateQueryHistoryFromLocalStorage(): Promise<LocalStorageMigrationResult> {
   const richHistoryLocalStorage = new RichHistoryLocalStorage();
   const richHistoryRemoteStorage = new RichHistoryRemoteStorage();
 
@@ -145,14 +150,14 @@ export async function migrateQueryHistoryFromLocalStorage(): Promise<LocalStorag
       to: 14,
     });
     if (richHistory.length === 0) {
-      return LocalStorageMigrationStatus.NotNeeded;
+      return { status: LocalStorageMigrationStatus.NotNeeded };
     }
     await richHistoryRemoteStorage.migrate(richHistory);
     dispatch(notifyApp(createSuccessNotification('Query history successfully migrated from local storage')));
-    return LocalStorageMigrationStatus.Successful;
+    return { status: LocalStorageMigrationStatus.Successful };
   } catch (error) {
     dispatch(notifyApp(createWarningNotification(`Query history migration failed. ${error.message}`)));
-    return LocalStorageMigrationStatus.Failed;
+    return { status: LocalStorageMigrationStatus.Failed, error };
   }
 }
 
diff --git a/public/app/features/explore/state/history.ts b/public/app/features/explore/state/history.ts
index 23aba3ee9e5..e0fe9e9ee19 100644
--- a/public/app/features/explore/state/history.ts
+++ b/public/app/features/explore/state/history.ts
@@ -1,7 +1,7 @@
 import { AnyAction, createAction } from '@reduxjs/toolkit';
 
 import { DataQuery, HistoryItem } from '@grafana/data';
-import { config } from '@grafana/runtime';
+import { config, logError } from '@grafana/runtime';
 import { RICH_HISTORY_SETTING_KEYS } from 'app/core/history/richHistoryLocalStorageUtils';
 import store from 'app/core/store';
 import {
@@ -178,9 +178,10 @@ export const initRichHistory = (): ThunkResult<void> => {
     // the migration attempt happens only once per session, and the user is informed about the failure
     // in a way that can help with potential investigation.
     if (config.queryHistoryEnabled && !queriesMigrated && !migrationFailedDuringThisSession) {
-      const migrationStatus = await migrateQueryHistoryFromLocalStorage();
-      if (migrationStatus === LocalStorageMigrationStatus.Failed) {
+      const migrationResult = await migrateQueryHistoryFromLocalStorage();
+      if (migrationResult.status === LocalStorageMigrationStatus.Failed) {
         dispatch(richHistoryMigrationFailedAction());
+        logError(migrationResult.error!, { explore: { event: 'QueryHistoryMigrationFailed' } });
       } else {
         store.set(RICH_HISTORY_SETTING_KEYS.migrated, true);
       }