Merge pull request #120 from divyansh-cyber/resolving_issue_119

Fix file upload timeout handling in fetchChatbotReplyWithFiles

Author: cnu1812

frontend/src/api/chatbot.ts

@@ -74,6 +74,7 @@ export const fetchChatbotReply = async (
  * @param sessionId - The session id of the chat
  * @param userMessage - The message input from the user
  * @param files - Array of File objects to upload
+ * @param signal - External abort signal for user-initiated cancellation
  * @returns A Promise resolving to a bot-generated Message
  */
 export const fetchChatbotReplyWithFiles = async (
@@ -82,11 +83,11 @@ export const fetchChatbotReplyWithFiles = async (
   files: File[],
   signal: AbortSignal,
 ): Promise<Message> => {
-  const controller = new AbortController();
-  const timeoutId = setTimeout(
-    () => controller.abort(),
+  // Combine external signal with timeout using AbortSignal.any()
+  const timeoutSignal = AbortSignal.timeout(
     CHATBOT_API_TIMEOUTS_MS.GENERATE_MESSAGE,
   );
+  const combinedSignal = AbortSignal.any([signal, timeoutSignal]);
 
   try {
     const formData = new FormData();
@@ -101,7 +102,7 @@ export const fetchChatbotReplyWithFiles = async (
       {
         method: "POST",
         body: formData,
-        signal: signal,
+        signal: combinedSignal,
       },
     );
 
@@ -117,15 +118,17 @@ export const fetchChatbotReplyWithFiles = async (
     return createBotMessage(botReply);
   } catch (error: unknown) {
     if (error instanceof DOMException && error.name === "AbortError") {
-      console.error(
-        `API request timed out after ${CHATBOT_API_TIMEOUTS_MS.GENERATE_MESSAGE}ms`,
-      );
+      if (signal.aborted) {
+        console.error("API request cancelled by user");
+      } else {
+        console.error(
+          `API request timed out after ${CHATBOT_API_TIMEOUTS_MS.GENERATE_MESSAGE}ms`,
+        );
+      }
     } else {
       console.error("API error uploading files:", error);
     }
     return createBotMessage(getChatbotText("errorMessage"));
-  } finally {
-    clearTimeout(timeoutId);
   }
 };
 

frontend/src/components/Chatbot.tsx

@@ -82,8 +82,7 @@ export const Chatbot = () => {
    * @returns The messages of the chat with id equals to sessionId
    */
   const getSessionMessages = (sessionId: string | null) => {
-    if (currentSessionId === null) {
-      console.error("No current session");
+    if (sessionId === null) {
       return [];
     }
     const chatSession = sessions.find((item) => item.id === sessionId);

frontend/src/tests/api_chatbot.test.ts

@@ -3,10 +3,12 @@ import {
   createChatSession,
   fetchChatbotReply,
   deleteChatSession,
+  fetchChatbotReplyWithFiles,
 } from "../api/chatbot";
 
 import { callChatbotApi } from "../utils/callChatbotApi";
 import { getChatbotText } from "../data/chatbotTexts";
+import { API_BASE_URL, CHATBOT_API_TIMEOUTS_MS } from "../config";
 
 jest.mock("uuid", () => ({
   v4: () => "mock-uuid",
@@ -20,6 +22,9 @@ jest.mock("../data/chatbotTexts", () => ({
   getChatbotText: jest.fn().mockReturnValue("Fallback error message"),
 }));
 
+// Mock global fetch for file upload tests
+global.fetch = jest.fn();
+
 describe("chatbotApi", () => {
   describe("createBotMessage", () => {
     it("creates a bot message with text", () => {
@@ -134,4 +139,217 @@ describe("chatbotApi", () => {
       );
     });
   });
+
+  describe("fetchChatbotReplyWithFiles", () => {
+    beforeEach(() => {
+      jest.clearAllMocks();
+      (global.fetch as jest.Mock).mockClear();
+      jest.useFakeTimers();
+    });
+
+    afterEach(() => {
+      jest.useRealTimers();
+    });
+
+    it("successfully uploads files and returns bot reply", async () => {
+      const mockResponse = {
+        reply: "File analyzed successfully!",
+      };
+      (global.fetch as jest.Mock).mockResolvedValueOnce({
+        ok: true,
+        json: async () => mockResponse,
+      });
+
+      const files = [new File(["content"], "test.txt", { type: "text/plain" })];
+      const controller = new AbortController();
+
+      const result = await fetchChatbotReplyWithFiles(
+        "session-xyz",
+        "Analyze this file",
+        files,
+        controller.signal,
+      );
+
+      expect(result).toEqual({
+        id: "mock-uuid",
+        sender: "jenkins-bot",
+        text: "File analyzed successfully!",
+      });
+
+      expect(global.fetch).toHaveBeenCalledWith(
+        `${API_BASE_URL}/api/chatbot/sessions/session-xyz/message/upload`,
+        expect.objectContaining({
+          method: "POST",
+          signal: expect.any(AbortSignal),
+        }),
+      );
+    });
+
+    it("returns fallback message when API response is not ok", async () => {
+      (global.fetch as jest.Mock).mockResolvedValueOnce({
+        ok: false,
+        status: 500,
+        json: async () => ({ detail: "Internal server error" }),
+      });
+
+      const files = [new File(["content"], "test.txt", { type: "text/plain" })];
+      const controller = new AbortController();
+      const consoleErrorSpy = jest.spyOn(console, "error").mockImplementation();
+
+      const result = await fetchChatbotReplyWithFiles(
+        "session-xyz",
+        "Hello",
+        files,
+        controller.signal,
+      );
+
+      expect(result.text).toBe("Fallback error message");
+      expect(consoleErrorSpy).toHaveBeenCalled();
+      consoleErrorSpy.mockRestore();
+    });
+
+    it("aborts the request when timeout elapses", async () => {
+      // Mock fetch to reject with AbortError when signal is aborted
+      (global.fetch as jest.Mock).mockImplementationOnce(
+        (url: string, options?: RequestInit) =>
+          new Promise((_, reject) => {
+            // Reject with AbortError when signal is aborted
+            if (options?.signal) {
+              options.signal.addEventListener("abort", () => {
+                const error = new DOMException("Aborted", "AbortError");
+                reject(error);
+              });
+            }
+          }) as unknown as Promise<Response>,
+      );
+
+      const files = [new File(["content"], "test.txt", { type: "text/plain" })];
+      const controller = new AbortController();
+      const consoleErrorSpy = jest.spyOn(console, "error").mockImplementation();
+
+      const promise = fetchChatbotReplyWithFiles(
+        "session-xyz",
+        "Hello",
+        files,
+        controller.signal,
+      );
+
+      // Fast-forward time to trigger timeout
+      jest.advanceTimersByTime(CHATBOT_API_TIMEOUTS_MS.GENERATE_MESSAGE);
+
+      // Wait for promise to resolve after timeout
+      const result = await promise;
+
+      expect(result.text).toBe("Fallback error message");
+      expect(consoleErrorSpy).toHaveBeenCalledWith(
+        expect.stringContaining("timed out"),
+      );
+      consoleErrorSpy.mockRestore();
+    });
+
+    it("cancels the request when external signal is aborted", async () => {
+      // Mock fetch to reject when signal is aborted
+      (global.fetch as jest.Mock).mockImplementationOnce(
+        (url: string, options?: RequestInit) =>
+          new Promise((_, reject) => {
+            if (options?.signal) {
+              options.signal.addEventListener("abort", () => {
+                reject(new DOMException("Aborted", "AbortError"));
+              });
+            }
+          }) as unknown as Promise<Response>,
+      );
+
+      const files = [new File(["content"], "test.txt", { type: "text/plain" })];
+      const controller = new AbortController();
+      const consoleErrorSpy = jest.spyOn(console, "error").mockImplementation();
+
+      const promise = fetchChatbotReplyWithFiles(
+        "session-xyz",
+        "Hello",
+        files,
+        controller.signal,
+      );
+
+      // Abort the external signal (simulating user clicking Cancel)
+      controller.abort();
+
+      const result = await promise;
+
+      expect(result.text).toBe("Fallback error message");
+      expect(consoleErrorSpy).toHaveBeenCalledWith(
+        "API request cancelled by user",
+      );
+      consoleErrorSpy.mockRestore();
+    });
+
+    it("handles already aborted external signal", async () => {
+      const files = [new File(["content"], "test.txt", { type: "text/plain" })];
+      const controller = new AbortController();
+      controller.abort(); // Abort before calling the function
+
+      const consoleErrorSpy = jest.spyOn(console, "error").mockImplementation();
+
+      const result = await fetchChatbotReplyWithFiles(
+        "session-xyz",
+        "Hello",
+        files,
+        controller.signal,
+      );
+
+      expect(result.text).toBe("Fallback error message");
+      expect(consoleErrorSpy).toHaveBeenCalled();
+      consoleErrorSpy.mockRestore();
+    });
+
+    it("handles network errors gracefully", async () => {
+      (global.fetch as jest.Mock).mockRejectedValueOnce(
+        new Error("Network error"),
+      );
+
+      const files = [new File(["content"], "test.txt", { type: "text/plain" })];
+      const controller = new AbortController();
+      const consoleErrorSpy = jest.spyOn(console, "error").mockImplementation();
+
+      const result = await fetchChatbotReplyWithFiles(
+        "session-xyz",
+        "Hello",
+        files,
+        controller.signal,
+      );
+
+      expect(result.text).toBe("Fallback error message");
+      expect(consoleErrorSpy).toHaveBeenCalledWith(
+        "API error uploading files:",
+        expect.any(Error),
+      );
+      consoleErrorSpy.mockRestore();
+    });
+
+    it("creates FormData with message and files correctly", async () => {
+      const mockResponse = {
+        reply: "Success",
+      };
+      (global.fetch as jest.Mock).mockResolvedValueOnce({
+        ok: true,
+        json: async () => mockResponse,
+      });
+
+      const files = [
+        new File(["content1"], "file1.txt", { type: "text/plain" }),
+        new File(["content2"], "file2.txt", { type: "text/plain" }),
+      ];
+      const controller = new AbortController();
+
+      await fetchChatbotReplyWithFiles(
+        "session-xyz",
+        "Test message",
+        files,
+        controller.signal,
+      );
+
+      const fetchCall = (global.fetch as jest.Mock).mock.calls[0];
+      expect(fetchCall[1]?.body).toBeInstanceOf(FormData);
+    });
+  });
 });