feat: add error handling

Closes #352
dbsystel · Dec 22, 2024 · 43681e3 · 43681e3
1 parent 57231d1
commit 43681e3
Show file tree

Hide file tree

Showing 7 changed files with 213 additions and 33 deletions.
diff --git a/e2e/home-page.ts b/e2e/home-page.ts
@@ -23,14 +23,19 @@ export class HomePage {
     )
   }
 
-  async fetchTestFileFromUrl(url: string) {
+  async fetchTestFileFromUrl(url: string, waitForResponse = true) {
     await this.urlBtn.click()
     const fetchButton = this.page.getByRole("button", { name: "Fetch" })
     expect(await fetchButton.isDisabled()).toBe(true)
     await this.page.getByRole("textbox", { name: "Url" }).fill(url)
-    const responsePromise = this.page.waitForResponse(url)
+    let responsePromise
+    if (waitForResponse) {
+      responsePromise = this.page.waitForResponse(url)
+    }
     await fetchButton.click()
-    await responsePromise
+    if (responsePromise) {
+      await responsePromise
+    }
   }
 
   async uploadLocalTestFile(relativePath: string) {
@@ -138,6 +143,10 @@ export class HomePage {
     expect(await detailsRow.getByRole("link").getAttribute("href")).toEqual(url)
   }
 
+  async getErrorMessage() {
+    return await this.page.locator("[role=alert].text-error").textContent()
+  }
+
   private async checkClipboardContents(expectedContents: string[]) {
     const clipboardContent = await this.page.evaluate(() =>
       navigator.clipboard.readText(),

diff --git a/e2e/smoke.spec.ts b/e2e/smoke.spec.ts
@@ -5,7 +5,6 @@ import { HomePage } from "./home-page"
 const filenameSchemaVersion1 = "test-result-v1.json"
 const filenameSchemaVersion2 = "test-result-v2.json"
 const filenameTestManyResults = "test-many-results.json"
-const trivyReportUrl = `https://raw.githubusercontent.com/dbsystel/trivy-vulnerability-explorer/refs/heads/e2e-playwright/public/${filenameSchemaVersion2}`
 const cveEntries = ["CVE-2021-3450", "CVE-2021-3449", "CVE-2019-14697"]
 
 test("should have the correct title", async ({ page }) => {
@@ -17,7 +16,9 @@ test("should have the correct title", async ({ page }) => {
 test("fetches from URL", async ({ page }) => {
   const homePage = new HomePage(page)
   await homePage.goto()
-  await homePage.fetchTestFileFromUrl(trivyReportUrl)
+  await homePage.fetchTestFileFromUrl(
+    `https://raw.githubusercontent.com/dbsystel/trivy-vulnerability-explorer/refs/heads/e2e-playwright/public/${filenameSchemaVersion2}`,
+  )
   await homePage.verifyTableResult(5)
   await homePage.verifyTrivyignore(cveEntries)
 })
@@ -115,3 +116,74 @@ test("vulnerability details can be shown", async ({ page }) => {
     "https://example.org",
   )
 })
+
+test.describe.parallel("Error handling", () => {
+  test("Error message for files with wrong format is shown", async ({
+    page,
+  }) => {
+    const homePage = new HomePage(page)
+    await homePage.goto()
+
+    await homePage.uploadLocalTestFile(
+      "../public/test-invalid-report-format.json",
+    )
+    let message = await homePage.getErrorMessage()
+    expect(message).toContain("Invalid report format")
+    expect(message).toContain(
+      "The cannot be parsed. Please make sure the report is in the correct format.",
+    )
+
+    await homePage.uploadLocalTestFile(
+      "../public/test-invalid-vulnerability-format.json",
+    )
+    message = await homePage.getErrorMessage()
+    expect(message).toContain("Invalid report format")
+    expect(message).toContain(
+      "The cannot be parsed. Please make sure the report is in the correct format.",
+    )
+  })
+
+  test("Error message for a network error is shown", async ({ page }) => {
+    const homePage = new HomePage(page)
+    await homePage.goto()
+
+    const url = `http://localhost:8080/my-non-existing-file.json`
+
+    await homePage.page.route(url, (route) => route.abort("failed"))
+    await homePage.fetchTestFileFromUrl(url, false)
+
+    const message = await homePage.getErrorMessage()
+    expect(message).toContain("Error when fetching report")
+    expect(message).toContain("Failed to fetch")
+  })
+
+  test("Error message for an invalid JSON file is shown", async ({ page }) => {
+    const homePage = new HomePage(page)
+    await homePage.goto()
+
+    const url = `index.html`
+
+    await homePage.page.route(url, (route) => route.abort("failed"))
+    await homePage.fetchTestFileFromUrl(url, false)
+
+    const message = await homePage.getErrorMessage()
+    expect(message).toContain("Error when fetching report")
+    expect(message).toContain("is not valid JSON")
+  })
+
+  test("Error message for a non 200 OK status is shown", async ({ page }) => {
+    const homePage = new HomePage(page)
+    await homePage.goto()
+
+    const url = `http://localhost:8080/my-non-existing-file.json`
+
+    await homePage.page.route(url, (route) => route.fulfill({ status: 404 }))
+    await homePage.fetchTestFileFromUrl(url, false)
+
+    const message = await homePage.getErrorMessage()
+    expect(message).toContain("Response not successful")
+    expect(message).toContain(
+      "Did not receive Response status 200 OK. Got 404 Not Found",
+    )
+  })
+})
diff --git a/public/test-invalid-report-format.json b/public/test-invalid-report-format.json
@@ -0,0 +1,8 @@
+{
+  "SchemaVersion": 0,
+  "Data": [
+    {
+      "Target": "dummy-image:2.0.0"
+    }
+  ]
+}
diff --git a/public/test-invalid-vulnerability-format.json b/public/test-invalid-vulnerability-format.json
@@ -0,0 +1,14 @@
+{
+  "SchemaVersion": 2,
+  "ArtifactName": "dummy-image:2.0.0",
+  "ArtifactType": "container_image",
+  "Metadata": {},
+  "Results": [
+    {
+      "Target": "dummy-image:2.0.0",
+      "Class": "",
+      "Type": "",
+      "Vulnerabilities": [{}]
+    }
+  ]
+}
diff --git a/src/components/DataInput.vue b/src/components/DataInput.vue
@@ -24,7 +24,7 @@
     />
 
     <ReportUrlFetcher
-      :onNewReport="onNewReport"
+      :onNewReport="onNewReportFromUrl"
       v-if="reportSource === ReportSource.Url"
       :presetUrl="presetUrl"
     />
@@ -44,11 +44,28 @@
       hide-details
     />
   </v-toolbar>
+
+  <v-alert
+    v-if="reportError"
+    class="mt-5"
+    color="error"
+    icon="$error"
+    variant="outlined"
+    closable
+    @click:close="reportError = undefined"
+    :title="reportError.title"
+  >
+    <p>{{ reportError.text }}</p>
+    <a v-if="reportError.link" :href="reportError.link.href" target="_blank">{{
+      reportError.link.text
+    }}</a>
+  </v-alert>
 </template>
 
 <script setup lang="ts">
 import { ref, computed, watch, defineProps, defineEmits, onMounted } from "vue"
 import type {
+  ReportError,
   Version1OrVersion2,
   VulnerabilityReportFile,
   VulnerabilityReportTarget,
@@ -66,16 +83,15 @@ const emit = defineEmits(["inputChanged"])
 const selectedTarget = ref("")
 const reportSource = ref(ReportSource.File)
 const vulnerabilityReport = ref<VulnerabilityReportTarget[]>([])
+const reportError = ref<ReportError>()
 
 const handleFileUpload = (files: File[] | File) => {
   const file = Array.isArray(files) ? files[0] : files
   const reader = new FileReader()
   const temp = reader.readAsText(file)
   reader.onload = (e) => {
     const vulnerabilityTargets = parseFile(e)
-    if (vulnerabilityTargets) {
-      handleNewReport(vulnerabilityTargets)
-    }
+    handleNewReport(vulnerabilityTargets)
   }
 }
 
@@ -117,17 +133,63 @@ const parseFile = (
 }
 
 const extractTargetsFromReport = (parsedReport: Version1OrVersion2) => {
+  reportError.value = undefined
   let vulnerabilityTargets: VulnerabilityReportTarget[]
-  if (isSchemaVersion2(parsedReport)) {
-    vulnerabilityTargets = parsedReport.Results
-  } else {
-    vulnerabilityTargets = parsedReport
+
+  function setReportError() {
+    reportError.value = {
+      title: "Invalid report format",
+      text: `The cannot be parsed. Please make sure the report is in the correct format.`,
+      link: {
+        text: "For more information check the 'Version1OrVersion2' interface",
+        href: "https://github.com/dbsystel/trivy-vulnerability-explorer/blob/main/src/types.ts",
+      },
+    }
   }
 
-  vulnerabilityTargets.forEach((vr) =>
-    vr.Vulnerabilities?.forEach((v) => (v.Target = vr.Target)),
-  )
-  return vulnerabilityTargets
+  try {
+    if (isSchemaVersion2(parsedReport)) {
+      vulnerabilityTargets = parsedReport.Results
+    } else {
+      vulnerabilityTargets = parsedReport
+    }
+
+    console.log(vulnerabilityTargets)
+
+    // validate report format
+    if (
+      !vulnerabilityTargets.every((i) => {
+        const targetValid =
+          i.Target && i.Vulnerabilities && Array.isArray(i.Vulnerabilities)
+        const vulnerabilitiesValid = i.Vulnerabilities?.every((v) => {
+          return v.PkgName && v.VulnerabilityID && v.Title
+        })
+        return targetValid && vulnerabilitiesValid
+      })
+    ) {
+      setReportError()
+      return []
+    }
+
+    vulnerabilityTargets.forEach((vr) =>
+      vr.Vulnerabilities?.forEach((v) => (v.Target = vr.Target)),
+    )
+    return vulnerabilityTargets
+  } catch (e) {
+    setReportError()
+    return []
+  }
+}
+
+const onNewReportFromUrl = (
+  report: Version1OrVersion2 | { error: ReportError },
+) => {
+  if (report instanceof Object && "error" in report) {
+    reportError.value = report.error
+    return
+  }
+
+  return onNewReport(report)
 }
 
 const onNewReport = (report: Version1OrVersion2) => {
@@ -136,9 +198,11 @@ const onNewReport = (report: Version1OrVersion2) => {
 }
 
 const handleNewReport = (vulnerabilityTargets: VulnerabilityReportTarget[]) => {
-  vulnerabilityReport.value.splice(0)
-  vulnerabilityReport.value.push(...vulnerabilityTargets)
-  emit("inputChanged", selectedVulnerabilities.value)
+  if (vulnerabilityTargets.length > 0) {
+    vulnerabilityReport.value.splice(0)
+    vulnerabilityReport.value.push(...vulnerabilityTargets)
+    emit("inputChanged", selectedVulnerabilities.value)
+  }
 }
 
 const isSchemaVersion2 = (

diff --git a/src/components/ReportUrlFetcher.vue b/src/components/ReportUrlFetcher.vue
@@ -68,11 +68,12 @@
 <script setup lang="ts">
 import { ref, onMounted } from "vue"
 import { useLocalStorage } from "@vueuse/core"
+import type { ReportError, Version1OrVersion2 } from "@/types"
 
-const props = defineProps({
-  onNewReport: Function,
-  presetUrl: String,
-})
+const props = defineProps<{
+  onNewReport: (report: Version1OrVersion2 | { error: ReportError }) => void
+  presetUrl: String
+}>()
 
 const url = ref("")
 const state = ref("ready")
@@ -103,16 +104,22 @@ const fetchReportFromUrl = async () => {
       state.value = "ready"
       if (response.ok) {
         const report = await response.json()
-        if (!report) {
-          throw new Error("Response from URL doesn't look like JSON")
-        }
-        if (props.onNewReport) {
-          props.onNewReport(report)
-        }
-        return
+        props.onNewReport(report)
+      } else {
+        props.onNewReport({
+          error: {
+            title: "Response not successful",
+            text: `Did not receive Response status 200 OK. Got ${response.status} ${response.statusText}`,
+          },
+        })
       }
-    } catch (error) {
-      console.error(error)
+    } catch (error: unknown) {
+      props.onNewReport({
+        error: {
+          title: "Error when fetching report",
+          text: error instanceof Error ? error.message : String(error),
+        },
+      })
     }
     state.value = "error"
   }

diff --git a/src/types.ts b/src/types.ts
@@ -30,3 +30,9 @@ export type VulnerabilitySeverityInformation = {
   severity: string
   count: number
 }
+
+export type ReportError = {
+  title: string
+  text: string
+  link?: { text: string; href: string }
+}