From 1fb921de99d45f130fed92f7bdb3f4a1db73387c Mon Sep 17 00:00:00 2001
From: Tim Down <158919+timdown@users.noreply.github.com>
Date: Tue, 2 May 2023 15:04:58 +0100
Subject: [PATCH] History migration: Add error handling for all history
 requests (#12872)

* Add error handling for all history requests

* Remove comment

GitOrigin-RevId: 528dc98a0fc4ab523f8536274996c4166be45064
---
 .../components/change-list/modal-error.tsx    |  4 +-
 .../components/diff-view/diff-view.tsx        | 30 +++++++-----
 .../history/components/error-message.tsx      | 13 ++++++
 .../history/components/history-file-tree.tsx  |  6 +--
 .../history/components/history-root.tsx       | 28 ++++++-----
 .../history/context/history-context.tsx       | 46 +++++++++++--------
 .../context/hooks/use-restore-deleted-file.ts |  3 +-
 .../context/types/history-context-value.ts    |  1 +
 .../stylesheets/app/editor/history-react.less |  4 ++
 9 files changed, 86 insertions(+), 49 deletions(-)
 create mode 100644 services/web/frontend/js/features/history/components/error-message.tsx

diff --git a/services/web/frontend/js/features/history/components/change-list/modal-error.tsx b/services/web/frontend/js/features/history/components/change-list/modal-error.tsx
index dfe891b94b..0d1399d231 100644
--- a/services/web/frontend/js/features/history/components/change-list/modal-error.tsx
+++ b/services/web/frontend/js/features/history/components/change-list/modal-error.tsx
@@ -4,7 +4,7 @@ import { Alert } from 'react-bootstrap'
 // Using this workaround due to inconsistent and improper error responses from the server
 type ModalErrorProps = {
   error: {
-    response: Response
+    response?: Response
     data?: {
       message?: string
     }
@@ -14,7 +14,7 @@ type ModalErrorProps = {
 function ModalError({ error }: ModalErrorProps) {
   const { t } = useTranslation()
 
-  if (error.response.status === 400 && error?.data?.message) {
+  if (error.response?.status === 400 && error.data?.message) {
     return <Alert bsStyle="danger">{error.data.message}</Alert>
   }
 
diff --git a/services/web/frontend/js/features/history/components/diff-view/diff-view.tsx b/services/web/frontend/js/features/history/components/diff-view/diff-view.tsx
index 3a07c2fc93..06687d3ff8 100644
--- a/services/web/frontend/js/features/history/components/diff-view/diff-view.tsx
+++ b/services/web/frontend/js/features/history/components/diff-view/diff-view.tsx
@@ -7,12 +7,13 @@ import { useHistoryContext } from '../../context/history-context'
 import { diffDoc } from '../../services/api'
 import { highlightsFromDiffResponse } from '../../utils/highlights-from-diff-response'
 import useAsync from '../../../../shared/hooks/use-async'
+import ErrorMessage from '../error-message'
 
 function DiffView() {
   const [diff, setDiff] = useState<Nullable<Diff>>(null)
   const { selection, projectId } = useHistoryContext()
 
-  const { isLoading, runAsync } = useAsync<DocDiffResponse>()
+  const { isLoading, runAsync, error } = useAsync<DocDiffResponse>()
 
   const { updateRange, selectedFile } = selection
 
@@ -23,9 +24,8 @@ function DiffView() {
 
     const { fromV, toV } = updateRange
 
-    // TODO: Error handling
-    runAsync(diffDoc(projectId, fromV, toV, selectedFile.pathname)).then(
-      data => {
+    runAsync(diffDoc(projectId, fromV, toV, selectedFile.pathname))
+      .then(data => {
         let diff: Diff | undefined
 
         if (!data?.diff) {
@@ -42,18 +42,24 @@ function DiffView() {
         }
 
         setDiff(diff)
-      }
-    )
+      })
+      .catch(console.error)
   }, [projectId, runAsync, updateRange, selectedFile])
 
   return (
     <div className="doc-panel">
-      <div className="history-header toolbar-container">
-        <Toolbar diff={diff} selection={selection} />
-      </div>
-      <div className="doc-container">
-        <Main diff={diff} isLoading={isLoading} />
-      </div>
+      {error ? (
+        <ErrorMessage />
+      ) : (
+        <>
+          <div className="history-header toolbar-container">
+            <Toolbar diff={diff} selection={selection} />
+          </div>
+          <div className="doc-container">
+            <Main diff={diff} isLoading={isLoading} />
+          </div>
+        </>
+      )}
     </div>
   )
 }
diff --git a/services/web/frontend/js/features/history/components/error-message.tsx b/services/web/frontend/js/features/history/components/error-message.tsx
new file mode 100644
index 0000000000..dda8cb4c99
--- /dev/null
+++ b/services/web/frontend/js/features/history/components/error-message.tsx
@@ -0,0 +1,13 @@
+import { useTranslation } from 'react-i18next'
+
+export default function ErrorMessage() {
+  const { t } = useTranslation()
+
+  return (
+    <div className="history-error">
+      <div className="text-danger error">
+        {t('generic_something_went_wrong')}
+      </div>
+    </div>
+  )
+}
diff --git a/services/web/frontend/js/features/history/components/history-file-tree.tsx b/services/web/frontend/js/features/history/components/history-file-tree.tsx
index 9b8e70dc7f..8433a4b8c9 100644
--- a/services/web/frontend/js/features/history/components/history-file-tree.tsx
+++ b/services/web/frontend/js/features/history/components/history-file-tree.tsx
@@ -7,13 +7,13 @@ import {
 import HistoryFileTreeFolderList from './file-tree/history-file-tree-folder-list'
 
 export default function HistoryFileTree() {
-  const { files } = useHistoryContext().selection
+  const { selection, error } = useHistoryContext()
 
-  const fileTree = _.reduce(files, reducePathsToTree, [])
+  const fileTree = _.reduce(selection.files, reducePathsToTree, [])
 
   const mappedFileTree = fileTreeDiffToFileTreeData(fileTree)
 
-  return (
+  return error ? null : (
     <HistoryFileTreeFolderList
       folders={mappedFileTree.folders}
       docs={mappedFileTree.docs ?? []}
diff --git a/services/web/frontend/js/features/history/components/history-root.tsx b/services/web/frontend/js/features/history/components/history-root.tsx
index ecd051d8ab..f84fed47b6 100644
--- a/services/web/frontend/js/features/history/components/history-root.tsx
+++ b/services/web/frontend/js/features/history/components/history-root.tsx
@@ -4,27 +4,33 @@ import { HistoryProvider, useHistoryContext } from '../context/history-context'
 import { createPortal } from 'react-dom'
 import HistoryFileTree from './history-file-tree'
 import LoadingSpinner from '../../../shared/components/loading-spinner'
+import ErrorMessage from './error-message'
 
 const fileTreeContainer = document.getElementById('history-file-tree')
 
 function Main() {
-  const { loadingState } = useHistoryContext()
+  const { loadingState, error } = useHistoryContext()
+
+  let content = null
+  if (loadingState === 'loadingInitial') {
+    content = <LoadingSpinner />
+  } else if (error) {
+    content = <ErrorMessage />
+  } else {
+    content = (
+      <>
+        <DiffView />
+        <ChangeList />
+      </>
+    )
+  }
 
   return (
     <>
       {fileTreeContainer
         ? createPortal(<HistoryFileTree />, fileTreeContainer)
         : null}
-      <div className="history-react">
-        {loadingState === 'loadingInitial' ? (
-          <LoadingSpinner />
-        ) : (
-          <>
-            <DiffView />
-            <ChangeList />
-          </>
-        )}
-      </div>
+      <div className="history-react">{content}</div>
     </>
   )
 }
diff --git a/services/web/frontend/js/features/history/context/history-context.tsx b/services/web/frontend/js/features/history/context/history-context.tsx
index 4340d4192e..5e54517e59 100644
--- a/services/web/frontend/js/features/history/context/history-context.tsx
+++ b/services/web/frontend/js/features/history/context/history-context.tsx
@@ -88,7 +88,7 @@ function useHistory() {
   const [labels, setLabels] = useState<HistoryContextValue['labels']>(null)
   const [loadingState, setLoadingState] =
     useState<HistoryContextValue['loadingState']>('loadingInitial')
-  const [error, setError] = useState(null)
+  const [error, setError] = useState<HistoryContextValue['error']>(null)
 
   const fetchNextBatchOfUpdates = useCallback(() => {
     const loadUpdates = (updatesData: Update[]) => {
@@ -197,28 +197,32 @@ function useHistory() {
     }
     const { fromV, toV } = updateRange
 
-    diffFiles(projectId, fromV, toV).then(({ diff: files }) => {
-      const selectedFile = autoSelectFile(
-        files,
-        updateRange.toV,
-        comparing,
-        updates
-      )
-      const newFiles = files.map(file => {
-        if (isFileRenamed(file) && file.newPathname) {
-          return renamePathnameKey(file)
-        }
+    diffFiles(projectId, fromV, toV)
+      .then(({ diff: files }) => {
+        const selectedFile = autoSelectFile(
+          files,
+          updateRange.toV,
+          comparing,
+          updates
+        )
+        const newFiles = files.map(file => {
+          if (isFileRenamed(file) && file.newPathname) {
+            return renamePathnameKey(file)
+          }
 
-        return file
+          return file
+        })
+        setSelection({
+          updateRange,
+          comparing,
+          files: newFiles,
+          selectedFile,
+        })
       })
-      setSelection({
-        updateRange,
-        comparing,
-        files: newFiles,
-        selectedFile,
+      .catch(error => {
+        setError(error)
       })
-    })
-  }, [updateRange, projectId, updates, comparing])
+  }, [updateRange, projectId, updates, comparing, setError])
 
   useEffect(() => {
     // Set update range if there isn't one and updates have loaded
@@ -239,6 +243,7 @@ function useHistory() {
   const value = useMemo<HistoryContextValue>(
     () => ({
       error,
+      setError,
       loadingState,
       setLoadingState,
       updatesInfo,
@@ -255,6 +260,7 @@ function useHistory() {
     }),
     [
       error,
+      setError,
       loadingState,
       setLoadingState,
       updatesInfo,
diff --git a/services/web/frontend/js/features/history/context/hooks/use-restore-deleted-file.ts b/services/web/frontend/js/features/history/context/hooks/use-restore-deleted-file.ts
index 55365e94ab..4998f4d8b1 100644
--- a/services/web/frontend/js/features/history/context/hooks/use-restore-deleted-file.ts
+++ b/services/web/frontend/js/features/history/context/hooks/use-restore-deleted-file.ts
@@ -9,7 +9,7 @@ import type { HistoryContextValue } from '../types/history-context-value'
 
 export function useRestoreDeletedFile() {
   const { runAsync } = useAsync()
-  const { setLoadingState, projectId } = useHistoryContext()
+  const { setLoadingState, projectId, setError } = useHistoryContext()
   const ide = useIdeContext()
   const { setView } = useLayoutContext()
 
@@ -35,6 +35,7 @@ export function useRestoreDeletedFile() {
 
             setView('editor')
           })
+          .catch(error => setError(error))
           .finally(() => {
             setLoadingState('ready')
           })
diff --git a/services/web/frontend/js/features/history/context/types/history-context-value.ts b/services/web/frontend/js/features/history/context/types/history-context-value.ts
index 71fe67e9cb..a61bd56afa 100644
--- a/services/web/frontend/js/features/history/context/types/history-context-value.ts
+++ b/services/web/frontend/js/features/history/context/types/history-context-value.ts
@@ -27,6 +27,7 @@ export type HistoryContextValue = {
     React.SetStateAction<HistoryContextValue['loadingState']>
   >
   error: Nullable<unknown>
+  setError: React.Dispatch<React.SetStateAction<HistoryContextValue['error']>>
   labels: Nullable<LoadedLabel[]>
   setLabels: React.Dispatch<React.SetStateAction<HistoryContextValue['labels']>>
   projectId: string
diff --git a/services/web/frontend/stylesheets/app/editor/history-react.less b/services/web/frontend/stylesheets/app/editor/history-react.less
index 9f080f39a3..b6b04f31e3 100644
--- a/services/web/frontend/stylesheets/app/editor/history-react.less
+++ b/services/web/frontend/stylesheets/app/editor/history-react.less
@@ -485,3 +485,7 @@ history-root {
     }
   }
 }
+
+.history-error {
+  padding: 16px;
+}