From 3b4674afafce880c9921ec4a3e491fe2c7050415 Mon Sep 17 00:00:00 2001 From: Carlos Feria Date: Mon, 29 Jun 2026 19:12:37 +0200 Subject: [PATCH] fix: Fix/error message upload (#1111) Signed-off-by: Carlos Feria <2582866+carlosthe19916@users.noreply.github.com> (cherry picked from commit 319e799328801ef2438a343c3514e43b15eb59bd) --- .../pages/advisory-upload/advisory-upload.tsx | 9 +- .../src/app/pages/sbom-upload/sbom-upload.tsx | 9 +- client/src/app/utils/utils.test.ts | 114 +++++++++++++++++- client/src/app/utils/utils.ts | 25 ++-- e2e/tests/ui/assertions/FileUploadMatchers.ts | 21 +++- .../ui/pages/advisory-upload/upload.spec.ts | 17 +++ .../ui/pages/common/upload-test-helpers.ts | 42 +++++++ e2e/tests/ui/pages/sbom-upload/upload.spec.ts | 16 +++ 8 files changed, 230 insertions(+), 23 deletions(-) diff --git a/client/src/app/pages/advisory-upload/advisory-upload.tsx b/client/src/app/pages/advisory-upload/advisory-upload.tsx index fbe256a56..b1947cb5d 100644 --- a/client/src/app/pages/advisory-upload/advisory-upload.tsx +++ b/client/src/app/pages/advisory-upload/advisory-upload.tsx @@ -1,7 +1,7 @@ import React from "react"; import { Link } from "react-router-dom"; -import type { AxiosError, AxiosResponse } from "axios"; +import type { AxiosResponse } from "axios"; import { Breadcrumb, @@ -19,6 +19,7 @@ import { ReadOnlyContext } from "@app/components/ReadOnlyContext"; import { UploadFiles } from "@app/components/UploadFiles"; import { useUploadAdvisory } from "@app/queries/advisories"; import { Paths } from "@app/Routes"; +import { getAxiosErrorMessage } from "@app/utils/utils"; export const AdvisoryUpload: React.FC = () => { const { isReadOnly } = React.useContext(ReadOnlyContext); @@ -72,11 +73,7 @@ export const AdvisoryUpload: React.FC = () => { ) => { return `${response.data.document_id} uploaded`; }} - extractErrorMessage={(error: AxiosError) => - error.response?.data - ? error.message - : "Error while uploading file" - } + extractErrorMessage={getAxiosErrorMessage} /> diff --git a/client/src/app/pages/sbom-upload/sbom-upload.tsx b/client/src/app/pages/sbom-upload/sbom-upload.tsx index 802d0f660..c27ba5331 100644 --- a/client/src/app/pages/sbom-upload/sbom-upload.tsx +++ b/client/src/app/pages/sbom-upload/sbom-upload.tsx @@ -1,7 +1,7 @@ import React from "react"; import { Link } from "react-router-dom"; -import type { AxiosError, AxiosResponse } from "axios"; +import type { AxiosResponse } from "axios"; import { Breadcrumb, @@ -19,6 +19,7 @@ import { ReadOnlyContext } from "@app/components/ReadOnlyContext"; import { UploadFiles } from "@app/components/UploadFiles"; import { useUploadSBOM } from "@app/queries/sboms"; import { Paths } from "@app/Routes"; +import { getAxiosErrorMessage } from "@app/utils/utils"; export const SbomUpload: React.FC = () => { const { isReadOnly } = React.useContext(ReadOnlyContext); @@ -74,11 +75,7 @@ export const SbomUpload: React.FC = () => { ) => { return `${response.data.document_id} uploaded`; }} - extractErrorMessage={(error: AxiosError) => - error.response?.data - ? error.message - : "Error while uploading file" - } + extractErrorMessage={getAxiosErrorMessage} /> diff --git a/client/src/app/utils/utils.test.ts b/client/src/app/utils/utils.test.ts index 21904e547..91f7423b7 100644 --- a/client/src/app/utils/utils.test.ts +++ b/client/src/app/utils/utils.test.ts @@ -1,4 +1,17 @@ -import { getToolbarChipKey } from "./utils"; +import type { AxiosError } from "axios"; + +import { getAxiosErrorMessage, getToolbarChipKey } from "./utils"; + +const createAxiosError = ( + overrides: Partial = {}, +): AxiosError => + ({ + message: "Request failed with status code 400", + isAxiosError: true, + name: "AxiosError", + toJSON: () => ({}), + ...overrides, + }) as AxiosError; describe("utils", () => { // getToolbarChipKey @@ -12,4 +25,103 @@ describe("utils", () => { const result = getToolbarChipKey({ key: "myKey", node: "myNode" }); expect(result).toBe("myKey"); }); + + // getAxiosErrorMessage + + it("getAxiosErrorMessage: concatenates error and message (ErrorInformation)", () => { + const error = createAxiosError({ + response: { + data: { error: "InvalidFormat", message: "expected CycloneDX or SPDX" }, + status: 400, + statusText: "Bad Request", + headers: {}, + config: {} as never, + }, + }); + expect(getAxiosErrorMessage(error)).toBe( + "InvalidFormat: expected CycloneDX or SPDX", + ); + }); + + it("getAxiosErrorMessage: includes details when error and message present", () => { + const error = createAxiosError({ + response: { + data: { + error: "InvalidFormat", + message: "could not parse document", + details: "line 42: unexpected token", + }, + status: 400, + statusText: "Bad Request", + headers: {}, + config: {} as never, + }, + }); + expect(getAxiosErrorMessage(error)).toBe( + "InvalidFormat: could not parse document\nline 42: unexpected token", + ); + }); + + it("getAxiosErrorMessage: returns message when only message present", () => { + const error = createAxiosError({ + response: { + data: { message: "Invalid SBOM format" }, + status: 400, + statusText: "Bad Request", + headers: {}, + config: {} as never, + }, + }); + expect(getAxiosErrorMessage(error)).toBe("Invalid SBOM format"); + }); + + it("getAxiosErrorMessage: returns error when only error present", () => { + const error = createAxiosError({ + response: { + data: { error: "Something went wrong" }, + status: 500, + statusText: "Internal Server Error", + headers: {}, + config: {} as never, + }, + }); + expect(getAxiosErrorMessage(error)).toBe("Something went wrong"); + }); + + it("getAxiosErrorMessage: returns plain string response data", () => { + const error = createAxiosError({ + response: { + data: "Plain text error body", + status: 400, + statusText: "Bad Request", + headers: {}, + config: {} as never, + }, + }); + expect(getAxiosErrorMessage(error)).toBe("Plain text error body"); + }); + + it("getAxiosErrorMessage: falls back to axiosError.message", () => { + const error = createAxiosError({ + message: "Network Error", + }); + expect(getAxiosErrorMessage(error)).toBe("Network Error"); + }); + + it("getAxiosErrorMessage: ignores unknown fields and uses error+message", () => { + const error = createAxiosError({ + response: { + data: { + error: "SomeError", + message: "Something happened", + unknownField: "ignored", + }, + status: 400, + statusText: "Bad Request", + headers: {}, + config: {} as never, + }, + }); + expect(getAxiosErrorMessage(error)).toBe("SomeError: Something happened"); + }); }); diff --git a/client/src/app/utils/utils.ts b/client/src/app/utils/utils.ts index 4fe81883f..73dc8fb96 100644 --- a/client/src/app/utils/utils.ts +++ b/client/src/app/utils/utils.ts @@ -10,14 +10,25 @@ import type { ToolbarLabel } from "@patternfly/react-core"; // eslint-disable-next-line @typescript-eslint/no-explicit-any -- allowed export const getAxiosErrorMessage = (axiosError: AxiosError) => { - if (axiosError.response?.data?.errorMessage) { - return axiosError.response.data.errorMessage; + const data = axiosError.response?.data; + + const error = typeof data?.error === "string" ? data.error : undefined; + const message = typeof data?.message === "string" ? data.message : undefined; + const details = typeof data?.details === "string" ? data.details : undefined; + + if (error && message) { + const base = `${error}: ${message}`; + return details ? `${base}\n${details}` : base; + } + if (message) { + return message; } - if ( - axiosError.response?.data?.error && - typeof axiosError?.response?.data?.error === "string" - ) { - return axiosError?.response?.data?.error; + if (error) { + return error; + } + + if (typeof data === "string") { + return data; } return axiosError.message; }; diff --git a/e2e/tests/ui/assertions/FileUploadMatchers.ts b/e2e/tests/ui/assertions/FileUploadMatchers.ts index b6d2890bb..cdcea4744 100644 --- a/e2e/tests/ui/assertions/FileUploadMatchers.ts +++ b/e2e/tests/ui/assertions/FileUploadMatchers.ts @@ -10,6 +10,7 @@ export interface FileUploadMatchers { toHaveItemUploadStatus(expectedStatus: { fileName: string; status: "success" | "danger"; + message?: string; }): Promise; } @@ -55,6 +56,7 @@ export const fileUploadAssertions = expectedStatus: { fileName: string; status: "success" | "danger"; + message?: string; }, ): Promise => { try { @@ -64,9 +66,22 @@ export const fileUploadAssertions = await baseExpect( statusItem.locator(`.pf-v6-c-progress.pf-m-${expectedStatus.status}`), ).toBeVisible(); - await baseExpect( - statusItem.locator(".pf-v6-c-progress__status", { hasText: "100%" }), - ).toBeVisible(); + + if (expectedStatus.message) { + const helperTextVariant = + expectedStatus.status === "danger" ? "error" : "default"; + await baseExpect( + statusItem.locator( + `.pf-v6-c-helper-text__item.pf-m-${helperTextVariant}`, + ), + ).toContainText(expectedStatus.message); + } else { + await baseExpect( + statusItem.locator(".pf-v6-c-progress__status", { + hasText: "100%", + }), + ).toBeVisible(); + } return { pass: true, diff --git a/e2e/tests/ui/pages/advisory-upload/upload.spec.ts b/e2e/tests/ui/pages/advisory-upload/upload.spec.ts index 9f61bea0d..f428724f6 100644 --- a/e2e/tests/ui/pages/advisory-upload/upload.spec.ts +++ b/e2e/tests/ui/pages/advisory-upload/upload.spec.ts @@ -5,6 +5,7 @@ import { login } from "../../helpers/Auth"; import { testInvalidFileExtensions, testRemoveFiles, + testUploadApiErrorMessage, testUploadFilesParallel, testUploadFilesSequentially, } from "../common/upload-test-helpers"; @@ -147,4 +148,20 @@ test.describe("File Upload", { tag: ["@upload"] }, () => { return { fileUploader }; }, }); + + testUploadApiErrorMessage("displays API error message for advisory", { + filePath: TEST_FILES.INVALID_JSON, + apiRoutePattern: "**/api/v3/advisory", + errorResponseBody: { + error: "InvalidFormat", + message: "not a valid CSAF, CVE, or OSV document", + }, + expectedErrorMessage: + "InvalidFormat: not a valid CSAF, CVE, or OSV document", + getConfig: async ({ page }) => { + const uploadPage = await AdvisoryUploadPage.buildFromBrowserPath(page); + const fileUploader = await uploadPage.getFileUploader(); + return { fileUploader }; + }, + }); }); diff --git a/e2e/tests/ui/pages/common/upload-test-helpers.ts b/e2e/tests/ui/pages/common/upload-test-helpers.ts index 9e07cf560..f27cc1e8e 100644 --- a/e2e/tests/ui/pages/common/upload-test-helpers.ts +++ b/e2e/tests/ui/pages/common/upload-test-helpers.ts @@ -141,6 +141,48 @@ export const testRemoveFiles = ({ } }); +export const testUploadApiErrorMessage = ( + testName: string, + { + filePath, + apiRoutePattern, + errorResponseBody, + expectedErrorMessage, + getConfig, + }: { + filePath: string; + apiRoutePattern: string; + errorResponseBody: { error: string; message?: string; details?: string }; + expectedErrorMessage: string; + getConfig: ({ page }: { page: Page }) => Promise; + }, +) => + test(`Upload API error message - ${testName}`, async ({ page }) => { + const config = await getConfig({ page }); + const fileUploader = config.fileUploader; + + await page.route(apiRoutePattern, async (route) => { + await route.fulfill({ + status: 400, + contentType: "application/json", + body: JSON.stringify(errorResponseBody), + }); + }); + + try { + await fileUploader.uploadFiles([filePath]); + + const fileName = path.basename(filePath); + await expect(fileUploader).toHaveItemUploadStatus({ + fileName, + status: "danger", + message: expectedErrorMessage, + }); + } finally { + await page.unroute(apiRoutePattern); + } + }); + export const testInvalidFileExtensions = ({ filesPaths, getConfig, diff --git a/e2e/tests/ui/pages/sbom-upload/upload.spec.ts b/e2e/tests/ui/pages/sbom-upload/upload.spec.ts index fe17560a5..de13ac744 100644 --- a/e2e/tests/ui/pages/sbom-upload/upload.spec.ts +++ b/e2e/tests/ui/pages/sbom-upload/upload.spec.ts @@ -5,6 +5,7 @@ import { login } from "../../helpers/Auth"; import { testInvalidFileExtensions, testRemoveFiles, + testUploadApiErrorMessage, testUploadFilesParallel, testUploadFilesSequentially, } from "../common/upload-test-helpers"; @@ -147,4 +148,19 @@ test.describe("File Upload", { tag: ["@upload"] }, () => { return { fileUploader }; }, }); + + testUploadApiErrorMessage("displays API error message for SBOM", { + filePath: TEST_FILES.INVALID_JSON, + apiRoutePattern: "**/api/v3/sbom", + errorResponseBody: { + error: "InvalidFormat", + message: "expected CycloneDX or SPDX document", + }, + expectedErrorMessage: "InvalidFormat: expected CycloneDX or SPDX document", + getConfig: async ({ page }) => { + const uploadPage = await SBOMUploadPage.buildFromBrowserPath(page); + const fileUploader = await uploadPage.getFileUploader(); + return { fileUploader }; + }, + }); });