Skip to content

Commit

Permalink
Fix useTasks crash on error (apache#24152)
Browse files Browse the repository at this point in the history
* Prevent UI from crashing on Get API error

* add test

* don't show API errors in test logs

* use setMinutes inline
  • Loading branch information
bbovenzi authored Jun 6, 2022
1 parent 80c1ce7 commit fd4e344
Show file tree
Hide file tree
Showing 8 changed files with 142 additions and 8 deletions.
15 changes: 15 additions & 0 deletions airflow/www/jest-setup.js
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,21 @@
*/

import '@testing-library/jest-dom';
import axios from 'axios';
import { setLogger } from 'react-query';

axios.defaults.adapter = require('axios/lib/adapters/http');

axios.interceptors.response.use(
(res) => res.data || res,
);

setLogger({
log: console.log,
warn: console.warn,
// ✅ no more errors on the console
error: () => {},
});

// Mock global objects we use across the app
global.stateColors = {
Expand Down
1 change: 1 addition & 0 deletions airflow/www/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@
"mini-css-extract-plugin": "1.6.0",
"moment": "^2.29.2",
"moment-locales-webpack-plugin": "^1.2.0",
"nock": "^13.2.4",
"optimize-css-assets-webpack-plugin": "6.0.0",
"style-loader": "^1.2.1",
"stylelint": "^13.6.1",
Expand Down
3 changes: 3 additions & 0 deletions airflow/www/static/js/grid/api/useMappedInstances.js
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,10 @@ export default function useMappedInstances({
}),
{
keepPreviousData: true,
initialData: { taskInstances: [], totalEntries: 0 },
refetchInterval: isRefreshOn && autoRefreshInterval * 1000,
// staleTime should be similar to the refresh interval
staleTime: autoRefreshInterval * 1000,
},
);
}
9 changes: 5 additions & 4 deletions airflow/www/static/js/grid/api/useTasks.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,15 @@ import axios from 'axios';
import { useQuery } from 'react-query';
import { getMetaValue } from '../../utils';

const tasksUrl = getMetaValue('tasks_api');

export default function useTasks() {
return useQuery(
'tasks',
() => axios.get(tasksUrl),
() => {
const tasksUrl = getMetaValue('tasks_api');
return axios.get(tasksUrl);
},
{
placeholderData: { tasks: [] },
initialData: { tasks: [], totalEntries: 0 },
},
);
}
89 changes: 89 additions & 0 deletions airflow/www/static/js/grid/api/useTasks.test.jsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,89 @@
/*!
* Licensed to the Apache Software Foundation (ASF) under one
* or more contributor license agreements. See the NOTICE file
* distributed with this work for additional information
* regarding copyright ownership. The ASF licenses this file
* to you under the Apache License, Version 2.0 (the
* "License"); you may not use this file except in compliance
* with the License. You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing,
* software distributed under the License is distributed on an
* "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
* KIND, either express or implied. See the License for the
* specific language governing permissions and limitations
* under the License.
*/

/* global describe, test, expect, beforeEach, afterEach, jest */

import React from 'react';
import { renderHook } from '@testing-library/react-hooks';
import { QueryClient, QueryClientProvider } from 'react-query';
import nock from 'nock';

import useTasks from './useTasks';
import * as metaUtils from '../../utils';

const Wrapper = ({ children }) => {
const queryClient = new QueryClient({
defaultOptions: {
queries: {
retry: 0,
},
},
});
return (
<QueryClientProvider client={queryClient}>
{children}
</QueryClientProvider>
);
};

const fakeUrl = 'http://fake.api';

describe('Test useTasks hook', () => {
let spy;
beforeEach(() => {
spy = jest.spyOn(metaUtils, 'getMetaValue').mockReturnValue(`${fakeUrl}`);
});

afterEach(() => {
spy.mockRestore();
nock.cleanAll();
});

test('initialData works normally', async () => {
const scope = nock(fakeUrl)
.get('/')
.reply(200, { totalEntries: 1, tasks: [{ taskId: 'task_id' }] });
const { result, waitFor } = renderHook(() => useTasks(), { wrapper: Wrapper });

expect(result.current.data.totalEntries).toBe(0);

await waitFor(() => result.current.isFetching);

expect(result.current.data.totalEntries).toBe(0);

await waitFor(() => !result.current.isFetching);

expect(result.current.data.totalEntries).toBe(1);
scope.done();
});

test('initialData persists even if there is an error', async () => {
const scope = nock(fakeUrl)
.get('/')
.replyWithError('something awful happened');
const { result, waitFor } = renderHook(() => useTasks(), { wrapper: Wrapper });

expect(result.current.data.totalEntries).toBe(0);

await waitFor(() => result.current.isError);

expect(result.current.data.totalEntries).toBe(0);
scope.done();
});
});
4 changes: 1 addition & 3 deletions airflow/www/static/js/grid/details/content/Dag.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -40,10 +40,8 @@ import { SimpleStatus } from '../../components/StatusBox';
const dagDetailsUrl = getMetaValue('dag_details_url');

const Dag = () => {
const { data: taskData } = useTasks();
const { data: { tasks, totalEntries } } = useTasks();
const { data: { dagRuns } } = useGridData();
if (!taskData) return null;
const { tasks = [], totalEntries = '' } = taskData;

// Build a key/value object of operator counts, the name is hidden inside of t.classRef.className
const operators = {};
Expand Down
4 changes: 3 additions & 1 deletion airflow/www/static/js/grid/index.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -45,11 +45,13 @@ shadowRoot.appendChild(mainElement);
const queryClient = new QueryClient({
defaultOptions: {
queries: {
notifyOnChangeProps: 'tracked',
refetchOnWindowFocus: false,
retry: 1,
retryDelay: 500,
staleTime: 5 * 60 * 1000, // 5 minutes
refetchOnMount: true, // Refetches stale queries, not "always"
staleTime: 5 * 60 * 1000, // 5 minutes
initialDataUpdatedAt: new Date().setMinutes(-6), // make sure initial data is already expired
},
mutations: {
retry: 1,
Expand Down
25 changes: 25 additions & 0 deletions airflow/www/yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -7794,6 +7794,11 @@ json-stable-stringify-without-jsonify@^1.0.1:
resolved "https://registry.yarnpkg.com/json-stable-stringify-without-jsonify/-/json-stable-stringify-without-jsonify-1.0.1.tgz#9db7b59496ad3f3cfef30a75142d2d930ad72651"
integrity sha1-nbe1lJatPzz+8wp1FC0tkwrXJlE=

json-stringify-safe@^5.0.1:
version "5.0.1"
resolved "https://registry.yarnpkg.com/json-stringify-safe/-/json-stringify-safe-5.0.1.tgz#1296a2d58fd45f19a0f6ce01d65701e2c735b6eb"
integrity sha512-ZClg6AaYvamvYEE82d3Iyd3vSSIjQ+odgjaTzRuO3s7toCdFKczob2i0zCh7JE8kWn17yvAWhUVxvqGwUalsRA==

json5@^0.5.0, json5@^0.5.1:
version "0.5.1"
resolved "https://registry.yarnpkg.com/json5/-/json5-0.5.1.tgz#1eade7acc012034ad84e2396767ead9fa5495821"
Expand Down Expand Up @@ -8009,6 +8014,11 @@ lodash.mergewith@4.6.2:
resolved "https://registry.yarnpkg.com/lodash.mergewith/-/lodash.mergewith-4.6.2.tgz#617121f89ac55f59047c7aec1ccd6654c6590f55"
integrity sha512-GK3g5RPZWTRSeLSpgP8Xhra+pnjBC56q9FZYe1d5RN3TJ35dbkGy3YqBSMbyCrlbi+CM9Z3Jk5yTL7RCsqboyQ==

lodash.set@^4.3.2:
version "4.3.2"
resolved "https://registry.yarnpkg.com/lodash.set/-/lodash.set-4.3.2.tgz#d8757b1da807dde24816b0d6a84bea1a76230b23"
integrity sha512-4hNPN5jlm/N/HLMCO43v8BXKq9Z7QdAGc/VGrRD61w8gN9g/6jF9A4L1pbUgBLCffi0w9VsXfTOij5x8iTyFvg==

lodash.truncate@^4.4.2:
version "4.4.2"
resolved "https://registry.yarnpkg.com/lodash.truncate/-/lodash.truncate-4.4.2.tgz#5a350da0b1113b837ecfffd5812cbe58d6eae193"
Expand Down Expand Up @@ -8543,6 +8553,16 @@ nice-try@^1.0.4:
resolved "https://registry.yarnpkg.com/nice-try/-/nice-try-1.0.5.tgz#a3378a7696ce7d223e88fc9b764bd7ef1089e366"
integrity sha512-1nh45deeb5olNY7eX82BkPO7SSxR5SSYJiPTrTdFUVYwAl8CKMA5N9PjTYkHiRjisVcxcQ1HXdLhx2qxxJzLNQ==

nock@^13.2.4:
version "13.2.4"
resolved "https://registry.yarnpkg.com/nock/-/nock-13.2.4.tgz#43a309d93143ee5cdcca91358614e7bde56d20e1"
integrity sha512-8GPznwxcPNCH/h8B+XZcKjYPXnUV5clOKCjAqyjsiqA++MpNx9E9+t8YPp0MbThO+KauRo7aZJ1WuIZmOrT2Ug==
dependencies:
debug "^4.1.0"
json-stringify-safe "^5.0.1"
lodash.set "^4.3.2"
propagate "^2.0.0"

node-domexception@^1.0.0:
version "1.0.0"
resolved "https://registry.yarnpkg.com/node-domexception/-/node-domexception-1.0.0.tgz#6888db46a1f71c0b76b3f7555016b63fe64766e5"
Expand Down Expand Up @@ -9658,6 +9678,11 @@ prop-types@^15.6.2, prop-types@^15.7.2:
object-assign "^4.1.1"
react-is "^16.13.1"

propagate@^2.0.0:
version "2.0.1"
resolved "https://registry.yarnpkg.com/propagate/-/propagate-2.0.1.tgz#40cdedab18085c792334e64f0ac17256d38f9a45"
integrity sha512-vGrhOavPSTz4QVNuBNdcNXePNdNMaO1xj9yBeH1ScQPjk/rhg9sSlCXPhMkFuaNNW/syTvYqsnbIJxMBfRbbag==

prr@~1.0.1:
version "1.0.1"
resolved "https://registry.yarnpkg.com/prr/-/prr-1.0.1.tgz#d3fc114ba06995a45ec6893f484ceb1d78f5f476"
Expand Down

0 comments on commit fd4e344

Please sign in to comment.