Skip to content

Commit

Permalink
Merge pull request galaxyproject#18726 from ahmedhamidawan/invocation…
Browse files Browse the repository at this point in the history
…_state_unhandled_error

[24.1] Add error handling in `WorkflowInvocationState`
  • Loading branch information
dannon authored Aug 21, 2024
2 parents 33d3fa4 + 303577c commit be7ec30
Show file tree
Hide file tree
Showing 2 changed files with 62 additions and 17 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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", () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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";
Expand Down Expand Up @@ -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);
Expand All @@ -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);
Expand Down Expand Up @@ -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(() => {
Expand All @@ -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);
}
Expand Down Expand Up @@ -152,7 +167,7 @@ function getWorkflowId() {
}
function getWorkflowName() {
return workflowStore.getStoredWorkflowNameByInstanceId(invocation.value?.workflow_id);
return workflowStore.getStoredWorkflowNameByInstanceId(invocation.value?.workflow_id || "");
}
</script>

Expand Down Expand Up @@ -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>

0 comments on commit be7ec30

Please sign in to comment.