From e915330a785842381e8d8e98a1c35f31999bb0d5 Mon Sep 17 00:00:00 2001 From: Leon Date: Thu, 17 Jul 2025 10:24:21 +0200 Subject: [PATCH] fix: better error handling for api calls --- frontend/src/lib/__tests__/api.test.ts | 191 ++++++++++++++++++++----- frontend/src/lib/api.ts | 108 +++++++------- 2 files changed, 210 insertions(+), 89 deletions(-) diff --git a/frontend/src/lib/__tests__/api.test.ts b/frontend/src/lib/__tests__/api.test.ts index 4701c46..744158b 100644 --- a/frontend/src/lib/__tests__/api.test.ts +++ b/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 () => { - mockFetchError() - await expect(getNewsletters()).rejects.toThrow("Failed to fetch newsletters") + it("should throw an error and show toast if fetching newsletters fails with HTTP error", async () => { + mockFetchError({}, "Not Found") + 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 () => { - const newNewsletter: NewsletterCreate = { name: "New Newsletter", sender_emails: [] } - mockFetchError() - await expect(createNewsletter(newNewsletter)).rejects.toThrow("Failed to create newsletter") + it("should throw an error and show toast if creating newsletter fails with HTTP error", async () => { + const newNewsletter: NewsletterCreate = { name: "New Newsletter", sender_emails: [], extract_content: false } + mockFetchError({}, "Conflict") + 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 () => { - const updatedNewsletter: NewsletterUpdate = { name: "Updated Newsletter", sender_emails: [] } + it("should throw an error and show toast if updating newsletter fails with HTTP error", async () => { + const updatedNewsletter: NewsletterUpdate = { name: "Updated Newsletter", sender_emails: [], extract_content: true } const newsletterId = 1 - mockFetchError() - await expect(updateNewsletter(newsletterId, updatedNewsletter)).rejects.toThrow("Failed to update newsletter") + mockFetchError({}, "Bad Request") + 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() - await expect(deleteNewsletter(newsletterId)).rejects.toThrow("Failed to delete newsletter") + mockFetchError({}, "Forbidden") + 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 () => { - mockFetchError() - await expect(getSettings()).rejects.toThrow("Failed to fetch settings") + it("should throw an error and show toast if fetching settings fails with HTTP error", async () => { + mockFetchError({}, "Unauthorized") + 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() - await expect(updateSettings(newSettings)).rejects.toThrow("Failed to update settings") + mockFetchError({}, "Internal Server Error") + 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 () => { - mockFetchError() + it("should return an empty array and show toast if fetching IMAP folders fails with HTTP error", async () => { + 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 () => { - mockFetchError() - await expect(testImapConnection()).rejects.toThrow("Failed to test IMAP connection") + it("should throw a generic error and show toast if testing IMAP connection fails without detail with HTTP error", async () => { + mockFetchError({}, "Bad Gateway") + 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.") }) }) diff --git a/frontend/src/lib/api.ts b/frontend/src/lib/api.ts index d0758e0..fde6406 100644 --- a/frontend/src/lib/api.ts +++ b/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 { - const response = await fetch(`${API_BASE_URL}/newsletters`); - if (!response.ok) { - throw new Error("Failed to fetch newsletters"); +import { toast } from "sonner"; + +async function fetcher( + url: string, + options: RequestInit = {}, + errorMessagePrefix: string, + returnEmptyArrayOnFailure: boolean = false +): Promise { + 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 { + return fetcher(`${API_BASE_URL}/newsletters`, {}, "Failed to fetch newsletters"); } export async function createNewsletter(newsletter: NewsletterCreate): Promise { - const response = await fetch(`${API_BASE_URL}/newsletters`, { + return fetcher(`${API_BASE_URL}/newsletters`, { method: 'POST', headers: { 'Content-Type': 'application/json', }, body: JSON.stringify(newsletter), - }); - if (!response.ok) { - throw new Error("Failed to create newsletter"); - } - return response.json(); + }, "Failed to create newsletter"); } export async function updateNewsletter(id: number, newsletter: NewsletterUpdate): Promise { - const response = await fetch(`${API_BASE_URL}/newsletters/${id}`, { + return fetcher(`${API_BASE_URL}/newsletters/${id}`, { method: 'PUT', headers: { 'Content-Type': 'application/json', }, body: JSON.stringify(newsletter), - }); - if (!response.ok) { - throw new Error("Failed to update newsletter"); - } - return response.json(); + }, "Failed to update newsletter"); } export async function deleteNewsletter(id: number): Promise { - const response = await fetch(`${API_BASE_URL}/newsletters/${id}`, { + await fetcher(`${API_BASE_URL}/newsletters/${id}`, { method: 'DELETE', - }); - if (!response.ok) { - throw new Error("Failed to delete newsletter"); - } + }, "Failed to delete newsletter"); } export async function getSettings(): Promise { - const response = await fetch(`${API_BASE_URL}/imap/settings`); - if (!response.ok) { - throw new Error("Failed to fetch settings"); - } - return response.json(); + return fetcher(`${API_BASE_URL}/imap/settings`, {}, "Failed to fetch settings"); } export async function updateSettings(settings: SettingsCreate): Promise { - const response = await fetch(`${API_BASE_URL}/imap/settings`, { + return fetcher(`${API_BASE_URL}/imap/settings`, { method: 'POST', headers: { 'Content-Type': 'application/json', }, body: JSON.stringify(settings), - }); - if (!response.ok) { - throw new Error("Failed to update settings"); - } - return response.json(); + }, "Failed to update settings"); } export async function getImapFolders(): Promise { - const response = await fetch(`${API_BASE_URL}/imap/folders`); - // If it fails, it's probably because settings are not configured. Return empty array. - if (!response.ok) { - return []; - } - return response.json(); + return fetcher(`${API_BASE_URL}/imap/folders`, {}, "Failed to fetch IMAP folders", true); } 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', - }); - if (!response.ok) { - const error = await response.json(); - throw new Error(error.detail || "Failed to test IMAP connection"); - } - return response.json(); + }, "Failed to test IMAP connection"); } 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', - }); - if (!response.ok) { - const error = await response.json(); - throw new Error(error.detail || "Failed to process emails"); - } - return response.json(); + }, "Failed to process emails"); } export function getFeedUrl(newsletterId: number): string {