Merge pull request #18726 from ahmedhamidawan/invocation_state_unhand…

…led_error

[24.1] Add error handling in `WorkflowInvocationState`

client/src/components/WorkflowInvocationState/WorkflowInvocationState.test.ts

@@ -51,7 +51,11 @@ const invocationJobsSummaryById = {
 // Mock the invocation store to return the expected invocation data given the invocation ID
 jest.mock("@/stores/invocationStore", () => {
     const originalModule = jest.requireActual("@/stores/invocationStore");
-    const mockFetchInvocationForId = jest.fn();
+    const mockFetchInvocationForId = jest.fn().mockImplementation((fetchParams) => {
+        if (fetchParams.id === "error-invocation") {
+            throw new Error("User does not own specified item.");
+        }
+    });
     const mockFetchInvocationJobsSummaryForId = jest.fn();
     return {
         ...originalModule,
@@ -106,37 +110,57 @@ describe("WorkflowInvocationState check invocation and job terminal states", ()
         const wrapper = await mountWorkflowInvocationState(invocationData.id);
         expect(isInvocationAndJobTerminal(wrapper)).toBe(true);
 
-        // Neither the invocation nor the jobs summary should be fetched for terminal invocations
-        assertInvocationFetched(0);
+        // Invocation is fetched once and the jobs summary isn't fetched at all for terminal invocations
+        assertInvocationFetched(1);
         assertJobsSummaryFetched(0);
     });
 
     it("determines that invocation and job states are not terminal with no fetched invocation", async () => {
         const wrapper = await mountWorkflowInvocationState("not-fetched-invocation");
         expect(isInvocationAndJobTerminal(wrapper)).toBe(false);
 
-        // Both, the invocation and jobs summary should be fetched once if the invocation is not in the store
+        // Invocation is fetched once and the jobs summary is then never fetched if the invocation is not in the store
         assertInvocationFetched(1);
-        assertJobsSummaryFetched(1);
+        assertJobsSummaryFetched(0);
+
+        // expect there to be an alert for the missing invocation
+        const alert = wrapper.find("balert-stub");
+        expect(alert.attributes("variant")).toBe("info");
+        const span = alert.find("span");
+        expect(span.text()).toBe("Invocation not found.");
     });
 
     it("determines that invocation is not terminal with non-terminal state", async () => {
         const wrapper = await mountWorkflowInvocationState("non-terminal-id");
         expect(isInvocationAndJobTerminal(wrapper)).toBe(false);
 
-        // Only the invocation should be fetched for non-terminal invocations
-        assertInvocationFetched(1);
+        // Only the invocation is fetched for non-terminal invocations; once for the initial fetch and then for the polling
+        assertInvocationFetched(2);
         assertJobsSummaryFetched(0);
     });
 
     it("determines that job states are not terminal with non-terminal jobs but scheduled invocation", async () => {
         const wrapper = await mountWorkflowInvocationState("non-terminal-jobs");
         expect(isInvocationAndJobTerminal(wrapper)).toBe(false);
 
-        // Only the jobs summary should be fetched, not the invocation since it is in scheduled/terminal state
-        assertInvocationFetched(0);
+        // Only the jobs summary should be polled, the invocation is initially fetched only since it is in scheduled/terminal state
+        assertInvocationFetched(1);
         assertJobsSummaryFetched(1);
     });
+
+    it("determines that errored invocation fetches are handled correctly", async () => {
+        const wrapper = await mountWorkflowInvocationState("error-invocation");
+        expect(isInvocationAndJobTerminal(wrapper)).toBe(false);
+
+        // Invocation is fetched once and the jobs summary isn't fetched at all for errored invocations
+        assertInvocationFetched(1);
+        assertJobsSummaryFetched(0);
+
+        // expect there to be an alert for the handled error
+        const alert = wrapper.find("balert-stub");
+        expect(alert.attributes("variant")).toBe("danger");
+        expect(alert.text()).toBe("User does not own specified item.");
+    });
 });
 
 describe("WorkflowInvocationState check 'Report' tab disabled state", () => {

client/src/components/WorkflowInvocationState/WorkflowInvocationState.vue

@@ -11,6 +11,7 @@ import { useAnimationFrameResizeObserver } from "@/composables/sensors/animation
 import { useInvocationStore } from "@/stores/invocationStore";
 import { useWorkflowStore } from "@/stores/workflowStore";
 import localize from "@/utils/localization";
+import { errorMessageAsString } from "@/utils/simple-error";
 
 import { cancelWorkflowScheduling } from "./services";
 import { isTerminal, jobCount, runningCount } from "./util";
@@ -49,6 +50,8 @@ const invocationStore = useInvocationStore();
 
 const stepStatesInterval = ref<any>(undefined);
 const jobStatesInterval = ref<any>(undefined);
+const initialLoading = ref(true);
+const errorMessage = ref<string | null>(null);
 
 // after the report tab is first activated, no longer lazy-render it from then on
 const reportActive = ref(false);
@@ -69,8 +72,10 @@ useAnimationFrameResizeObserver(scrollableDiv, ({ clientSize, scrollSize }) => {
     isScrollable.value = scrollSize.height >= clientSize.height + 1;
 });
 
-const invocation = computed(
-    () => invocationStore.getInvocationById(props.invocationId) as WorkflowInvocationElementView
+const invocation = computed(() =>
+    !initialLoading.value && !errorMessage.value
+        ? (invocationStore.getInvocationById(props.invocationId) as WorkflowInvocationElementView)
+        : null
 );
 const invocationState = computed(() => invocation.value?.state || "new");
 const invocationAndJobTerminal = computed(() => invocationSchedulingTerminal.value && jobStatesTerminal.value);
@@ -105,9 +110,19 @@ const workflowStore = useWorkflowStore();
 const isDeletedWorkflow = computed(() => getWorkflow()?.deleted === true);
 const workflowVersion = computed(() => getWorkflow()?.version);
 
-onMounted(() => {
-    pollStepStatesUntilTerminal();
-    pollJobStatesUntilTerminal();
+onMounted(async () => {
+    try {
+        await invocationStore.fetchInvocationForId({ id: props.invocationId });
+        initialLoading.value = false;
+        if (invocation.value) {
+            await pollStepStatesUntilTerminal();
+            await pollJobStatesUntilTerminal();
+        }
+    } catch (e) {
+        errorMessage.value = errorMessageAsString(e);
+    } finally {
+        initialLoading.value = false;
+    }
 });
 
 onUnmounted(() => {
@@ -116,7 +131,7 @@ onUnmounted(() => {
 });
 
 async function pollStepStatesUntilTerminal() {
-    if (!invocation.value || !invocationSchedulingTerminal.value) {
+    if (!invocationSchedulingTerminal.value) {
         await invocationStore.fetchInvocationForId({ id: props.invocationId });
         stepStatesInterval.value = setTimeout(pollStepStatesUntilTerminal, 3000);
     }
@@ -152,7 +167,7 @@ function getWorkflowId() {
 }
 
 function getWorkflowName() {
-    return workflowStore.getStoredWorkflowNameByInstanceId(invocation.value?.workflow_id);
+    return workflowStore.getStoredWorkflowNameByInstanceId(invocation.value?.workflow_id || "");
 }
 </script>
 
@@ -261,7 +276,13 @@ function getWorkflowName() {
             </BTab>
         </BTabs>
     </div>
-    <BAlert v-else variant="info" show>
+    <BAlert v-else-if="initialLoading" variant="info" show>
         <LoadingSpan message="Loading invocation" />
     </BAlert>
+    <BAlert v-else-if="errorMessage" variant="danger" show>
+        {{ errorMessage }}
+    </BAlert>
+    <BAlert v-else variant="info" show>
+        <span v-localize>Invocation not found.</span>
+    </BAlert>
 </template>