aminghadersohi commented on code in PR #42581:
URL: https://github.com/apache/superset/pull/42581#discussion_r3685254528
##########
superset-frontend/src/features/datasets/AddDataset/DatasetPanel/index.tsx:
##########
@@ -78,74 +87,127 @@ const DatasetPanelWrapper = ({
}: IDatasetPanelWrapperProps) => {
const [columnList, setColumnList] = useState<ITableColumn[]>([]);
const [loading, setLoading] = useState(false);
- const [hasError, setHasError] = useState(false);
- const tableNameRef = useRef(tableName);
+ const [error, setError] = useState<SupersetError>();
+ const requestIdRef = useRef(0);
+ const currentRequestRef = useRef<TableMetadataRequest>();
+ const supportsSchemas = database?.supports_schemas;
- const getTableMetadata = async (props: IColumnProps) => {
- const { dbId, tableName, schema } = props;
- setLoading(true);
- setHasColumns?.(false);
- const path = `/api/v1/database/${dbId}/table_metadata/${toQueryString({
- name: tableName,
- catalog,
- schema,
- })}`;
- try {
- const response = await SupersetClient.get({
- endpoint: path,
- });
+ const getTableMetadata = useCallback(
+ async (props: TableMetadataRequest) => {
+ const { dbId, tableName, catalog, schema } = props;
+ requestIdRef.current += 1;
+ const requestId = requestIdRef.current;
+ setLoading(true);
+ setColumnList([]);
+ setError(undefined);
+ setHasColumns?.(false);
+ const path = `/api/v1/database/${dbId}/table_metadata/${toQueryString({
+ name: tableName,
+ catalog,
+ schema,
+ })}`;
+ try {
+ const response = await SupersetClient.get({
+ endpoint: path,
+ });
+
+ if (requestId !== requestIdRef.current) {
+ return;
+ }
- if (isIDatabaseTable(response?.json)) {
- const table: IDatabaseTable = response.json as IDatabaseTable;
- /**
- * The user is able to click other table columns while the http call
for last selected table column is made
- * This check ensures we process the response that matches the last
selected table name and ignore the others
- */
- if (table.name === tableNameRef.current) {
+ const table = isIDatabaseTable(response?.json)
+ ? (response.json as IDatabaseTable)
+ : undefined;
+ if (table?.name === tableName) {
setColumnList(table.columns);
setHasColumns?.(table.columns.length > 0);
- setHasError(false);
- }
- } else {
- setColumnList([]);
- setHasColumns?.(false);
- setHasError(true);
- addDangerToast(
- t(
- 'The API response from %s does not match the IDatabaseTable
interface.',
- path,
- ),
- );
- logging.error(
- t(
+ setError(undefined);
+ } else {
+ const message = t(
'The API response from %s does not match the IDatabaseTable
interface.',
path,
- ),
+ );
+ setColumnList([]);
+ setHasColumns?.(false);
+ setError({
+ error_type: ErrorTypeEnum.GENERIC_BACKEND_ERROR,
+ extra: null,
+ level: 'error',
+ message,
+ });
+ addDangerToast(message);
+ logging.error(message);
+ }
+ } catch (caughtError) {
+ const clientError = await getClientErrorObject(
+ caughtError as Parameters<typeof getClientErrorObject>[0],
);
+
+ if (requestId === requestIdRef.current) {
+ const parsedError = clientError.errors?.[0] ?? {
+ error_type: ErrorTypeEnum.GENERIC_BACKEND_ERROR,
+ extra: null,
+ level: 'error' as const,
+ message: clientError.error,
+ };
Review Comment:
Fixed in fdea5b3306. The fallback now prefers the parsed error, message, and
status text, then guarantees a localized “Unable to load columns for the
selected table.” string. I also added a regression test using an HTTP 500 `{}`
response with the registered `DatabaseErrorMessage` to verify the preview
renders the fallback instead of crashing.
##########
superset-frontend/src/components/ErrorMessage/OAuth2RedirectMessage.tsx:
##########
@@ -103,13 +104,18 @@ export function OAuth2RedirectMessage({
);
const dispatch = useDispatch();
+ const lastHandledTabIdRef = useRef<string>();
useEffect(() => {
const handleOAuthComplete = (tabId?: string) => {
- if (tabId !== extra.tab_id) {
+ if (tabId !== extra.tab_id || tabId === lastHandledTabIdRef.current) {
return;
}
- if (source === 'sqllab' && query) {
+ lastHandledTabIdRef.current = tabId;
+
+ if (errorMitigationFunction) {
+ errorMitigationFunction();
+ } else if (source === 'sqllab' && query) {
dispatch(reRunQuery(query));
Review Comment:
Fixed in fdea5b3306. The tab ID is now recorded only after a
mitigation/rerun/invalidation action can execute; when SQL Lab has no query
yet, the completion is left unconsumed. Added coverage where BroadcastChannel
arrives before the query is available and the storage notification succeeds
after the query appears, plus a dual-transport exactly-once assertion.
##########
superset-frontend/src/features/datasets/AddDataset/DatasetPanel/index.tsx:
##########
@@ -78,74 +87,127 @@ const DatasetPanelWrapper = ({
}: IDatasetPanelWrapperProps) => {
const [columnList, setColumnList] = useState<ITableColumn[]>([]);
const [loading, setLoading] = useState(false);
- const [hasError, setHasError] = useState(false);
- const tableNameRef = useRef(tableName);
+ const [error, setError] = useState<SupersetError>();
+ const requestIdRef = useRef(0);
+ const currentRequestRef = useRef<TableMetadataRequest>();
+ const supportsSchemas = database?.supports_schemas;
- const getTableMetadata = async (props: IColumnProps) => {
- const { dbId, tableName, schema } = props;
- setLoading(true);
- setHasColumns?.(false);
- const path = `/api/v1/database/${dbId}/table_metadata/${toQueryString({
- name: tableName,
- catalog,
- schema,
- })}`;
- try {
- const response = await SupersetClient.get({
- endpoint: path,
- });
+ const getTableMetadata = useCallback(
+ async (props: TableMetadataRequest) => {
+ const { dbId, tableName, catalog, schema } = props;
+ requestIdRef.current += 1;
+ const requestId = requestIdRef.current;
+ setLoading(true);
+ setColumnList([]);
+ setError(undefined);
+ setHasColumns?.(false);
+ const path = `/api/v1/database/${dbId}/table_metadata/${toQueryString({
+ name: tableName,
+ catalog,
+ schema,
+ })}`;
+ try {
+ const response = await SupersetClient.get({
+ endpoint: path,
+ });
+
+ if (requestId !== requestIdRef.current) {
+ return;
+ }
- if (isIDatabaseTable(response?.json)) {
- const table: IDatabaseTable = response.json as IDatabaseTable;
- /**
- * The user is able to click other table columns while the http call
for last selected table column is made
- * This check ensures we process the response that matches the last
selected table name and ignore the others
- */
- if (table.name === tableNameRef.current) {
+ const table = isIDatabaseTable(response?.json)
+ ? (response.json as IDatabaseTable)
+ : undefined;
+ if (table?.name === tableName) {
setColumnList(table.columns);
setHasColumns?.(table.columns.length > 0);
- setHasError(false);
- }
- } else {
- setColumnList([]);
- setHasColumns?.(false);
- setHasError(true);
- addDangerToast(
- t(
- 'The API response from %s does not match the IDatabaseTable
interface.',
- path,
- ),
- );
- logging.error(
- t(
+ setError(undefined);
+ } else {
+ const message = t(
'The API response from %s does not match the IDatabaseTable
interface.',
path,
- ),
+ );
+ setColumnList([]);
+ setHasColumns?.(false);
+ setError({
+ error_type: ErrorTypeEnum.GENERIC_BACKEND_ERROR,
+ extra: null,
+ level: 'error',
+ message,
+ });
+ addDangerToast(message);
+ logging.error(message);
+ }
+ } catch (caughtError) {
+ const clientError = await getClientErrorObject(
+ caughtError as Parameters<typeof getClientErrorObject>[0],
);
+
+ if (requestId === requestIdRef.current) {
+ const parsedError = clientError.errors?.[0] ?? {
+ error_type: ErrorTypeEnum.GENERIC_BACKEND_ERROR,
+ extra: null,
+ level: 'error' as const,
+ message: clientError.error,
Review Comment:
Fixed in fdea5b3306. The generic fallback now always supplies a string
(`clientError.error || clientError.message || clientError.statusText ||` the
localized column-loading fallback). Added an HTTP 500 `{}` regression test
through `DatabaseErrorMessage` to ensure this path renders without throwing.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
