fix: better error handling for api calls

frontend/src/lib/__tests__/api.test.ts

@@ -7,26 +7,37 @@ import {
   updateSettings,
   getImapFolders,
   testImapConnection,
+  processEmails,
   getFeedUrl,
   NewsletterCreate,
   NewsletterUpdate,
   SettingsCreate,
 } from "../api"
+import { toast } from "sonner"
 
 // Mock the global fetch function
 global.fetch = jest.fn()
 
-const mockFetch = (data: any, ok = true) => { // eslint-disable-line @typescript-eslint/no-explicit-any
+// Mock the toast object
+jest.mock("sonner", () => ({
+  toast: {
+    error: jest.fn(),
+  },
+}))
+
+const mockFetch = (data: any, ok = true, statusText = "OK") => { // eslint-disable-line @typescript-eslint/no-explicit-any
   ;(fetch as jest.Mock).mockResolvedValueOnce({
     ok,
     json: () => Promise.resolve(data),
+    statusText,
   })
 }
 
-const mockFetchError = (data: any = {}) => { // eslint-disable-line @typescript-eslint/no-explicit-any
+const mockFetchError = (data: any = {}, statusText = "Bad Request") => { // eslint-disable-line @typescript-eslint/no-explicit-any
   ;(fetch as jest.Mock).mockResolvedValueOnce({
     ok: false,
     json: () => Promise.resolve(data),
+    statusText,
   })
 }
 
@@ -36,6 +47,7 @@ describe("API Functions", () => {
   beforeEach(() => {
     // Reset the mock before each test
     ;(fetch as jest.Mock).mockClear()
+    ;(toast.error as jest.Mock).mockClear()
   })
 
   describe("getNewsletters", () => {
@@ -48,18 +60,26 @@ describe("API Functions", () => {
 
       const newsletters = await getNewsletters()
       expect(newsletters).toEqual(mockNewsletters)
-      expect(fetch).toHaveBeenCalledWith(`${API_BASE_URL}/newsletters`)
+      expect(fetch).toHaveBeenCalledWith(`${API_BASE_URL}/newsletters`, {})
+      expect(toast.error).not.toHaveBeenCalled()
     })
 
-    it("should throw an error if fetching newsletters fails", async () => {
+    it("should throw an error and show toast if fetching newsletters fails with HTTP error", async () => {
-      mockFetchError()
+      mockFetchError({}, "Not Found")
-      await expect(getNewsletters()).rejects.toThrow("Failed to fetch newsletters")
+      await expect(getNewsletters()).rejects.toThrow("Failed to fetch newsletters: Not Found")
+      expect(toast.error).toHaveBeenCalledWith("Failed to fetch newsletters: Not Found")
+    })
+
+    it("should throw an error and show toast if fetching newsletters fails with network error", async () => {
+      ;(fetch as jest.Mock).mockRejectedValueOnce(new TypeError("Network request failed"))
+      await expect(getNewsletters()).rejects.toThrow("Network request failed")
+      expect(toast.error).toHaveBeenCalledWith("Network error: Could not connect to the backend.")
     })
   })
 
   describe("createNewsletter", () => {
     it("should create a newsletter successfully", async () => {
-      const newNewsletter: NewsletterCreate = { name: "New Newsletter", sender_emails: ["test@example.com"] }
+      const newNewsletter: NewsletterCreate = { name: "New Newsletter", sender_emails: ["test@example.com"], extract_content: false }
       const createdNewsletter = {
         id: 3,
         ...newNewsletter,
@@ -76,18 +96,27 @@ describe("API Functions", () => {
         headers: { "Content-Type": "application/json" },
         body: JSON.stringify(newNewsletter),
       })
+      expect(toast.error).not.toHaveBeenCalled()
     })
 
-    it("should throw an error if creating newsletter fails", async () => {
+    it("should throw an error and show toast if creating newsletter fails with HTTP error", async () => {
-      const newNewsletter: NewsletterCreate = { name: "New Newsletter", sender_emails: [] }
+      const newNewsletter: NewsletterCreate = { name: "New Newsletter", sender_emails: [], extract_content: false }
-      mockFetchError()
+      mockFetchError({}, "Conflict")
-      await expect(createNewsletter(newNewsletter)).rejects.toThrow("Failed to create newsletter")
+      await expect(createNewsletter(newNewsletter)).rejects.toThrow("Failed to create newsletter: Conflict")
+      expect(toast.error).toHaveBeenCalledWith("Failed to create newsletter: Conflict")
+    })
+
+    it("should throw an error and show toast if creating newsletter fails with network error", async () => {
+      const newNewsletter: NewsletterCreate = { name: "New Newsletter", sender_emails: [], extract_content: false }
+      ;(fetch as jest.Mock).mockRejectedValueOnce(new TypeError("Network request failed"))
+      await expect(createNewsletter(newNewsletter)).rejects.toThrow("Network request failed")
+      expect(toast.error).toHaveBeenCalledWith("Network error: Could not connect to the backend.")
     })
   })
 
   describe("updateNewsletter", () => {
     it("should update a newsletter successfully", async () => {
-      const updatedNewsletter: NewsletterUpdate = { name: "Updated Newsletter", sender_emails: ["updated@example.com"] }
+      const updatedNewsletter: NewsletterUpdate = { name: "Updated Newsletter", sender_emails: ["updated@example.com"], extract_content: true }
       const newsletterId = 1
       const returnedNewsletter = {
         id: newsletterId,
@@ -105,13 +134,23 @@ describe("API Functions", () => {
         headers: { "Content-Type": "application/json" },
         body: JSON.stringify(updatedNewsletter),
       })
+      expect(toast.error).not.toHaveBeenCalled()
     })
 
-    it("should throw an error if updating newsletter fails", async () => {
+    it("should throw an error and show toast if updating newsletter fails with HTTP error", async () => {
-      const updatedNewsletter: NewsletterUpdate = { name: "Updated Newsletter", sender_emails: [] }
+      const updatedNewsletter: NewsletterUpdate = { name: "Updated Newsletter", sender_emails: [], extract_content: true }
       const newsletterId = 1
-      mockFetchError()
+      mockFetchError({}, "Bad Request")
-      await expect(updateNewsletter(newsletterId, updatedNewsletter)).rejects.toThrow("Failed to update newsletter")
+      await expect(updateNewsletter(newsletterId, updatedNewsletter)).rejects.toThrow("Failed to update newsletter: Bad Request")
+      expect(toast.error).toHaveBeenCalledWith("Failed to update newsletter: Bad Request")
+    })
+
+    it("should throw an error and show toast if updating newsletter fails with network error", async () => {
+      const updatedNewsletter: NewsletterUpdate = { name: "Updated Newsletter", sender_emails: [], extract_content: true }
+      const newsletterId = 1
+      ;(fetch as jest.Mock).mockRejectedValueOnce(new TypeError("Network request failed"))
+      await expect(updateNewsletter(newsletterId, updatedNewsletter)).rejects.toThrow("Network request failed")
+      expect(toast.error).toHaveBeenCalledWith("Network error: Could not connect to the backend.")
     })
   })
 
@@ -124,12 +163,21 @@ describe("API Functions", () => {
       expect(fetch).toHaveBeenCalledWith(`${API_BASE_URL}/newsletters/${newsletterId}`, {
         method: "DELETE",
       })
+      expect(toast.error).not.toHaveBeenCalled()
     })
 
-    it("should throw an error if deleting newsletter fails", async () => {
+    it("should throw an error and show toast if deleting newsletter fails with HTTP error", async () => {
       const newsletterId = 1
-      mockFetchError()
+      mockFetchError({}, "Forbidden")
-      await expect(deleteNewsletter(newsletterId)).rejects.toThrow("Failed to delete newsletter")
+      await expect(deleteNewsletter(newsletterId)).rejects.toThrow("Failed to delete newsletter: Forbidden")
+      expect(toast.error).toHaveBeenCalledWith("Failed to delete newsletter: Forbidden")
+    })
+
+    it("should throw an error and show toast if deleting newsletter fails with network error", async () => {
+      const newsletterId = 1
+      ;(fetch as jest.Mock).mockRejectedValueOnce(new TypeError("Network request failed"))
+      await expect(deleteNewsletter(newsletterId)).rejects.toThrow("Network request failed")
+      expect(toast.error).toHaveBeenCalledWith("Network error: Could not connect to the backend.")
     })
   })
 
@@ -150,12 +198,20 @@ describe("API Functions", () => {
 
       const settings = await getSettings()
       expect(settings).toEqual(mockSettings)
-      expect(fetch).toHaveBeenCalledWith(`${API_BASE_URL}/imap/settings`)
+      expect(fetch).toHaveBeenCalledWith(`${API_BASE_URL}/imap/settings`, {})
+      expect(toast.error).not.toHaveBeenCalled()
     })
 
-    it("should throw an error if fetching settings fails", async () => {
+    it("should throw an error and show toast if fetching settings fails with HTTP error", async () => {
-      mockFetchError()
+      mockFetchError({}, "Unauthorized")
-      await expect(getSettings()).rejects.toThrow("Failed to fetch settings")
+      await expect(getSettings()).rejects.toThrow("Failed to fetch settings: Unauthorized")
+      expect(toast.error).toHaveBeenCalledWith("Failed to fetch settings: Unauthorized")
+    })
+
+    it("should throw an error and show toast if fetching settings fails with network error", async () => {
+      ;(fetch as jest.Mock).mockRejectedValueOnce(new TypeError("Network request failed"))
+      await expect(getSettings()).rejects.toThrow("Network request failed")
+      expect(toast.error).toHaveBeenCalledWith("Network error: Could not connect to the backend.")
     })
   })
 
@@ -181,9 +237,10 @@ describe("API Functions", () => {
         headers: { "Content-Type": "application/json" },
         body: JSON.stringify(newSettings),
       })
+      expect(toast.error).not.toHaveBeenCalled()
     })
 
-    it("should throw an error if updating settings fails", async () => {
+    it("should throw an error and show toast if updating settings fails with HTTP error", async () => {
       const newSettings: SettingsCreate = {
         imap_server: "new.imap.com",
         imap_username: "newuser@example.com",
@@ -192,8 +249,23 @@ describe("API Functions", () => {
         email_check_interval: 120,
         auto_add_new_senders: true,
       }
-      mockFetchError()
+      mockFetchError({}, "Internal Server Error")
-      await expect(updateSettings(newSettings)).rejects.toThrow("Failed to update settings")
+      await expect(updateSettings(newSettings)).rejects.toThrow("Failed to update settings: Internal Server Error")
+      expect(toast.error).toHaveBeenCalledWith("Failed to update settings: Internal Server Error")
+    })
+
+    it("should throw an error and show toast if updating settings fails with network error", async () => {
+      const newSettings: SettingsCreate = {
+        imap_server: "new.imap.com",
+        imap_username: "newuser@example.com",
+        search_folder: "Archive",
+        mark_as_read: false,
+        email_check_interval: 120,
+        auto_add_new_senders: true,
+      }
+      ;(fetch as jest.Mock).mockRejectedValueOnce(new TypeError("Network request failed"))
+      await expect(updateSettings(newSettings)).rejects.toThrow("Network request failed")
+      expect(toast.error).toHaveBeenCalledWith("Network error: Could not connect to the backend.")
     })
   })
 
@@ -204,13 +276,22 @@ describe("API Functions", () => {
 
       const folders = await getImapFolders()
       expect(folders).toEqual(mockFolders)
-      expect(fetch).toHaveBeenCalledWith(`${API_BASE_URL}/imap/folders`)
+      expect(fetch).toHaveBeenCalledWith(`${API_BASE_URL}/imap/folders`, {})
+      expect(toast.error).not.toHaveBeenCalled()
     })
 
-    it("should return an empty array if fetching IMAP folders fails", async () => {
+    it("should return an empty array and show toast if fetching IMAP folders fails with HTTP error", async () => {
-      mockFetchError()
+      mockFetchError({}, "Forbidden")
       const folders = await getImapFolders()
       expect(folders).toEqual([])
+      expect(toast.error).toHaveBeenCalledWith("Failed to fetch IMAP folders: Forbidden")
+    })
+
+    it("should return an empty array and show toast if fetching IMAP folders fails with network error", async () => {
+      ;(fetch as jest.Mock).mockRejectedValueOnce(new TypeError("Network request failed"))
+      const folders = await getImapFolders()
+      expect(folders).toEqual([])
+      expect(toast.error).toHaveBeenCalledWith("Network error: Could not connect to the backend.")
     })
   })
 
@@ -224,17 +305,59 @@ describe("API Functions", () => {
       expect(fetch).toHaveBeenCalledWith(`${API_BASE_URL}/imap/test`, {
         method: "POST",
       })
+      expect(toast.error).not.toHaveBeenCalled()
     })
 
-    it("should throw an error with detail if testing IMAP connection fails", async () => {
+    it("should throw an error with detail and show toast if testing IMAP connection fails with HTTP error", async () => {
       const errorMessage = "Invalid credentials"
-      mockFetchError({ detail: errorMessage })
+      mockFetchError({ detail: errorMessage }, "Unauthorized")
       await expect(testImapConnection()).rejects.toThrow(errorMessage)
+      expect(toast.error).toHaveBeenCalledWith(errorMessage)
     })
 
-    it("should throw a generic error if testing IMAP connection fails without detail", async () => {
+    it("should throw a generic error and show toast if testing IMAP connection fails without detail with HTTP error", async () => {
-      mockFetchError()
+      mockFetchError({}, "Bad Gateway")
-      await expect(testImapConnection()).rejects.toThrow("Failed to test IMAP connection")
+      await expect(testImapConnection()).rejects.toThrow("Failed to test IMAP connection: Bad Gateway")
+      expect(toast.error).toHaveBeenCalledWith("Failed to test IMAP connection: Bad Gateway")
+    })
+
+    it("should throw an error and show toast if testing IMAP connection fails with network error", async () => {
+      ;(fetch as jest.Mock).mockRejectedValueOnce(new TypeError("Network request failed"))
+      await expect(testImapConnection()).rejects.toThrow("Network request failed")
+      expect(toast.error).toHaveBeenCalledWith("Network error: Could not connect to the backend.")
+    })
+  })
+
+  describe("processEmails", () => {
+    it("should process emails successfully", async () => {
+      const mockResponse = { message: "Emails processed" }
+      mockFetch(mockResponse)
+
+      const result = await processEmails()
+      expect(result).toEqual(mockResponse)
+      expect(fetch).toHaveBeenCalledWith(`${API_BASE_URL}/imap/process`, {
+        method: "POST",
+      })
+      expect(toast.error).not.toHaveBeenCalled()
+    })
+
+    it("should throw an error with detail and show toast if processing emails fails with HTTP error", async () => {
+      const errorMessage = "IMAP not configured"
+      mockFetchError({ detail: errorMessage }, "Bad Request")
+      await expect(processEmails()).rejects.toThrow(errorMessage)
+      expect(toast.error).toHaveBeenCalledWith(errorMessage)
+    })
+
+    it("should throw a generic error and show toast if processing emails fails without detail with HTTP error", async () => {
+      mockFetchError({}, "Service Unavailable")
+      await expect(processEmails()).rejects.toThrow("Failed to process emails: Service Unavailable")
+      expect(toast.error).toHaveBeenCalledWith("Failed to process emails: Service Unavailable")
+    })
+
+    it("should throw an error and show toast if processing emails fails with network error", async () => {
+      ;(fetch as jest.Mock).mockRejectedValueOnce(new TypeError("Network request failed"))
+      await expect(processEmails()).rejects.toThrow("Network request failed")
+      expect(toast.error).toHaveBeenCalledWith("Network error: Could not connect to the backend.")
     })
   })
 

frontend/src/lib/api.ts

@@ -1,5 +1,3 @@
-// frontend/src/lib/api.ts
-
 const API_BASE_URL = process.env.NEXT_PUBLIC_API_URL;
 
 export interface Sender {
@@ -53,102 +51,102 @@ export interface SettingsCreate {
 }
 
 
-export async function getNewsletters(): Promise<Newsletter[]> {
+import { toast } from "sonner";
-    const response = await fetch(`${API_BASE_URL}/newsletters`);
+
-    if (!response.ok) {
+async function fetcher<T>(
-        throw new Error("Failed to fetch newsletters");
+    url: string,
+    options: RequestInit = {},
+    errorMessagePrefix: string,
+    returnEmptyArrayOnFailure: boolean = false
+): Promise<T> {
+    try {
+        const response = await fetch(url, options);
+        if (!response.ok) {
+            let errorText = `${errorMessagePrefix}: ${response.statusText}`;
+            try {
+                const errorData = await response.json();
+                if (errorData.detail) {
+                    errorText = errorData.detail;
+                }
+            } catch (e) { // eslint-disable-line @typescript-eslint/no-unused-vars
+                // ignore error if response is not JSON
+            }
+            toast.error(errorText);
+            if (returnEmptyArrayOnFailure) {
+                return [] as T;
+            }
+            throw new Error(errorText);
+        }
+        return response.json();
+    } catch (error) {
+        if (error instanceof TypeError) {
+            toast.error("Network error: Could not connect to the backend.");
+        }
+        if (returnEmptyArrayOnFailure) {
+            return [] as T;
+        }
+        throw error;
     }
-    return response.json();
+}
+
+export async function getNewsletters(): Promise<Newsletter[]> {
+    return fetcher<Newsletter[]>(`${API_BASE_URL}/newsletters`, {}, "Failed to fetch newsletters");
 }
 
 export async function createNewsletter(newsletter: NewsletterCreate): Promise<Newsletter> {
-    const response = await fetch(`${API_BASE_URL}/newsletters`, {
+    return fetcher<Newsletter>(`${API_BASE_URL}/newsletters`, {
         method: 'POST',
         headers: {
             'Content-Type': 'application/json',
         },
         body: JSON.stringify(newsletter),
-    });
+    }, "Failed to create newsletter");
-    if (!response.ok) {
-        throw new Error("Failed to create newsletter");
-    }
-    return response.json();
 }
 
 export async function updateNewsletter(id: number, newsletter: NewsletterUpdate): Promise<Newsletter> {
-    const response = await fetch(`${API_BASE_URL}/newsletters/${id}`, {
+    return fetcher<Newsletter>(`${API_BASE_URL}/newsletters/${id}`, {
         method: 'PUT',
         headers: {
             'Content-Type': 'application/json',
         },
         body: JSON.stringify(newsletter),
-    });
+    }, "Failed to update newsletter");
-    if (!response.ok) {
-        throw new Error("Failed to update newsletter");
-    }
-    return response.json();
 }
 
 export async function deleteNewsletter(id: number): Promise<void> {
-    const response = await fetch(`${API_BASE_URL}/newsletters/${id}`, {
+    await fetcher<void>(`${API_BASE_URL}/newsletters/${id}`, {
         method: 'DELETE',
-    });
+    }, "Failed to delete newsletter");
-    if (!response.ok) {
-        throw new Error("Failed to delete newsletter");
-    }
 }
 
 export async function getSettings(): Promise<Settings> {
-    const response = await fetch(`${API_BASE_URL}/imap/settings`);
+    return fetcher<Settings>(`${API_BASE_URL}/imap/settings`, {}, "Failed to fetch settings");
-    if (!response.ok) {
-        throw new Error("Failed to fetch settings");
-    }
-    return response.json();
 }
 
 export async function updateSettings(settings: SettingsCreate): Promise<Settings> {
-    const response = await fetch(`${API_BASE_URL}/imap/settings`, {
+    return fetcher<Settings>(`${API_BASE_URL}/imap/settings`, {
         method: 'POST',
         headers: {
             'Content-Type': 'application/json',
         },
         body: JSON.stringify(settings),
-    });
+    }, "Failed to update settings");
-    if (!response.ok) {
-        throw new Error("Failed to update settings");
-    }
-    return response.json();
 }
 
 export async function getImapFolders(): Promise<string[]> {
-    const response = await fetch(`${API_BASE_URL}/imap/folders`);
+    return fetcher<string[]>(`${API_BASE_URL}/imap/folders`, {}, "Failed to fetch IMAP folders", true);
-    // If it fails, it's probably because settings are not configured. Return empty array.
-    if (!response.ok) {
-        return [];
-    }
-    return response.json();
 }
 
 export async function testImapConnection(): Promise<{ message: string }> {
-    const response = await fetch(`${API_BASE_URL}/imap/test`, {
+    return fetcher<{ message: string }>(`${API_BASE_URL}/imap/test`, {
         method: 'POST',
-    });
+    }, "Failed to test IMAP connection");
-    if (!response.ok) {
-        const error = await response.json();
-        throw new Error(error.detail || "Failed to test IMAP connection");
-    }
-    return response.json();
 }
 
 export async function processEmails(): Promise<{ message: string }> {
-    const response = await fetch(`${API_BASE_URL}/imap/process`, {
+    return fetcher<{ message: string }>(`${API_BASE_URL}/imap/process`, {
         method: 'POST',
-    });
+    }, "Failed to process emails");
-    if (!response.ok) {
-        const error = await response.json();
-        throw new Error(error.detail || "Failed to process emails");
-    }
-    return response.json();
 }
 
 export function getFeedUrl(newsletterId: number): string {