Merge pull request #217 from unfoldingWord/feature-mcleanb-130-handle…

…HttpErrors.2

Bugfix/Fixes for long timeouts due to internet disconnects

.gitignore

@@ -1,7 +1,7 @@
 # See https://help.github.com/ignore-files/ for more about ignoring files.
 yalc.lock
 .yalc
-.env.local
+.env.local*
 # dependencies
 /node_modules
 

package.json

@@ -1,6 +1,6 @@
 {
   "name": "gateway-edit",
-  "version": "0.9.0",
+  "version": "1.0.0",
   "scripts": {
     "dev": "bash -c \"source ./scripts/set-env.sh && next\"",
     "build": "bash -c \"source ./scripts/set-env.sh && next build\"",
@@ -18,7 +18,7 @@
     "bible-reference-rcl": "0.5.0",
     "core-js": "^3.8.3",
     "deep-equal": "^2.0.5",
-    "gitea-react-toolkit": "1.7.0",
+    "gitea-react-toolkit": "1.8.0",
     "localforage": "^1.9.0",
     "markdown-translatable": "^1.2.15",
     "next": "10.2.0",
@@ -28,9 +28,9 @@
     "regenerator-runtime": "^0.13.7",
     "resource-workspace-rcl": "1.0.0",
     "scripture-resources-rcl": "4.1.8",
-    "single-scripture-rcl": "2.0.4",
+    "single-scripture-rcl": "2.0.5",
     "tailwindcss": "^2.0.4",
-    "translation-helps-rcl": "1.46.20",
+    "translation-helps-rcl": "1.47.0",
     "use-deep-compare-effect": "^1.3.1"
   },
   "devDependencies": {

pages/feedback.js

@@ -13,9 +13,9 @@ import CloseIcon from '@material-ui/icons/Close'
 import Layout from '@components/Layout'
 import { StoreContext } from '@context/StoreContext'
 import { getBuildId } from '@utils/build'
-import { getUserItem, getUserKey } from '@hooks/useUserLocalStorage'
+import { getLocalStorageItem, getUserKey } from '@hooks/useUserLocalStorage'
 import { processNetworkError } from '@utils/network'
-import { CLOSE } from '@common/constants'
+import { CLOSE, HTTP_GET_MAX_WAIT_TIME } from '@common/constants'
 import NetworkErrorPopup from '@components/NetworkErrorPopUp'
 
 function Alert({ severity, message }) {
@@ -89,11 +89,11 @@ const SettingsPage = () => {
 
   /**
    * in the case of a network error, process and display error dialog
-   * @param {string} errorMessage - optional error message returned
+   * @param {string|Error} error - initial error message message or object
    * @param {number} httpCode - http code returned
    */
-  function processError(errorMessage, httpCode=0) {
-    processNetworkError(errorMessage, httpCode, null, router, setNetworkError, null, null )
+  function processError(error, httpCode=0) {
+    processNetworkError(error, httpCode, null, router, setNetworkError, null, null )
   }
 
   function onClose() {
@@ -118,7 +118,7 @@ const SettingsPage = () => {
 
   function getUserSettings(username, baseKey) {
     const key = getUserKey(username, baseKey)
-    const savedValue = getUserItem(key)
+    const savedValue = getLocalStorageItem(key)
     return savedValue
   }
 
@@ -178,16 +178,24 @@ const SettingsPage = () => {
     let res
 
     try {
-      res = await fetch('/api/feedback', {
+      const fetchPromise = fetch('/api/feedback', {
         method: 'POST',
         headers: { 'Content-Type': 'application/json' },
         body: JSON.stringify({
           name, email, category, message, extraData,
         }),
       })
+      const timeout = new Promise((_r, rej) => {
+        const TIMEOUT_ERROR = `Network Timeout Error ${HTTP_GET_MAX_WAIT_TIME}ms`
+        return setTimeout(() => rej(TIMEOUT_ERROR), HTTP_GET_MAX_WAIT_TIME)
+      })
+      res = await Promise.race([fetchPromise, timeout])
     } catch (e) {
       console.warn(`onSubmitFeedback() - failure calling '/api/feedback'`, e)
-      processError(`Failure calling '/api/feedback': ${e.toString()}`)
+      processError(e)
+      setSubmitting(false)
+      setShowSuccess(false)
+      setShowError(true)
       return
     }
 

src/common/constants.js

@@ -6,6 +6,9 @@ export const BASE_URL = 'https://git.door43.org'
 export const TOKEN_ID = 'gatewayEdit'
 export const FEEDBACK_PAGE = '/feedback'
 
+export const SERVER_MAX_WAIT_TIME_RETRY = 10000 // in milliseconds
+export const HTTP_GET_MAX_WAIT_TIME = 5000 // in milliseconds
+
 // UI text - may eventually need to localize
 export const MANIFEST_NOT_FOUND_ERROR = 'This resource manifest failed to load. Please confirm that the correct manifest.yaml file exists in the resource at:\n'
 export const MANIFEST_INVALID_ERROR = 'The manifest for this resource is invalid. Resource is at:\n'

src/components/ResourceCard.js

@@ -10,6 +10,7 @@ import {
 } from 'translation-helps-rcl'
 import { getResourceMessage } from '@utils/resources'
 import { getResourceErrorMessage } from 'single-scripture-rcl'
+import { HTTP_GET_MAX_WAIT_TIME } from '@common/constants'
 
 export default function ResourceCard({
   id,
@@ -50,6 +51,7 @@ export default function ResourceCard({
     owner,
     server,
     onResourceError,
+    timeout: HTTP_GET_MAX_WAIT_TIME,
   })
 
   const {

src/components/TranslationSettings.js

@@ -12,9 +12,14 @@ import { getGatewayLanguages } from '@common/languages'
 import { StoreContext } from '@context/StoreContext'
 import { FormHelperText } from '@material-ui/core'
 import {
-  LOADING, NO_ORGS_ERROR, ORGS_NETWORK_ERROR,
+  HTTP_GET_MAX_WAIT_TIME,
+  LOADING,
+  NO_ORGS_ERROR,
+  ORGS_NETWORK_ERROR,
 } from '@common/constants'
 import {
+  doFetch,
+  isServerDisconnected,
   onNetworkActionButton,
   processNetworkError,
   reloadApp,
@@ -50,11 +55,11 @@ export default function TranslationSettings({ authentication }) {
 
   /**
    * in the case of a network error, process and display error dialog
-   * @param {string} errorMessage - optional error message returned
+   * @param {string|Error} error - initial error message message or object
    * @param {number} httpCode - http code returned
    */
-  function processError(errorMessage, httpCode=0) {
-    processNetworkError(errorMessage, httpCode, logout, router, setNetworkError, setLastError, setOrgErrorMessage )
+  function processError(error, httpCode=0) {
+    processNetworkError(error, httpCode, logout, router, setNetworkError, setLastError, setOrgErrorMessage )
   }
 
   useEffect(() => {
@@ -64,15 +69,16 @@ export default function TranslationSettings({ authentication }) {
       let errorCode = 0
 
       try {
-        const orgs = await fetch('https://git.door43.org/api/v1/user/orgs', { ...authentication.config })
+        const orgs = await doFetch('https://git.door43.org/api/v1/user/orgs',
+          authentication, HTTP_GET_MAX_WAIT_TIME)
           .then(response => {
             if (response?.status !== 200) {
-              console.warn(`TranslationSettings - error fetching user orgs, status code ${response?.status}`)
               errorCode = response?.status
+              console.warn(`TranslationSettings - error fetching user orgs, status code ${errorCode}`)
               processError(null, errorCode)
               return null
             }
-            return response?.json()
+            return response?.data
           })
           .then(data => {
             if (Array.isArray(data)) {
@@ -83,17 +89,20 @@ export default function TranslationSettings({ authentication }) {
           })
 
         if (!orgs?.length) { // if no orgs
+          console.warn(`TranslationSettings - empty orgs`)
           setOrgErrorMessage(NO_ORGS_ERROR)
         } else {
           setOrgErrorMessage(null)
         }
 
         setOrganizations(orgs)
       } catch (e) {
-        console.warn(`TranslationSettings - error fetching user orgs`, e)
+        const message = e?.message
+        const disconnected = isServerDisconnected(e)
+        console.warn(`TranslationSettings - error fetching user orgs, message '${message}', disconnected=${disconnected}`, e)
         setOrganizations([])
-        setOrgErrorMessage(NO_ORGS_ERROR)
-        processError(ORGS_NETWORK_ERROR)
+        setOrgErrorMessage(disconnected ? ORGS_NETWORK_ERROR : NO_ORGS_ERROR)
+        processError(e)
       }
     }
 

src/components/WorkspaceContainer.js

@@ -18,8 +18,7 @@ import {
   NT_ORIG_LANG_BIBLE,
   OT_ORIG_LANG_BIBLE,
 } from 'single-scripture-rcl'
-import DraggableCard from 'translation-helps-rcl/dist/components/DraggableCard'
-import useResourceClickListener from 'translation-helps-rcl/dist/hooks/useResourceClickListener'
+import { DraggableCard, useResourceClickListener } from 'translation-helps-rcl'
 import ResourceCard from '@components/ResourceCard'
 import { getResourceBibles } from '@utils/resources'
 import { StoreContext } from '@context/StoreContext'
@@ -33,7 +32,7 @@ import {
   reloadApp,
 } from '@utils/network'
 import { useRouter } from 'next/router'
-import { MANIFEST_INVALID_ERROR } from '@common/constants'
+import { HTTP_GET_MAX_WAIT_TIME, MANIFEST_INVALID_ERROR } from '@common/constants'
 import NetworkErrorPopup from '@components/NetworkErrorPopUp'
 
 const useStyles = makeStyles(() => ({
@@ -96,6 +95,7 @@ function WorkspaceContainer() {
     taArticle,
     languageId,
     onResourceError,
+    timeout: HTTP_GET_MAX_WAIT_TIME,
   })
 
   function isNT(bookId) {
@@ -144,10 +144,10 @@ function WorkspaceContainer() {
     return null
   }
 
-  function onResourceError(message, isAccessError) {
+  function onResourceError(message, isAccessError, resourceStatus, error) {
     if (!networkError && // only show if another error not already showing
         isAccessError) { // we only show popup for access errors
-      addNetworkDisconnectError(message, 0, logout, router, setNetworkError, setLastError )
+      addNetworkDisconnectError(error || message, 0, logout, router, setNetworkError, setLastError )
     }
   }
 
@@ -160,6 +160,7 @@ function WorkspaceContainer() {
     useUserLocalStorage,
     originalLanguageOwner: scriptureOwner,
     onResourceError,
+    timeout: HTTP_GET_MAX_WAIT_TIME,
   }
 
   const commonResourceCardConfigs = {
@@ -229,6 +230,7 @@ function WorkspaceContainer() {
     server,
     branch,
     cache: { maxAge: 1 * 1 * 1 * 60 * 1000 },
+    timeout: HTTP_GET_MAX_WAIT_TIME,
   }
 
   const originalScriptureConfig = useScripture({

src/context/AuthContext.js

@@ -1,10 +1,17 @@
-import React, { useState, createContext } from 'react'
+import React, { createContext, useState } from 'react'
 import localforage from 'localforage'
 import { AuthenticationContextProvider } from 'gitea-react-toolkit'
 import {
-  BASE_URL, CLOSE, TOKEN_ID,
+  BASE_URL,
+  CLOSE,
+  HTTP_GET_MAX_WAIT_TIME,
+  TOKEN_ID,
 } from '@common/constants'
-import { processNetworkError, unAuthenticated } from '@utils/network'
+import {
+  doFetch,
+  processNetworkError,
+  unAuthenticated,
+} from '@utils/network'
 import NetworkErrorPopup from '@components/NetworkErrorPopUp'
 
 export const AuthContext = createContext({})
@@ -15,11 +22,11 @@ export default function AuthContextProvider(props) {
 
   /**
    * in the case of a network error, process and display error dialog
-   * @param {string} errorMessage - optional error message returned
+   * @param {string|Error} error - initial error message message or object
    * @param {number} httpCode - http code returned
    */
-  function processError(errorMessage, httpCode=0) {
-    processNetworkError(errorMessage, httpCode, null, null, setNetworkError, null, null )
+  function processError(error, httpCode=0) {
+    processNetworkError(error, httpCode, null, null, setNetworkError, null, null )
   }
 
   const myAuthStore = localforage.createInstance({
@@ -31,7 +38,7 @@ export default function AuthContextProvider(props) {
     const auth = await myAuthStore.getItem('authentication')
 
     if (auth) { // verify that auth is still valid
-      fetch('https://git.door43.org/api/v1/user', { ...auth.config })
+      doFetch('https://git.door43.org/api/v1/user', auth, HTTP_GET_MAX_WAIT_TIME)
         .then(response => {
           const httpCode = response?.status || 0
 
@@ -47,7 +54,7 @@ export default function AuthContextProvider(props) {
           }
         }).catch(e => {
           console.warn(`TranslationSettings - hard error fetching user info, error=`, e)
-          processError('Unknown networking error')
+          processError(e)
         })
     }
     return auth
@@ -100,6 +107,7 @@ export default function AuthContextProvider(props) {
         config={{
           server: BASE_URL,
           tokenid: TOKEN_ID,
+          timeout: HTTP_GET_MAX_WAIT_TIME,
         }}
         authentication={authentication}
         onAuthentication={setAuthentication}

src/hooks/useUserLocalStorage.js

@@ -65,8 +65,7 @@ export function setUserItem(key, currentValue, setState, newValue, username) {
   // Allow value to be a function so we have same API as useState
   const valueToStore =
     newValue instanceof Function ? newValue(currentValue) : newValue
-  const valueToStoreStr = JSON.stringify(valueToStore)
-  localStorage.setItem(key_, valueToStoreStr)
+  setLocalStorageValue(key_, valueToStore)
   setState && setState(valueToStore)
 }
 
@@ -81,7 +80,7 @@ export function setUserItem(key, currentValue, setState, newValue, username) {
  */
 export function readUserItem(key, currentValue, setState, initialValue, username) {
   const key_ = getUserKey(username, key)
-  let savedValue = getUserItem(key_)
+  let savedValue = getLocalStorageItem(key_)
 
   if (savedValue === null) {
     savedValue = initialValue
@@ -102,17 +101,32 @@ export function readUserItem(key, currentValue, setState, initialValue, username
  * @param {string} key - key for item
  * @return {any}
  */
-export function getUserItem(key) {
-  let savedValue = localStorage.getItem(key)
+export function getLocalStorageItem(key) {
+  if (typeof window !== 'undefined') {
+    let savedValue = localStorage.getItem(key)
 
-  if (savedValue !== null) {
-    try {
-      savedValue = JSON.parse(savedValue)
-    } catch {
-      savedValue = null // if not parsable
+    if (savedValue !== null) {
+      try {
+        savedValue = JSON.parse(savedValue)
+      } catch {
+        savedValue = null // if not parsable
+      }
     }
+    return savedValue
+  }
+  return null
+}
+
+/**
+ * saves item to local storage
+ * @param {string} key - key for item
+ * @param {any} value - value to save
+ */
+export function setLocalStorageValue(key, value) {
+  if (typeof window !== 'undefined') {
+    const valueToStoreStr = JSON.stringify(value)
+    localStorage.setItem(key, valueToStoreStr)
   }
-  return savedValue
 }
 
 export default useUserLocalStorage