Skip to content

Commit

Permalink
[dagit] Show a better error on invalid run filters (#7494)
Browse files Browse the repository at this point in the history
## Summary

Resolves #7219.

A handful of changes to handle errors a bit more gracefully on the Runs page.

- If filters are set but no matching runs are found, show a message that indicates this. Currently we just say that there are no runs and direct you to use the launchpad, but this isn't helpful if you *do* have runs but none of them match what you've searched.
- If a 400 error occurs during the filter request, show an error that represents this.
  - The current behavior is to show that a GraphQL error has occurred and continue spinning the page spinner indefinitely. This is a pretty bad experience given that the user might not realize that they have provided an invalid filter.
- Modify `Loading` to allow a custom error state. This is used in the error state described above.

## Test Plan

View Runs page. Set `status:foo`, verify that the new error appears instead of the page spinning forever.

Search for valid values, verify that they are retrieved correctly. Search for something that returns no runs, verify that the new empty state appears.
  • Loading branch information
hellendag committed Apr 21, 2022
1 parent 017bd49 commit 768c1b3
Show file tree
Hide file tree
Showing 3 changed files with 61 additions and 14 deletions.
13 changes: 10 additions & 3 deletions js_modules/dagit/packages/core/src/runs/RunTable.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {isAssetGroup} from '../asset-graph/Utils';
import {useSelectionReducer} from '../hooks/useSelectionReducer';
import {PipelineSnapshotLink} from '../pipelines/PipelinePathUtils';
import {PipelineReference} from '../pipelines/PipelineReference';
import {RunsFilter} from '../types/globalTypes';
import {
findRepositoryAmongOptions,
isThisThingAJob,
Expand All @@ -28,6 +29,7 @@ import {RunTableRunFragment} from './types/RunTableRunFragment';

interface RunTableProps {
runs: RunTableRunFragment[];
filter?: RunsFilter;
onSetFilter: (search: RunFilterToken[]) => void;
nonIdealState?: React.ReactNode;
actionBarComponents?: React.ReactNode;
Expand All @@ -37,7 +39,7 @@ interface RunTableProps {
}

export const RunTable = (props: RunTableProps) => {
const {runs, onSetFilter, nonIdealState, highlightedIds, actionBarComponents} = props;
const {runs, filter, onSetFilter, nonIdealState, highlightedIds, actionBarComponents} = props;
const allIds = runs.map((r) => r.runId);

const [{checkedIds}, {onToggleFactory, onToggleAll}] = useSelectionReducer(allIds);
Expand All @@ -48,6 +50,7 @@ export const RunTable = (props: RunTableProps) => {
const {options} = useRepositoryOptions();

if (runs.length === 0) {
const anyFilter = Object.keys(filter || {}).length;
return (
<div>
{actionBarComponents ? (
Expand All @@ -57,8 +60,12 @@ export const RunTable = (props: RunTableProps) => {
{nonIdealState || (
<NonIdealState
icon="run"
title="No runs to display"
description="Use the Launchpad to launch a run."
title={anyFilter ? 'No matching runs' : 'No runs to display'}
description={
anyFilter
? 'No runs were found for this filter.'
: 'Use the Launchpad to launch a run.'
}
/>
)}
</Box>
Expand Down
39 changes: 37 additions & 2 deletions js_modules/dagit/packages/core/src/runs/RunsRoot.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import {gql, useQuery} from '@apollo/client';
import {ApolloError, gql, useQuery} from '@apollo/client';
import {
Alert,
Box,
Expand Down Expand Up @@ -187,7 +187,41 @@ export const RunsRoot = () => {
</Box>
) : null}
<RunsQueryRefetchContext.Provider value={{refetch: queryResult.refetch}}>
<Loading queryResult={queryResult} allowStaleData={true}>
<Loading
queryResult={queryResult}
allowStaleData
renderError={(error: ApolloError) => {
// In this case, a 400 is most likely due to invalid run filters, which are a GraphQL
// validation error but surfaced as a 400.
const badRequest = !!(
error?.networkError &&
'statusCode' in error.networkError &&
error.networkError.statusCode === 400
);
return (
<Box
flex={{direction: 'column', gap: 32}}
padding={{vertical: 8, left: 24, right: 12}}
>
<RunsFilterInput
tokens={mutableTokens}
onChange={setFilterTokensWithStatus}
loading={queryResult.loading}
enabledFilters={enabledFilters}
/>
<NonIdealState
icon="warning"
title={badRequest ? 'Invalid run filters' : 'Unexpected error'}
description={
badRequest
? 'The specified run filters are not valid. Please check the filters and try again.'
: 'An unexpected error occurred. Check the console for details.'
}
/>
</Box>
);
}}
>
{({pipelineRunsOrError}) => {
if (pipelineRunsOrError.__typename !== 'Runs') {
return (
Expand All @@ -210,6 +244,7 @@ export const RunsRoot = () => {
<RunTable
runs={pipelineRunsOrError.results.slice(0, PAGE_SIZE)}
onSetFilter={setFilterTokensWithStatus}
filter={filter}
actionBarComponents={
showScheduled ? null : (
<Box flex={{direction: 'column', gap: 8}}>
Expand Down
23 changes: 14 additions & 9 deletions js_modules/dagit/packages/core/src/ui/Loading.tsx
Original file line number Diff line number Diff line change
@@ -1,18 +1,19 @@
import {QueryResult} from '@apollo/client';
import {ApolloError, QueryResult} from '@apollo/client';
import {Box, NonIdealState, Spinner} from '@dagster-io/ui';
import * as React from 'react';

interface ILoadingProps<TData> {
queryResult: QueryResult<TData, any>;
children: (data: TData) => React.ReactNode;
renderError?: (error: ApolloError) => React.ReactNode;
allowStaleData?: boolean;
purpose: 'section' | 'page';
}

const BLANK_LOADING_DELAY_MSEC = 500;

export const Loading = <TData extends Record<string, any>>(props: ILoadingProps<TData>) => {
const {children, purpose, allowStaleData = false} = props;
const {children, purpose, allowStaleData = false, renderError} = props;
const {error, data, loading} = props.queryResult;

const [blankLoading, setBlankLoading] = React.useState(true);
Expand All @@ -36,13 +37,17 @@ export const Loading = <TData extends Record<string, any>>(props: ILoadingProps<

// either error.networkError or error.graphQLErrors is set,
// so check that the error is not just a transient network error
if (error && !error.networkError) {
console.error(error);
return (
<Box padding={64} flex={{justifyContent: 'center'}}>
<NonIdealState icon="error" title="GraphQL Error - see console for details" />
</Box>
);
if (error) {
if (renderError) {
return <>{renderError(error)}</>;
}
if (!error.networkError) {
return (
<Box padding={64} flex={{justifyContent: 'center'}}>
<NonIdealState icon="error" title="GraphQL Error - see console for details" />
</Box>
);
}
}

if (isLoading) {
Expand Down

0 comments on commit 768c1b3

Please sign in to comment.