Better error handling

Netflix · Jul 27, 2023 · 8df82eb · 8df82eb
1 parent 997f3af
commit 8df82eb
Show file tree

Hide file tree

Showing 4 changed files with 27 additions and 8 deletions.
diff --git a/src/components/MFCard/CardIframe.tsx b/src/components/MFCard/CardIframe.tsx
@@ -1,4 +1,4 @@
-import React, { useEffect, useRef, useState } from 'react';
+import React, { ReactEventHandler, useEffect, useRef, useState } from 'react';
 import styled from 'styled-components';
 import { MESSAGE_NAME } from '../Plugins/PluginManager';
 import { apiHttp } from '../../constants';
@@ -76,7 +76,8 @@ const CardIframe: React.FC<Props> = ({ path }) => {
     setLoading(false);
   };
 
-  const handleIframeError = () => {
+  const handleIframeError: ReactEventHandler<HTMLIFrameElement> = (e) => {
+    console.error('Error loading card', e);
     setError(true);
   };
 

diff --git a/src/components/MFCard/useTaskCards.ts b/src/components/MFCard/useTaskCards.ts
@@ -72,7 +72,7 @@ export default function useTaskCards(task: Task | null, decorators: Decorator[])
       // We want to invalidate cache when polling since cache would return old results.
       // First request will be without invalidate and if that returns us all expected cards
       // we don't need to poll and invalidate.
-      return fetch(`${apiHttp(path)}${invalidate ? '?invalidate=true' : ''}`)
+      fetch(`${apiHttp(path)}${invalidate ? '?invalidate=true' : ''}`)
         .then((result) => result.json())
         .then((result: DataModel<CardDefinition[]>) => {
           if (result.status === 200) {
@@ -87,7 +87,11 @@ export default function useTaskCards(task: Task | null, decorators: Decorator[])
           }
         })
         .catch((e) => {
-          console.warn('Cards request failed for ', apiHttp(path), e);
+          console.error('Cards request failed for ', apiHttp(path), e);
+          setTaskCards((prev) => ({
+            ...prev,
+            [path]: { cards: prev[path]?.cards ?? [], status: 'error' },
+          }));
         })
         .finally(() => {
           setPoll(true);
@@ -109,7 +113,7 @@ export default function useTaskCards(task: Task | null, decorators: Decorator[])
         if (taskCards[url]?.status !== 'success') {
           setTaskCards((prev) => ({
             ...prev,
-            [url]: { ...prev[url], status: 'success' },
+            [url]: { cards: prev[url]?.cards ?? [], status: 'success' },
           }));
         }
         // If the timeout has been reached
@@ -118,15 +122,15 @@ export default function useTaskCards(task: Task | null, decorators: Decorator[])
         if (taskCards[url]?.status !== 'timeout') {
           setTaskCards((prev) => ({
             ...prev,
-            [url]: { ...prev[url], status: 'timeout' },
+            [url]: { cards: prev[url]?.cards ?? [], status: 'timeout' },
           }));
         }
       } else {
         // Otherwise set the status to loading and continue polling
         if (taskCards[url]?.status !== 'loading') {
           setTaskCards((prev) => ({
             ...prev,
-            [url]: { ...prev[url], status: 'loading' },
+            [url]: { cards: prev[url]?.cards ?? [], status: 'loading' },
           }));
         }
         t = window.setTimeout(() => {

diff --git a/src/pages/Task/index.tsx b/src/pages/Task/index.tsx
@@ -401,7 +401,7 @@ const Task: React.FC<TaskViewProps> = ({
                     ]
                   : []),
                 // Render cards at the end of sections if enabled by feature flags.
-                ...(showCards
+                ...(showCards && cardsResult?.cards?.length
                   ? cardsResult?.cards?.map((def) => ({
                       key: def.hash,
                       order: 99,
@@ -466,6 +466,19 @@ const Task: React.FC<TaskViewProps> = ({
                       ]
                     : []
                   : []),
+                // Show error if cards were not fetched before timeout
+                ...(showCards
+                  ? cardsResult?.status === 'error'
+                    ? [
+                        {
+                          key: 'card_error',
+                          order: 99,
+                          label: t('card.card_error'),
+                          component: <></>,
+                        },
+                      ]
+                    : []
+                  : []),
               ].sort((a, b) => a.order - b.order)}
             />
           </>

diff --git a/src/translations/en.ts b/src/translations/en.ts
@@ -330,6 +330,7 @@ const en = {
       download_card: 'Download card HTML file',
       link_card: 'Link to card',
       card_timeout: 'Timeout: loading cards',
+      card_error: 'Error: loading cards',
       card_loading: 'Loading cards',
     },
   },