feat: Improve error handling and user feedback

bburda · bburda · commit 4bf61b052d11 · 2025-12-02T17:56:40.000Z
- Add toast notifications for API failures (entity details, children loading)
- Display fallback UI panel when entity details fail to load instead of empty state
- Infer entity type from path structure for better error context
- Add explicit error property to SovdEntityDetails interface
- Improve error messages: include details for HTTP errors, friendly JSON validation
- Remove unnecessary console.error calls and try/catch wrappers
- Fix duplicate toast on auto-connect failure (React Strict Mode)
- Add request timeout handling with user-friendly messages
diff --git a/src/App.tsx b/src/App.tsx
@@ -1,4 +1,4 @@
-import { useState, useEffect } from 'react';
+import { useState, useEffect, useRef } from 'react';
 import { useShallow } from 'zustand/shallow';
 import { ToastContainer, toast } from 'react-toastify';
 import 'react-toastify/dist/ReactToastify.css';
@@ -18,17 +18,18 @@ function App() {
     );
 
     const [showConnectionDialog, setShowConnectionDialog] = useState(false);
+    const autoConnectAttempted = useRef(false);
 
     // Auto-connect on mount if we have a stored URL
     useEffect(() => {
-        if (serverUrl && !isConnected) {
-            Promise.resolve(connect(serverUrl, baseEndpoint))
-                .catch((err) => {
-                    toast.error(
-                        `Auto-connect failed: ${err?.message || 'Please check your server settings.'}`
-                    );
+        if (serverUrl && !isConnected && !autoConnectAttempted.current) {
+            autoConnectAttempted.current = true;
+            connect(serverUrl, baseEndpoint).then((success) => {
+                if (!success) {
+                    toast.error('Auto-connect failed. Please check your server settings.');
                     setShowConnectionDialog(true);
-                });
+                }
+            });
         }
         // eslint-disable-next-line react-hooks/exhaustive-deps
     }, []);
diff --git a/src/components/EntityDetailPanel.tsx b/src/components/EntityDetailPanel.tsx
@@ -58,7 +58,14 @@ export function EntityDetailPanel({ onConnectClick }: EntityDetailPanelProps) {
         setPublishingTopics(prev => new Set(prev).add(topic.topic));
 
         try {
-            const data = JSON.parse(inputData);
+            let data: unknown;
+            try {
+                data = JSON.parse(inputData);
+            } catch {
+                toast.error('Invalid JSON format. Please check your message data.');
+                return;
+            }
+            
             const messageType = inferMessageType(topic.data);
 
             await client.publishToComponentTopic(selectedEntity.id, topicName, {
diff --git a/src/lib/sovd-api.ts b/src/lib/sovd-api.ts
@@ -86,100 +86,88 @@ export class SovdApiClient {
    * @param path Optional path to get children of (e.g., "/devices/robot1")
    */
   async getEntities(path?: string): Promise<SovdEntity[]> {
-    try {
-      // Root level -> fetch areas
-      if (!path || path === '/') {
-        const response = await fetchWithTimeout(this.getUrl('areas'), {
-          method: 'GET',
-          headers: { 'Accept': 'application/json' },
-        });
-
-        if (!response.ok) throw new Error(`HTTP ${response.status}`);
-        const areas = await response.json();
-
-        return areas.map((area: { id: string }) => ({
-          id: area.id,
-          name: area.id,
-          type: 'area',
-          href: `/areas/${area.id}`,
-          hasChildren: true
-        }));
-      }
-
-      // Area level -> fetch components
-      // Path format: /area_id
-      const areaId = path.replace(/^\//, '');
-      // Check if it's a nested path (component inside area)
-      if (areaId.includes('/')) {
-        // We don't support children of components in this simple mapping yet
-        return [];
-      }
-
-      const response = await fetchWithTimeout(this.getUrl(`areas/${areaId}/components`), {
+    // Root level -> fetch areas
+    if (!path || path === '/') {
+      const response = await fetchWithTimeout(this.getUrl('areas'), {
         method: 'GET',
         headers: { 'Accept': 'application/json' },
       });
 
       if (!response.ok) throw new Error(`HTTP ${response.status}`);
-      const components = await response.json();
+      const areas = await response.json();
 
-      return components.map((comp: { id: string }) => ({
-        id: comp.id,
-        name: comp.id,
-        type: 'component',
-        href: `/components/${comp.id}`,
-        hasChildren: false // Components are leaves in this view
+      return areas.map((area: { id: string }) => ({
+        id: area.id,
+        name: area.id,
+        type: 'area',
+        href: `/areas/${area.id}`,
+        hasChildren: true
       }));
+    }
 
-    } catch (error) {
-      console.error(`Failed to fetch entities from ${path || 'root'}:`, error);
-      throw error;
+    // Area level -> fetch components
+    // Path format: /area_id
+    const areaId = path.replace(/^\//, '');
+    // Check if it's a nested path (component inside area)
+    if (areaId.includes('/')) {
+      // We don't support children of components in this simple mapping yet
+      return [];
     }
+
+    const response = await fetchWithTimeout(this.getUrl(`areas/${areaId}/components`), {
+      method: 'GET',
+      headers: { 'Accept': 'application/json' },
+    });
+
+    if (!response.ok) throw new Error(`HTTP ${response.status}`);
+    const components = await response.json();
+
+    return components.map((comp: { id: string }) => ({
+      id: comp.id,
+      name: comp.id,
+      type: 'component',
+      href: `/components/${comp.id}`,
+      hasChildren: false // Components are leaves in this view
+    }));
   }
 
   /**
    * Get detailed information about a specific entity
    * @param path Entity path (e.g., "/area/component")
    */
   async getEntityDetails(path: string): Promise<SovdEntityDetails> {
-    try {
-      // Path comes from the tree, e.g. "/area_id/component_id"
-      const parts = path.split('/').filter(p => p);
-
-      if (parts.length === 2) {
-        const componentId = parts[1];
-        const response = await fetchWithTimeout(this.getUrl(`components/${componentId}/data`), {
-          method: 'GET',
-          headers: { 'Accept': 'application/json' },
-        });
-
-        if (!response.ok) throw new Error(`HTTP ${response.status}`);
-        const topicsData = await response.json() as ComponentTopic[];
-
-        // Return entity details with topics array
-        return {
-          id: componentId,
-          name: componentId,
-          type: 'component',
-          href: path,
-          topics: topicsData,
-        };
-      }
-
-      // If it's an area (length 1), maybe return basic info?
-      // For now return empty object or basic info
+    // Path comes from the tree, e.g. "/area_id/component_id"
+    const parts = path.split('/').filter(p => p);
+
+    if (parts.length === 2) {
+      const componentId = parts[1];
+      const response = await fetchWithTimeout(this.getUrl(`components/${componentId}/data`), {
+        method: 'GET',
+        headers: { 'Accept': 'application/json' },
+      });
+
+      if (!response.ok) throw new Error(`HTTP ${response.status}`);
+      const topicsData = await response.json() as ComponentTopic[];
+
+      // Return entity details with topics array
       return {
-        id: parts[0],
-        name: parts[0],
-        type: 'area',
+        id: componentId,
+        name: componentId,
+        type: 'component',
         href: path,
-        hasChildren: true
+        topics: topicsData,
       };
-
-    } catch (error) {
-      console.error(`Failed to fetch entity details for ${path}:`, error);
-      throw error;
     }
+
+    // If it's an area (length 1), maybe return basic info?
+    // For now return empty object or basic info
+    return {
+      id: parts[0],
+      name: parts[0],
+      type: 'area',
+      href: path,
+      hasChildren: true
+    };
   }
 
   /**
@@ -200,23 +188,18 @@ export class SovdApiClient {
     topicName: string,
     request: ComponentTopicPublishRequest
   ): Promise<void> {
-    try {
-      const response = await fetchWithTimeout(this.getUrl(`components/${componentId}/data/${topicName}`), {
-        method: 'PUT',
-        headers: {
-          'Accept': 'application/json',
-          'Content-Type': 'application/json',
-        },
-        body: JSON.stringify(request),
-      }, 10000); // 10 second timeout for publish
-
-      if (!response.ok) {
-        const errorData = await response.json().catch(() => ({}));
-        throw new Error(errorData.error || `HTTP ${response.status}`);
-      }
-    } catch (error) {
-      console.error(`Failed to publish to ${componentId}/${topicName}:`, error);
-      throw error;
+    const response = await fetchWithTimeout(this.getUrl(`components/${componentId}/data/${topicName}`), {
+      method: 'PUT',
+      headers: {
+        'Accept': 'application/json',
+        'Content-Type': 'application/json',
+      },
+      body: JSON.stringify(request),
+    }, 10000); // 10 second timeout for publish
+
+    if (!response.ok) {
+      const errorData = await response.json().catch(() => ({}));
+      throw new Error(errorData.error || errorData.message || `Server error (HTTP ${response.status})`);
     }
   }
 
diff --git a/src/lib/store.ts b/src/lib/store.ts
@@ -217,16 +217,28 @@ export const useAppStore = create<AppState>()(
         try {
           const details = await client.getEntityDetails(path);
           set({ selectedEntity: details, isLoadingDetails: false });
-        } catch {
-          toast.error(`Failed to load entity details for ${path}`);
+        } catch (error) {
+          const message = error instanceof Error ? error.message : 'Unknown error';
+          toast.error(`Failed to load entity details for ${path}: ${message}`);
 
           // Set fallback entity to allow panel to render
-          const id = path.split('/').pop() || path;
+          // Infer entity type from path structure
+          const segments = path.split('/').filter(Boolean);
+          const id = segments[segments.length - 1] || path;
+          let inferredType: string;
+          if (segments.length === 1) {
+            inferredType = 'area';
+          } else if (segments.length === 2) {
+            inferredType = 'component';
+          } else {
+            inferredType = 'unknown';
+          }
+
           set({
             selectedEntity: {
               id,
               name: id,
-              type: 'component',
+              type: inferredType,
               href: path,
               topics: [],
               error: 'Failed to load details'
diff --git a/src/lib/types.ts b/src/lib/types.ts
@@ -24,6 +24,8 @@ export interface SovdEntity {
 export interface SovdEntityDetails extends SovdEntity {
   /** Topics available for this component */
   topics?: ComponentTopic[];
+  /** Error message if fetching details failed */
+  error?: string;
   /** Additional properties vary by entity type */
   [key: string]: unknown;
 }

Original file line number	Diff line number	Diff line change
`@@ -24,6 +24,8 @@ export interface SovdEntity {`
`24`	`24`	`export interface SovdEntityDetails extends SovdEntity {`
`25`	`25`	`/** Topics available for this component */`
`26`	`26`	`topics?: ComponentTopic[];`
	`27`	`+ /** Error message if fetching details failed */`
	`28`	`+ error?: string;`
`27`	`29`	`/** Additional properties vary by entity type */`
`28`	`30`	`[key: string]: unknown;`
`29`	`31`	`}`