Merge pull request #1581 from dodona-edu/fix/purged-report-error

Fix purged reports loading indefinitely

api/app/serializers/dataset_serializer.rb

@@ -13,6 +13,8 @@ class DatasetSerializer < ApplicationSerializer
   attributes :programming_language, :zipfile, :name
 
   def zipfile
+    return if object.zipfile.blank?
+
     url_for(object.zipfile)
   end
 end

api/test/controllers/reports_controller_test.rb

@@ -74,4 +74,15 @@ class ReportsControllerTest < ActionDispatch::IntegrationTest
       assert_response :not_found
     end
   end
+
+  test 'should successfully show purged reports' do
+    delete report_url(@report), as: :json
+    assert_response :no_content
+
+    @report.reload
+    assert_equal @report.status, 'purged'
+
+    get report_url(@report), as: :json
+    assert_response :success
+  end
 end

web/src/components/upload/UploadFormCard.vue

@@ -1,5 +1,5 @@
 <script lang="ts" setup>
-import { shallowRef, ref, computed, watch } from "vue";
+import { shallowRef, computed, watch } from "vue";
 import { useReportsStore } from "@/stores";
 
 const reports = useReportsStore();
@@ -194,87 +194,30 @@ const onSubmit = async (): Promise<void> => {
   }
 };
 
-// Poll configuration
-const pollingInterval = 1000;
 
-// List containing the the ids of the reports that are currently polling.
-const pollingReports = ref<string[]>([]);
-
-// Start polling for a specific report.
-const startPolling = (reportId: string): void => {
-  // Do not start polling when the report is already polling.
-  if (pollingReports.value.includes(reportId)) {
-    return;
-  }
-
-  // Create the interval for polling.
-  const interval = setInterval(async () => {
-    // Stop the interval when the report is not in the list of polling reports.
-    if (!pollingReports.value.includes(reportId)) {
-      clearInterval(interval);
-      return;
-    }
-
-    // Get the report from the reports list.
-    let report = reports.getReportById(reportId);
-
-    // Stop the polling if the report no longer exists.
-    if (!report) {
-      clearInterval(interval);
-      return;
-    }
-
-    try {
-      report = await reports.reloadReport(report.id);
-
-      // Stop the polling when the report status is final.
-      if (report.hasFinalStatus()) {
-        pollingReports.value = pollingReports.value.filter(
-          (id) => id !== reportId
-        );
-        clearInterval(interval);
+// If the submitted report is ready, show the result page
+watch(
+  () => reportActive.value,
+  (report) => {
+    if (report?.hasFinalStatus()) {
+      if (report.status === "finished") {
+        // Go to the results page.
+        step.value = 4;
+        // Clear the form.
+        clearForm();
       }
 
-      // If the report is the active report
-      // apply some changes to the form UI.
-      if (report.id === reportActive.value?.id) {
-        if (report.status === "finished") {
-          // Go to the results page.
-          step.value = 4;
-          // Clear the form.
-          clearForm();
-        }
-
-        if (report.status === "failed" || report.status === "error") {
-          stderr.value = report.stderr;
-          handleError(`An error occurred while analyzing the dataset (${report.error})`);
-        }
-      }
-    } catch (e: any) {
-      //
-    }
-  }, pollingInterval);
-
-  // Add the report.
-  pollingReports.value.push(reportId);
-};
-
-// Start polling for any report in the list of reports
-// of which the status is not final yet.
-watch(
-  () => reports.reports,
-  (reports) => {
-    for (const report of reports) {
-      if (!report.hasFinalStatus()) {
-         startPolling(report.id);
+      if (report.status === "failed" || report.status === "error") {
+        stderr.value = report.stderr;
+        handleError(`An error occurred while analyzing the dataset (${report.error})`);
       }
     }
   },
   {
-    immediate: true,
     deep: true,
   }
 );
+
 </script>
 
 <template>
@@ -366,9 +309,7 @@ watch(
         <v-window-item :value="2">
           <span>Uploading file...</span>
 
-          <v-progress-linear v-model="uploadProgress" class="mt-2" color="primary" height="25">
-            <strong>{{ uploadProgress }}%</strong>
-          </v-progress-linear>
+          <v-progress-linear indeterminate reverse class="mt-2" color="primary" height="25" />
 
           <v-card-actions class="mt-4 pa-0">
             <v-spacer />

web/src/components/upload/UploadStatus.vue

@@ -23,8 +23,11 @@ const props = defineProps<Props>();
   <v-chip v-else-if="props.status === 'error'" color="error" size="small">
     Error
   </v-chip>
-  <v-chip v-else-if="props.status === 'deleted'" color="grey" size="small">
+  <v-chip v-else-if="props.status === 'purged'" color="grey" size="small">
     Deleted
   </v-chip>
+  <v-chip v-else-if="props.status === 'api_error'" color="error" size="small">
+    <abbr>API</abbr>&nbsp;Error
+  </v-chip>
   <v-chip v-else color="grey" size="small"> Unknown </v-chip>
 </template>

web/src/components/upload/UploadsTableDeleteDialog.vue

@@ -32,7 +32,7 @@ const confirm = async (): Promise<void> => {
   try {
     // Attempt to delete the upload.
     // Only delete the upload if a report id is present and the report is not already deleted.
-    if (props.report.id && props.report.status !== "deleted") {
+    if (props.report.id && props.report.status !== "purged") {
       await axios.delete(reports.getReportUrlById(props.report.id));
     }
 
@@ -72,7 +72,7 @@ const confirm = async (): Promise<void> => {
         <v-btn variant="text" icon="mdi-close" @click="open = false" />
       </v-card-title>
 
-      <v-card-text v-if="props.report.status == 'deleted'">
+      <v-card-text v-if="props.report.status == 'purged'">
           <div>Are you sure you want to delete "{{ props.report.name }}" from the list?</div>
 
           <div>

web/src/components/upload/UploadsTableInfoDialog.vue

@@ -31,7 +31,7 @@ const isDone = computed(
     props.report.status === "finished" ||
     props.report.status === "error" ||
     props.report.status === "failed" ||
-    props.report.status === "deleted"
+    props.report.status === "purged"
 );
 </script>
 
@@ -117,7 +117,7 @@ const isDone = computed(
       </template>
 
       <!-- Status: Deleted -->
-      <template v-else-if="props.report.status === 'deleted'">
+      <template v-else-if="props.report.status === 'purged'">
         <v-card-text>
           <v-alert type="warning" variant="tonal" class="mt-2 mb-0">
             This report has been deleted on the server and is no longer available.<br />

web/src/main.ts

@@ -4,12 +4,15 @@ import { createApp } from "vue";
 import { createVuetify } from "vuetify";
 import { createPinia } from "pinia";
 import webFontLoader from "webfontloader";
+import axios from "axios";
 
 // Styles
 import "@mdi/font/css/materialdesignicons.css";
 import "vuetify/styles";
 import "@/assets/scss/main.scss";
 
+axios.defaults.validateStatus = () => true;
+
 // Create the app
 const app = createApp(App);
 

web/src/stores/reports.store.ts

@@ -1,14 +1,14 @@
-import {UploadReport, Report} from "@/types/uploads/UploadReport";
+import {Report} from "@/types/uploads/UploadReport";
 import {useLocalStorage} from "@vueuse/core";
 import {defineStore} from "pinia";
-import {computed, onMounted} from "vue";
+import {computed, onMounted, ref, shallowRef} from "vue";
 import {useRoute} from "vue-router";
 import slugify from "slugify";
 import axios from "axios";
 
+
 export const useReportsStore = defineStore("reports", () => {
   // List of uploaded reports in localstorage.
-  const oldReports = useLocalStorage<UploadReport[]>("reports:upload", []);
   const reports = useLocalStorage<Report[]>("dolos:reports", [], {
     serializer: {
       read: (v: any) => Report.arrayFromSerialized(v),
@@ -29,6 +29,33 @@ export const useReportsStore = defineStore("reports", () => {
     return slug;
   }
 
+  const MAX_POLLS = 90; // Default execution timeout is 60s
+  const POLLING_INTERVAL_MS = 1000;
+  const polling = ref<Map<string, number>>(new Map());
+  const pollingInterval = shallowRef<ReturnType<typeof setInterval> | null>(null);
+
+  function startPolling(report: Report) {
+    polling.value.set(report.id, 0);
+    if (pollingInterval.value == null) {
+      pollingInterval.value = setInterval(poll, POLLING_INTERVAL_MS);
+    }
+  }
+
+  async function poll() {
+    for (const [reportId, retries] of polling.value.entries()) {
+      const report = await reloadReport(reportId);
+      if (!report.hasFinalStatus() && retries < MAX_POLLS) {
+        polling.value.set(reportId, retries + 1);
+      } else {
+        polling.value.delete(reportId);
+      }
+    }
+    if (polling.value.size == 0 && pollingInterval.value != null) {
+      clearInterval(pollingInterval.value);
+      pollingInterval.value = null;
+    }
+  }
+
   // Upload a new report
   async function uploadReport(name: string, file: File, language?: string): Promise<Report> {
     const data = new FormData();
@@ -42,36 +69,48 @@ export const useReportsStore = defineStore("reports", () => {
     );
 
     if (response.status !== 201) {
-      console.log(response);
       throw new Error(`HTTP error while uploading report: ${response.statusText}`);
     }
 
     const slug = findNextSlug(name);
-    const report = Report.fromResponse(response.data, slug)
+    const report = Report.fromResponse(response.data, slug);
 
     // Add the report to the reports list in local storage.
     reports.value.push(report);
+    startPolling(report);
     return report;
   }
 
-  async function reloadReport(reportId: string): Promise<Report> {
+  async function addReportFromShared(reportId: string): Promise<Report> {
     const url = getReportUrlById(reportId);
     const response = await axios.get(url);
     if (response.status !== 200) {
-      throw new Error(`HTTP error ${response.status} while fetching report ${reportId}`);
+      throw new Error(`HTTP error while fetching shared report: ${response.statusText}`);
     }
+    const slug = findNextSlug(response.data["name"]);
+    const report = Report.fromResponse(response.data, slug, true);
+    reports.value.push(report);
+    return report;
+  }
+
+  async function reloadReport(reportId: string): Promise<Report> {
+    const url = getReportUrlById(reportId);
+    const response = await axios.get(url);
 
-    let report;
     const existing = reports.value.findIndex((r) => r.id === reportId);
-    if (existing >= 0) {
-      const previous = reports.value[existing];
-      report = Report.fromResponse(response.data, previous.slug, previous.fromSharing);
-      reports.value[existing] = report;
+    if (existing === -1) {
+      throw new Error("reportId not found");
+    }
+    const previous = reports.value[existing];
+    let report;
+    if (response.status !== 200) {
+      previous.status = "api_error";
+      report = previous;
     } else {
-      const slug = findNextSlug(response.data["name"]);
-      report = Report.fromResponse(response.data, slug, true);
-      reports.value.push(report);
+      report = Report.fromResponse(response.data, previous.slug, previous.fromSharing);
     }
+    reports.value[existing] = report;
+
     return report;
   }
 
@@ -125,27 +164,13 @@ export const useReportsStore = defineStore("reports", () => {
   };
 
   async function reloadAllReports() {
-    const toCheck = reports.value;
-
-    // These lines gracefully migrate previous reports,
-    // remove when all users have migrated.
-    for (const report of oldReports.value) {
-      toCheck.push(Report.fromUploadReport(report));
+    for (const report of reports.value) {
+      await reloadReport(report.id);
     }
-    oldReports.value = [];
-
-    for (const report of toCheck) {
-      if (report.status === "finished") {
-        try {
-          await reloadReport(report.id);
-        } catch (error: any) {
-          // Set the report status to deleted.
-          report.status = "api_error";
-          report.error = error?.message;
-        }
-      }
+    const pending = reports.value.filter(report => !report.hasFinalStatus());
+    for (const report of pending) {
+      startPolling(report);
     }
-    reports.value = toCheck;
   }
 
   // Fetch all stored reports and update if needed
@@ -167,6 +192,7 @@ export const useReportsStore = defineStore("reports", () => {
 
   return {
     reports,
+    addReportFromShared,
     uploadReport,
     reloadReport,
     getReportById,

web/src/types/uploads/UploadReport.ts

@@ -22,6 +22,7 @@ export type UploadReport = {
   isFromSharing: boolean;
 };
 
+
 export class Report {
   readonly fromSharing: boolean;
 
@@ -41,7 +42,7 @@ export class Report {
   }
 
   public hasFinalStatus() {
-    return this.status === "finished" || this.status === "error" || this.status === "failed";
+    return this.status !== "queued" && this.status !== "running";
   }
 
   static fromResponse(response: Record<string, any>, slug?: string, fromSharing?: boolean): Report {

web/src/types/uploads/UploadReportStatus.ts

@@ -4,5 +4,5 @@ export type UploadReportStatus =
   | "failed"
   | "error"
   | "finished"
-  | "deleted"
+  | "purged"
   | "api_error";

web/src/views/upload/share.vue

@@ -18,9 +18,9 @@ onMounted(async () => {
 
   try{
     // If the report reference does not exist, generate a new one.
-    if (!report) {
+    if (report === undefined) {
       // Fetch the report from the server
-      report = await reports.reloadReport(reportId);
+      report = await reports.addReportFromShared(reportId);
     }
 
     // Wait until status is final