fix(bff): improve error handling (images bff) (#381)

* fix(bff): improve error handling (images bff)

* test(bff): update unit tests

* test(bff): update unit test descriptions

Author: mark-karnaukh-extern-sap

apps/aurora-portal/src/server/Compute/helpers/imageHelpers.test.ts

@@ -7,7 +7,7 @@ import {
   buildNextPageUrl,
   getLastImageMarker,
   validateGlanceService,
-  mapResponseToTRPCError,
+  mapErrorResponseToTRPCError,
   ImageErrorHandlers,
   handleZodParsingError,
   wrapError,
@@ -309,106 +309,106 @@ describe("imageHelpers", () => {
     })
   })
 
-  describe("mapResponseToTRPCError", () => {
-    it("should map 400 status to BAD_REQUEST", () => {
-      const response = { status: 400, statusText: "Bad Request" }
+  describe("mapErrorResponseToTRPCError", () => {
+    it("should map 400 status BAD_REQUEST", () => {
+      const errorResponse = { name: "SignalOpenstackApiError", statusCode: 400, message: "Bad Request" }
       const context = { operation: "create image", imageId: "image-123" }
 
-      const error = mapResponseToTRPCError(response, context)
+      const error = mapErrorResponseToTRPCError(errorResponse, context)
 
       expect(error.code).toBe("BAD_REQUEST")
       expect(error.message).toBe("Failed to create image image: image-123")
     })
 
-    it("should map 403 status to FORBIDDEN", () => {
-      const response = { status: 403, statusText: "Forbidden" }
+    it("should map 403 status FORBIDDEN", () => {
+      const errorResponse = { name: "SignalOpenstackApiError", statusCode: 403, message: "Forbidden" }
       const context = { operation: "delete image", imageId: "image-123" }
 
-      const error = mapResponseToTRPCError(response, context)
+      const error = mapErrorResponseToTRPCError(errorResponse, context)
 
       expect(error.code).toBe("FORBIDDEN")
       expect(error.message).toBe("Access forbidden - cannot delete image image: image-123")
     })
 
-    it("should map 404 status to NOT_FOUND", () => {
-      const response = { status: 404, statusText: "Not Found" }
+    it("should map 404 status NOT_FOUND", () => {
+      const errorResponse = { name: "SignalOpenstackApiError", statusCode: 404, message: "Not Found" }
       const context = { operation: "get image", imageId: "image-123" }
 
-      const error = mapResponseToTRPCError(response, context)
+      const error = mapErrorResponseToTRPCError(errorResponse, context)
 
       expect(error.code).toBe("NOT_FOUND")
       expect(error.message).toBe("Image not found image: image-123")
     })
 
     it("should map 404 status with member info", () => {
-      const response = { status: 404, statusText: "Not Found" }
+      const errorResponse = { name: "SignalOpenstackApiError", statusCode: 404, message: "Not Found" }
       const context = { operation: "get member", imageId: "image-123", memberId: "member-456" }
 
-      const error = mapResponseToTRPCError(response, context)
+      const error = mapErrorResponseToTRPCError(errorResponse, context)
 
       expect(error.code).toBe("NOT_FOUND")
       expect(error.message).toBe("Image or member not found image: image-123, member: member-456")
     })
 
-    it("should map 409 status to CONFLICT", () => {
-      const response = { status: 409, statusText: "Conflict" }
+    it("should map 409 status CONFLICT to TRPC Error", () => {
+      const errorResponse = { name: "SignalOpenstackApiError", statusCode: 409, message: "Conflict" }
       const context = { operation: "update image", imageId: "image-123" }
 
-      const error = mapResponseToTRPCError(response, context)
+      const error = mapErrorResponseToTRPCError(errorResponse, context)
 
       expect(error.code).toBe("CONFLICT")
       expect(error.message).toBe("Conflict - update image image: image-123")
     })
 
-    it("should map 413 status to PAYLOAD_TOO_LARGE", () => {
-      const response = { status: 413, statusText: "Payload Too Large" }
+    it("should map 413 status PAYLOAD_TOO_LARGE", () => {
+      const errorResponse = { name: "SignalOpenstackApiError", statusCode: 413, message: "Payload Too Large" }
       const context = { operation: "upload image", imageId: "image-123" }
 
-      const error = mapResponseToTRPCError(response, context)
+      const error = mapErrorResponseToTRPCError(errorResponse, context)
 
       expect(error.code).toBe("PAYLOAD_TOO_LARGE")
       expect(error.message).toBe("Request entity too large image: image-123")
     })
 
-    it("should map 415 status to BAD_REQUEST", () => {
-      const response = { status: 415, statusText: "Unsupported Media Type" }
+    it("should map 415 status BAD_REQUEST", () => {
+      const errorResponse = { name: "SignalOpenstackApiError", statusCode: 415, message: "Unsupported Media Type" }
       const context = { operation: "upload image", imageId: "image-123" }
 
-      const error = mapResponseToTRPCError(response, context)
+      const error = mapErrorResponseToTRPCError(errorResponse, context)
 
       expect(error.code).toBe("BAD_REQUEST")
       expect(error.message).toBe("Unsupported media type image: image-123")
     })
 
-    it("should map unknown status to INTERNAL_SERVER_ERROR", () => {
-      const response = { status: 500, statusText: "Internal Server Error" }
+    it("should map unknown status INTERNAL_SERVER_ERROR", () => {
+      const errorResponse = { name: "SignalOpenstackApiError", statusCode: 500, message: "Internal Server Error" }
       const context = { operation: "create image" }
 
-      const error = mapResponseToTRPCError(response, context)
+      const error = mapErrorResponseToTRPCError(errorResponse, context)
 
       expect(error.code).toBe("INTERNAL_SERVER_ERROR")
       expect(error.message).toBe("Failed to create image: Internal Server Error")
     })
 
-    it("should handle missing statusText", () => {
-      const response = { status: 500 }
+    it("should handle missing status text", () => {
+      const errorResponse = { name: "SignalOpenstackApiError", statusCode: 500, message: "" }
       const context = { operation: "create image" }
 
-      const error = mapResponseToTRPCError(response, context)
+      const error = mapErrorResponseToTRPCError(errorResponse, context)
 
       expect(error.code).toBe("INTERNAL_SERVER_ERROR")
       expect(error.message).toBe("Failed to create image: Unknown error")
     })
 
     it("should include additional info when provided", () => {
-      const response = { status: 400, statusText: "Bad Request" }
+      const errorResponse = { name: "SignalOpenstackApiError", statusCode: 400, message: "Bad Request" }
       const context = {
         operation: "create image",
         imageId: "image-123",
         additionalInfo: "Invalid disk format",
       }
 
-      const error = mapResponseToTRPCError(response, context)
+      const error = mapErrorResponseToTRPCError(errorResponse, context)
 
       expect(error.message).toBe("Failed to create image image: image-123 - Invalid disk format")
     })

apps/aurora-portal/src/server/Compute/helpers/imageHelpers.ts

@@ -1,6 +1,7 @@
 import { TRPCError } from "@trpc/server"
 import { ZodError } from "zod"
 import { ListImagesInput } from "../types/image"
+import { SignalOpenstackApiError } from "@cobaltcore-dev/signal-openstack"
 
 /**
  * Applies sorting and filtering parameters to URLSearchParams for OpenStack Glance API calls
@@ -176,13 +177,13 @@ export function validateGlanceService(glance: unknown): asserts glance is NonNul
 }
 
 /**
- * Maps OpenStack API HTTP response status codes to appropriate TRPCError instances
- * @param response - The HTTP response from OpenStack API
+ * Maps Signal OpenStack API Error to appropriate TRPCError instances
+ * @param error - The Signal OpenStack API Error
  * @param context - Additional context for error messages (e.g., imageId, operation type)
  * @returns TRPCError instance with appropriate code and message
  */
-export function mapResponseToTRPCError(
-  response: { status?: number; statusText?: string },
+export function mapErrorResponseToTRPCError(
+  error: SignalOpenstackApiError,
   context: {
     operation: string
     imageId?: string
@@ -196,7 +197,7 @@ export function mapResponseToTRPCError(
   const memberInfo = memberId ? `, member: ${memberId}` : ""
   const extraInfo = additionalInfo ? ` - ${additionalInfo}` : ""
 
-  switch (response.status) {
+  switch (error.statusCode) {
     case 400:
       return new TRPCError({
         code: "BAD_REQUEST",
@@ -236,7 +237,7 @@ export function mapResponseToTRPCError(
     default:
       return new TRPCError({
         code: "INTERNAL_SERVER_ERROR",
-        message: `${baseMessage}: ${response.statusText || "Unknown error"}`,
+        message: `${baseMessage}: ${error.message || "Unknown error"}`,
       })
   }
 }

apps/aurora-portal/src/server/Compute/routers/imageRouter.test.ts

@@ -7,25 +7,29 @@ import { GlanceImage, ImageMember } from "../types/image"
 import { createCallerFactory, auroraRouter } from "../../trpc"
 
 // Mock the helpers
-vi.mock("../helpers/imageHelpers", () => ({
-  applyImageQueryParams: vi.fn(),
-  validateGlanceService: vi.fn(),
-  mapResponseToTRPCError: vi.fn(),
-  ImageErrorHandlers: {
-    upload: vi.fn(),
-    visibility: vi.fn(),
-    delete: vi.fn(),
-    member: {
-      list: vi.fn(),
-      get: vi.fn(),
-      create: vi.fn(),
-      update: vi.fn(),
+vi.mock("../helpers/imageHelpers", async (importOriginal) => {
+  const actual: object = await importOriginal()
+
+  return {
+    ...actual,
+    applyImageQueryParams: vi.fn(),
+    validateGlanceService: vi.fn(),
+    ImageErrorHandlers: {
+      upload: vi.fn(),
+      visibility: vi.fn(),
       delete: vi.fn(),
+      member: {
+        list: vi.fn(),
+        get: vi.fn(),
+        create: vi.fn(),
+        update: vi.fn(),
+        delete: vi.fn(),
+      },
     },
-  },
-  handleZodParsingError: vi.fn(),
-  withErrorHandling: vi.fn((fn) => fn()),
-}))
+    handleZodParsingError: vi.fn(),
+    withErrorHandling: vi.fn((fn) => fn()),
+  }
+})
 
 // Mock data
 const mockGlanceImage: GlanceImage = {
@@ -134,18 +138,11 @@ describe("imageRouter", () => {
       const caller = createCaller(mockCtx)
 
       // Mock failed response
-      mockCtx.mockGlance.get.mockResolvedValue({
-        ok: false,
-        status: 500,
-        json: vi.fn(),
-      })
-
-      const mockError = new TRPCError({ code: "INTERNAL_SERVER_ERROR", message: "API Error" })
-      ;(imageHelpers.mapResponseToTRPCError as Mock).mockReturnValue(mockError)
+      mockCtx.mockGlance.get.mockRejectedValue({ statusCode: 500, message: "Internal Server Error" })
 
       const input = {}
 
-      await expect(caller.image.listImages(input)).rejects.toThrow("API Error")
+      await expect(caller.image.listImages(input)).rejects.toThrow("Failed to list images: Internal Server Error")
     })
   })
 
@@ -170,18 +167,11 @@ describe("imageRouter", () => {
       const mockCtx = createMockContext()
       const caller = createCaller(mockCtx)
 
-      mockCtx.mockGlance.get.mockResolvedValue({
-        ok: false,
-        status: 404,
-        json: vi.fn(),
-      })
-
-      const mockError = new TRPCError({ code: "NOT_FOUND", message: "Image not found" })
-      ;(imageHelpers.mapResponseToTRPCError as Mock).mockReturnValue(mockError)
+      mockCtx.mockGlance.get.mockRejectedValue({ statusCode: 404, message: "Not Found" })
 
       const input = { imageId: "123e4567-e89b-12d3-a456-426614174111" }
 
-      await expect(caller.image.getImageById(input)).rejects.toThrow("Image not found")
+      await expect(caller.image.getImageById(input)).rejects.toThrow(`Image not found image: ${input.imageId}`)
     })
   })
 
@@ -223,22 +213,15 @@ describe("imageRouter", () => {
       const mockCtx = createMockContext()
       const caller = createCaller(mockCtx)
 
-      mockCtx.mockGlance.post.mockResolvedValue({
-        ok: false,
-        status: 400,
-        json: vi.fn(),
-      })
-
-      const mockError = new TRPCError({ code: "BAD_REQUEST", message: "Create failed" })
-      ;(imageHelpers.mapResponseToTRPCError as Mock).mockReturnValue(mockError)
+      mockCtx.mockGlance.post.mockRejectedValue({ statusCode: 400, message: "Bad Request" })
 
       const input = {
         name: "new-image",
         container_format: "bare" as const,
         disk_format: "qcow2" as const,
       }
 
-      await expect(caller.image.createImage(input)).rejects.toThrow("Create failed")
+      await expect(caller.image.createImage(input)).rejects.toThrow("Failed to create image")
     })
   })
 
@@ -271,10 +254,8 @@ describe("imageRouter", () => {
       const mockCtx = createMockContext()
       const caller = createCaller(mockCtx)
 
-      mockCtx.mockGlance.put.mockResolvedValue({
-        ok: false,
-        status: 400,
-        json: vi.fn(),
+      mockCtx.mockGlance.put.mockImplementation(() => {
+        return Promise.reject(new Error("400 Bad Request"))
       })
 
       const mockError = new TRPCError({ code: "BAD_REQUEST", message: "Upload failed" })