This is an automated email from the ASF dual-hosted git repository.
justinpark pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/superset.git
The following commit(s) were added to refs/heads/master by this push:
new 9b15e04bc4 fix(log): Missing failed query log on async queries (#33024)
9b15e04bc4 is described below
commit 9b15e04bc4bc7729842902dff1c06e940745fcc9
Author: JUST.in DO IT <[email protected]>
AuthorDate: Tue Apr 8 07:12:03 2025 -0700
fix(log): Missing failed query log on async queries (#33024)
---
superset-frontend/spec/fixtures/mockDatabases.ts | 48 +++++++++++++++
superset-frontend/src/SqlLab/actions/sqlLab.js | 30 +++++-----
.../src/SqlLab/actions/sqlLab.test.js | 10 +---
.../QueryAutoRefresh/QueryAutoRefresh.test.tsx | 69 ++++++++++++++++++++--
.../SqlLab/components/QueryAutoRefresh/index.tsx | 30 +++++++++-
5 files changed, 159 insertions(+), 28 deletions(-)
diff --git a/superset-frontend/spec/fixtures/mockDatabases.ts
b/superset-frontend/spec/fixtures/mockDatabases.ts
new file mode 100644
index 0000000000..8d3edc3783
--- /dev/null
+++ b/superset-frontend/spec/fixtures/mockDatabases.ts
@@ -0,0 +1,48 @@
+/**
+ * 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.
+ */
+
+export default {
+ 1: {
+ allow_ctas: false,
+ allow_cvas: false,
+ allow_dml: false,
+ allow_file_upload: false,
+ allow_run_async: true,
+ backend: 'postgresql',
+ database_name: 'examples',
+ expose_in_sqllab: true,
+ force_ctas_schema: null,
+ id: 1,
+ },
+};
+
+export const disabledAsyncDb = {
+ 21: {
+ allow_ctas: false,
+ allow_cvas: false,
+ allow_dml: false,
+ allow_file_upload: false,
+ allow_run_async: false,
+ backend: 'postgresql',
+ database_name: 'examples',
+ expose_in_sqllab: true,
+ force_ctas_schema: null,
+ id: 21,
+ },
+};
diff --git a/superset-frontend/src/SqlLab/actions/sqlLab.js
b/superset-frontend/src/SqlLab/actions/sqlLab.js
index 9f62d781ed..8b54319a56 100644
--- a/superset-frontend/src/SqlLab/actions/sqlLab.js
+++ b/superset-frontend/src/SqlLab/actions/sqlLab.js
@@ -252,28 +252,30 @@ export function querySuccess(query, results) {
return { type: QUERY_SUCCESS, query, results };
}
-export function queryFailed(query, msg, link, errors) {
+export function logFailedQuery(query, errors) {
return function (dispatch) {
const eventData = {
has_err: true,
start_offset: query.startDttm,
ts: new Date().getTime(),
};
- errors?.forEach(({ error_type: errorType, extra }) => {
- const messages = extra?.issue_codes?.map(({ message }) => message) || [
- errorType,
- ];
- messages.forEach(message => {
- dispatch(
- logEvent(LOG_ACTIONS_SQLLAB_FETCH_FAILED_QUERY, {
- ...eventData,
- error_type: errorType,
- error_details: message,
- }),
- );
- });
+ errors?.forEach(({ error_type: errorType, message, extra }) => {
+ const issueCodes = extra?.issue_codes?.map(({ code }) => code) || [-1];
+ dispatch(
+ logEvent(LOG_ACTIONS_SQLLAB_FETCH_FAILED_QUERY, {
+ ...eventData,
+ error_type: errorType,
+ issue_codes: issueCodes,
+ error_details: message,
+ }),
+ );
});
+ };
+}
+export function queryFailed(query, msg, link, errors) {
+ return function (dispatch) {
+ dispatch(logFailedQuery(query, errors));
dispatch({ type: QUERY_FAILED, query, msg, link, errors });
};
}
diff --git a/superset-frontend/src/SqlLab/actions/sqlLab.test.js
b/superset-frontend/src/SqlLab/actions/sqlLab.test.js
index 574350370a..fd0ea8b13f 100644
--- a/superset-frontend/src/SqlLab/actions/sqlLab.test.js
+++ b/superset-frontend/src/SqlLab/actions/sqlLab.test.js
@@ -294,7 +294,7 @@ describe('async actions', () => {
});
it('calls queryFailed on fetch error and logs the error details', () => {
- expect.assertions(3);
+ expect.assertions(2);
fetchMock.post(
runQueryEndpoint,
@@ -312,7 +312,6 @@ describe('async actions', () => {
const expectedActionTypes = [
actions.START_QUERY,
LOG_EVENT,
- LOG_EVENT,
actions.QUERY_FAILED,
];
const { dispatch } = store;
@@ -320,12 +319,7 @@ describe('async actions', () => {
return request(dispatch, () => initialState).then(() => {
const actions = store.getActions();
expect(actions.map(a => a.type)).toEqual(expectedActionTypes);
- expect(actions[1].payload.eventData.error_details).toContain(
- 'Issue 1000',
- );
- expect(actions[2].payload.eventData.error_details).toContain(
- 'Issue 1001',
- );
+ expect(actions[1].payload.eventData.issue_codes).toEqual([1000, 1001]);
});
});
});
diff --git
a/superset-frontend/src/SqlLab/components/QueryAutoRefresh/QueryAutoRefresh.test.tsx
b/superset-frontend/src/SqlLab/components/QueryAutoRefresh/QueryAutoRefresh.test.tsx
index 68353ad1f5..7a35adccc4 100644
---
a/superset-frontend/src/SqlLab/components/QueryAutoRefresh/QueryAutoRefresh.test.tsx
+++
b/superset-frontend/src/SqlLab/components/QueryAutoRefresh/QueryAutoRefresh.test.tsx
@@ -16,10 +16,12 @@
* specific language governing permissions and limitations
* under the License.
*/
+import { QueryState } from '@superset-ui/core';
import fetchMock from 'fetch-mock';
import configureStore from 'redux-mock-store';
import thunk from 'redux-thunk';
import { render, waitFor } from 'spec/helpers/testing-library';
+import { LOG_ACTIONS_SQLLAB_FETCH_FAILED_QUERY } from 'src/logger/LogUtils';
import {
CLEAR_INACTIVE_QUERIES,
REFRESH_QUERIES,
@@ -31,9 +33,13 @@ import QueryAutoRefresh, {
} from 'src/SqlLab/components/QueryAutoRefresh';
import { successfulQuery, runningQuery } from 'src/SqlLab/fixtures';
import { QueryDictionary } from 'src/SqlLab/types';
+import mockDatabases from 'spec/fixtures/mockDatabases';
const middlewares = [thunk];
const mockStore = configureStore(middlewares);
+const mockState = {
+ databases: mockDatabases,
+};
// NOTE: The uses of @ts-ignore in this file is to enable testing of bad
inputs to verify the
// function / component handles bad data elegantly
@@ -106,7 +112,9 @@ describe('QueryAutoRefresh', () => {
});
it('Attempts to refresh when given pending query', async () => {
- const store = mockStore();
+ const store = mockStore({
+ sqlLab: { ...mockState },
+ });
fetchMock.get(refreshApi, {
result: [
{
@@ -135,7 +143,7 @@ describe('QueryAutoRefresh', () => {
});
it('Attempts to clear inactive queries when updated queries are empty',
async () => {
- const store = mockStore();
+ const store = mockStore({ sqlLab: { ...mockState } });
fetchMock.get(refreshApi, {
result: [],
});
@@ -163,7 +171,7 @@ describe('QueryAutoRefresh', () => {
});
it('Does not fail and attempts to refresh when given pending query and
invalid query', async () => {
- const store = mockStore();
+ const store = mockStore({ sqlLab: { ...mockState } });
fetchMock.get(refreshApi, {
result: [
{
@@ -193,7 +201,7 @@ describe('QueryAutoRefresh', () => {
});
it('Does NOT Attempt to refresh when given only completed queries', async ()
=> {
- const store = mockStore();
+ const store = mockStore({ sqlLab: { ...mockState } });
fetchMock.get(refreshApi, {
result: [
{
@@ -220,4 +228,57 @@ describe('QueryAutoRefresh', () => {
);
expect(fetchMock.calls(refreshApi)).toHaveLength(0);
});
+
+ it('logs the failed error for async queries', async () => {
+ const store = mockStore({ sqlLab: { ...mockState } });
+ fetchMock.get(refreshApi, {
+ result: [
+ {
+ id: runningQuery.id,
+ dbId: 1,
+ state: QueryState.Failed,
+ extra: {
+ errors: [
+ {
+ error_type: 'TEST_ERROR',
+ level: 'error',
+ message: 'Syntax invalid',
+ extra: {
+ issue_codes: [
+ {
+ code: 102,
+ message: 'DB failed',
+ },
+ ],
+ },
+ },
+ ],
+ },
+ },
+ ],
+ });
+ render(
+ <QueryAutoRefresh
+ queries={runningQueries}
+ queriesLastUpdate={queriesLastUpdate}
+ />,
+ { useRedux: true, store },
+ );
+ await waitFor(
+ () =>
+ expect(store.getActions()).toContainEqual(
+ expect.objectContaining({
+ payload: expect.objectContaining({
+ eventName: LOG_ACTIONS_SQLLAB_FETCH_FAILED_QUERY,
+ eventData: expect.objectContaining({
+ error_type: 'TEST_ERROR',
+ error_details: 'Syntax invalid',
+ issue_codes: [102],
+ }),
+ }),
+ }),
+ ),
+ { timeout: QUERY_UPDATE_FREQ + 100 },
+ );
+ });
});
diff --git a/superset-frontend/src/SqlLab/components/QueryAutoRefresh/index.tsx
b/superset-frontend/src/SqlLab/components/QueryAutoRefresh/index.tsx
index ca9906a75f..c7de1208b5 100644
--- a/superset-frontend/src/SqlLab/components/QueryAutoRefresh/index.tsx
+++ b/superset-frontend/src/SqlLab/components/QueryAutoRefresh/index.tsx
@@ -17,7 +17,7 @@
* under the License.
*/
import { useRef } from 'react';
-import { useDispatch } from 'react-redux';
+import { useSelector, useDispatch } from 'react-redux';
import { isObject } from 'lodash';
import rison from 'rison';
import {
@@ -25,13 +25,17 @@ import {
Query,
runningQueryStateList,
QueryResponse,
+ QueryState,
+ lruCache,
} from '@superset-ui/core';
-import { QueryDictionary } from 'src/SqlLab/types';
+import { QueryDictionary, SqlLabRootState } from 'src/SqlLab/types';
import useInterval from 'src/SqlLab/utils/useInterval';
import {
refreshQueries,
clearInactiveQueries,
+ logFailedQuery,
} from 'src/SqlLab/actions/sqlLab';
+import type { DatabaseObject } from 'src/features/databases/types';
export const QUERY_UPDATE_FREQ = 2000;
const QUERY_UPDATE_BUFFER_MS = 5000;
@@ -67,6 +71,17 @@ function QueryAutoRefresh({
// pendingRequest check ensures we only have one active http call to check
for query statuses
const pendingRequestRef = useRef(false);
const cleanInactiveRequestRef = useRef(false);
+ const failedQueries = useRef(lruCache(1000));
+ const databases = useSelector<SqlLabRootState>(
+ ({ sqlLab }) => sqlLab.databases,
+ ) as Record<string, DatabaseObject>;
+ const asyncFetchDbs = useRef(
+ new Set(
+ Object.values(databases)
+ .filter(({ allow_run_async }) => Boolean(allow_run_async))
+ .map(({ id }) => id),
+ ),
+ );
const dispatch = useDispatch();
const checkForRefresh = () => {
@@ -97,6 +112,17 @@ function QueryAutoRefresh({
{},
) ?? {};
dispatch(refreshQueries(queries));
+ jsonPayload.result.forEach(query => {
+ const { id, dbId, state } = query;
+ if (
+ asyncFetchDbs.current.has(dbId) &&
+ !failedQueries.current.has(id) &&
+ state === QueryState.Failed
+ ) {
+ dispatch(logFailedQuery(query, query.extra?.errors));
+ failedQueries.current.set(id, true);
+ }
+ });
} else {
dispatch(clearInactiveQueries(QUERY_UPDATE_FREQ));
}
