From f5a9b80052388294f1600974778f4ca1ca8cb4fe Mon Sep 17 00:00:00 2001
From: Alf Eaton <alf.eaton@overleaf.com>
Date: Mon, 7 Jun 2021 10:51:27 +0100
Subject: [PATCH] Merge pull request #4087 from overleaf/hb-chat-error-boundary

Chat error boundary

GitOrigin-RevId: 19bc6ee243d9b30510f2164462760bad04516ec3
---
 .../web/frontend/extracted-translations.json  |  2 +
 .../chat/components/chat-fallback-error.js    | 29 ++++++++++++++
 .../js/features/chat/components/chat-pane.js  | 14 ++++++-
 .../js/features/chat/context/chat-context.js  | 37 +++++++++++++++---
 .../frontend/stylesheets/app/editor/chat.less |  9 +++++
 services/web/locales/en.json                  |  2 +
 .../chat/components/chat-pane.test.js         | 28 +++++++++++++-
 .../chat/context/chat-context.test.js         | 38 +++++++++++++++++++
 8 files changed, 151 insertions(+), 8 deletions(-)
 create mode 100644 services/web/frontend/js/features/chat/components/chat-fallback-error.js

diff --git a/services/web/frontend/extracted-translations.json b/services/web/frontend/extracted-translations.json
index 4a4fdd94f9..35f272b27e 100644
--- a/services/web/frontend/extracted-translations.json
+++ b/services/web/frontend/extracted-translations.json
@@ -32,6 +32,7 @@
   "change_owner": "",
   "change_project_owner": "",
   "chat": "",
+  "chat_error": "",
   "checking_dropbox_status": "",
   "checking_project_github_status": "",
   "clear_cached_files": "",
@@ -241,6 +242,7 @@
   "recent_commits_in_github": "",
   "recompile": "",
   "recompile_from_scratch": "",
+  "reconnect": "",
   "reference_error_relink_hint": "",
   "refresh": "",
   "refresh_page_after_linking_dropbox": "",
diff --git a/services/web/frontend/js/features/chat/components/chat-fallback-error.js b/services/web/frontend/js/features/chat/components/chat-fallback-error.js
new file mode 100644
index 0000000000..c79b0ed439
--- /dev/null
+++ b/services/web/frontend/js/features/chat/components/chat-fallback-error.js
@@ -0,0 +1,29 @@
+import React from 'react'
+import PropTypes from 'prop-types'
+import { useTranslation } from 'react-i18next'
+import { Button, Alert } from 'react-bootstrap'
+
+function ChatFallbackError({ reconnect }) {
+  const { t } = useTranslation()
+
+  return (
+    <aside className="chat">
+      <div className="chat-error">
+        <Alert bsStyle="danger">{t('chat_error')}</Alert>
+        {reconnect && (
+          <p className="text-center">
+            <Button bsStyle="info" type="button" onClick={reconnect}>
+              {t('reconnect')}
+            </Button>
+          </p>
+        )}
+      </div>
+    </aside>
+  )
+}
+
+ChatFallbackError.propTypes = {
+  reconnect: PropTypes.any,
+}
+
+export default ChatFallbackError
diff --git a/services/web/frontend/js/features/chat/components/chat-pane.js b/services/web/frontend/js/features/chat/components/chat-pane.js
index ba40859124..a67670874f 100644
--- a/services/web/frontend/js/features/chat/components/chat-pane.js
+++ b/services/web/frontend/js/features/chat/components/chat-pane.js
@@ -5,10 +5,12 @@ import { useTranslation } from 'react-i18next'
 import MessageList from './message-list'
 import MessageInput from './message-input'
 import InfiniteScroll from './infinite-scroll'
+import ChatFallbackError from './chat-fallback-error'
 import Icon from '../../../shared/components/icon'
 import { useLayoutContext } from '../../../shared/context/layout-context'
 import { useApplicationContext } from '../../../shared/context/application-context'
 import withErrorBoundary from '../../../infrastructure/error-boundary'
+import { FetchError } from '../../../infrastructure/fetch-json'
 import { useChatContext } from '../context/chat-context'
 
 function ChatPane() {
@@ -26,8 +28,10 @@ function ChatPane() {
     atEnd,
     loadInitialMessages,
     loadMoreMessages,
+    reset,
     sendMessage,
     markMessagesAsRead,
+    error,
   } = useChatContext()
 
   useEffect(() => {
@@ -43,6 +47,14 @@ function ChatPane() {
     0
   )
 
+  if (error) {
+    // let user try recover from fetch errors
+    if (error instanceof FetchError) {
+      return <ChatFallbackError reconnect={reset} />
+    }
+    throw error
+  }
+
   return (
     <aside className="chat">
       <InfiniteScroll
@@ -95,4 +107,4 @@ function Placeholder() {
   )
 }
 
-export default withErrorBoundary(ChatPane)
+export default withErrorBoundary(ChatPane, ChatFallbackError)
diff --git a/services/web/frontend/js/features/chat/context/chat-context.js b/services/web/frontend/js/features/chat/context/chat-context.js
index d49357d8b2..dca8854d32 100644
--- a/services/web/frontend/js/features/chat/context/chat-context.js
+++ b/services/web/frontend/js/features/chat/context/chat-context.js
@@ -72,6 +72,9 @@ export function chatReducer(state, action) {
         unreadMessageCount: 0,
       }
 
+    case 'CLEAR':
+      return { ...initialState }
+
     case 'ERROR':
       return {
         ...state,
@@ -108,6 +111,8 @@ ChatContext.Provider.propTypes = {
     loadMoreMessages: PropTypes.func.isRequired,
     sendMessage: PropTypes.func.isRequired,
     markMessagesAsRead: PropTypes.func.isRequired,
+    reset: PropTypes.func.isRequired,
+    error: PropTypes.object,
   }).isRequired,
 }
 
@@ -129,7 +134,7 @@ export function ChatProvider({ children }) {
 
   const [state, dispatch] = useReducer(chatReducer, initialState)
 
-  const { loadInitialMessages, loadMoreMessages } = useMemo(() => {
+  const { loadInitialMessages, loadMoreMessages, reset } = useMemo(() => {
     function fetchMessages() {
       if (state.atEnd) return
 
@@ -142,12 +147,19 @@ export function ChatProvider({ children }) {
       const queryString = new URLSearchParams(query)
       const url = `/project/${projectId}/messages?${queryString.toString()}`
 
-      getJSON(url).then((messages = []) => {
-        dispatch({
-          type: 'FETCH_MESSAGES_SUCCESS',
-          messages: messages.reverse(),
+      getJSON(url)
+        .then((messages = []) => {
+          dispatch({
+            type: 'FETCH_MESSAGES_SUCCESS',
+            messages: messages.reverse(),
+          })
+        })
+        .catch(error => {
+          dispatch({
+            type: 'ERROR',
+            error: error,
+          })
         })
-      })
     }
 
     function loadInitialMessages() {
@@ -162,9 +174,15 @@ export function ChatProvider({ children }) {
       fetchMessages()
     }
 
+    function reset() {
+      dispatch({ type: 'CLEAR' })
+      fetchMessages()
+    }
+
     return {
       loadInitialMessages,
       loadMoreMessages,
+      reset,
     }
   }, [projectId, state.atEnd, state.initialMessagesLoaded, state.lastTimestamp])
 
@@ -181,6 +199,11 @@ export function ChatProvider({ children }) {
       const url = `/project/${projectId}/messages`
       postJSON(url, {
         body: { content },
+      }).catch(error => {
+        dispatch({
+          type: 'ERROR',
+          error: error,
+        })
       })
     },
     [projectId, user]
@@ -249,8 +272,10 @@ export function ChatProvider({ children }) {
     unreadMessageCount: state.unreadMessageCount,
     loadInitialMessages,
     loadMoreMessages,
+    reset,
     sendMessage,
     markMessagesAsRead,
+    error: state.error,
   }
 
   return <ChatContext.Provider value={value}>{children}</ChatContext.Provider>
diff --git a/services/web/frontend/stylesheets/app/editor/chat.less b/services/web/frontend/stylesheets/app/editor/chat.less
index e98e0d1396..5c23142778 100644
--- a/services/web/frontend/stylesheets/app/editor/chat.less
+++ b/services/web/frontend/stylesheets/app/editor/chat.less
@@ -26,6 +26,15 @@
     color: @chat-instructions-color;
   }
 
+  .chat-error {
+    position: absolute;
+    top: 0;
+    bottom: 0;
+    background-color: @chat-bg;
+    padding: @line-height-computed / 2;
+    text-align: center;
+  }
+
   .messages {
     position: absolute;
     top: 0;
diff --git a/services/web/locales/en.json b/services/web/locales/en.json
index ef3f737800..317d927b80 100644
--- a/services/web/locales/en.json
+++ b/services/web/locales/en.json
@@ -1,4 +1,6 @@
 {
+  "chat_error": "Could not load chat messages, please try again.",
+  "reconnect": "Try again",
   "logs_pane_info_message": "We are testing a new logs pane",
   "logs_pane_info_message_popup": "We are testing a new logs pane. Click here to give feedback.",
   "github_symlink_error": "Your Github repository contains symbolic link files, which are not currently supported by Overleaf. Please remove these and try again.",
diff --git a/services/web/test/frontend/features/chat/components/chat-pane.test.js b/services/web/test/frontend/features/chat/components/chat-pane.test.js
index 3bd491c6ea..e2c1b266f6 100644
--- a/services/web/test/frontend/features/chat/components/chat-pane.test.js
+++ b/services/web/test/frontend/features/chat/components/chat-pane.test.js
@@ -1,6 +1,10 @@
 import React from 'react'
 import { expect } from 'chai'
-import { screen, waitForElementToBeRemoved } from '@testing-library/react'
+import {
+  fireEvent,
+  screen,
+  waitForElementToBeRemoved,
+} from '@testing-library/react'
 import fetchMock from 'fetch-mock'
 
 import ChatPane from '../../../../../frontend/js/features/chat/components/chat-pane'
@@ -52,6 +56,28 @@ describe('<ChatPane />', function () {
     await screen.findByText('another message')
   })
 
+  it('provides error message with reload button on FetchError', async function () {
+    fetchMock.get(/messages/, 500)
+
+    renderWithChatContext(<ChatPane />, { user })
+
+    // should have hit a FetchError and will prompt user to reconnect
+    await screen.findByText('Try again')
+
+    // bring chat back up
+    fetchMock.reset()
+    fetchMock.get(/messages/, [])
+
+    const reconnectButton = screen.getByRole('button', {
+      name: 'Try again',
+    })
+    expect(reconnectButton).to.exist
+
+    // should now reconnect with placeholder message
+    fireEvent.click(reconnectButton)
+    await screen.findByText('Send your first message to your collaborators')
+  })
+
   it('a loading spinner is rendered while the messages are loading, then disappears', async function () {
     fetchMock.get(/messages/, [])
 
diff --git a/services/web/test/frontend/features/chat/context/chat-context.test.js b/services/web/test/frontend/features/chat/context/chat-context.test.js
index 82c8e1d896..da0ae41364 100644
--- a/services/web/test/frontend/features/chat/context/chat-context.test.js
+++ b/services/web/test/frontend/features/chat/context/chat-context.test.js
@@ -208,6 +208,18 @@ describe('ChatContext', function () {
       result.current.loadInitialMessages()
       expect(fetchMock.calls()).to.have.lengthOf(1)
     })
+
+    it('provides an error on failure', async function () {
+      fetchMock.reset()
+      fetchMock.get('express:/project/:projectId/messages', 500)
+      const { result, waitForNextUpdate } = renderChatContextHook({ user })
+
+      result.current.loadInitialMessages()
+      await waitForNextUpdate()
+
+      expect(result.current.error).to.exist
+      expect(result.current.status).to.equal('error')
+    })
   })
 
   describe('loadMoreMessages', function () {
@@ -352,6 +364,18 @@ describe('ChatContext', function () {
         'socket message',
       ])
     })
+
+    it('provides an error on failures', async function () {
+      fetchMock.reset()
+      fetchMock.get('express:/project/:projectId/messages', 500)
+      const { result, waitForNextUpdate } = renderChatContextHook({ user })
+
+      result.current.loadMoreMessages()
+      await waitForNextUpdate()
+
+      expect(result.current.error).to.exist
+      expect(result.current.status).to.equal('error')
+    })
   })
 
   describe('sendMessage', function () {
@@ -397,6 +421,20 @@ describe('ChatContext', function () {
         })
       ).to.be.false
     })
+
+    it('provides an error on failure', async function () {
+      fetchMock.reset()
+      fetchMock
+        .get('express:/project/:projectId/messages', [])
+        .postOnce('express:/project/:projectId/messages', 500)
+      const { result, waitForNextUpdate } = renderChatContextHook({ user })
+
+      result.current.sendMessage('sent message')
+      await waitForNextUpdate()
+
+      expect(result.current.error).to.exist
+      expect(result.current.status).to.equal('error')
+    })
   })
 
   describe('unread messages', function () {