Skip to content

Commit 904fc19

Browse files
ppsreejithppsreejith
authored
Feature/retry on error (#337)
* Increase max steps * Handle async errors in dashboardState safely * Handle MBQLAppState construction safely * Handle SQL state construction safely * Preserve latest migration version
1 parent 02e887a commit 904fc19

File tree

6 files changed

+83
-39
lines changed

6 files changed

+83
-39
lines changed

‎apps/src/metabase/helpers/DOMToState.ts‎

Lines changed: 29 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import { getParsedIframeInfo, RPCs } from 'web'
22
import { getAllRelevantTablesForSelectedDb, getRelevantTablesForSelectedDb, getTablesWithFields, validateTablesInDB } from './getDatabaseSchema';
3-
import { getAllRelevantModelsForSelectedDb, getDatabaseInfo, getDatabases, getTableData } from './metabaseAPIHelpers';
3+
import { getAllRelevantModelsForSelectedDb, getDatabaseInfo, getDatabases, getTableData, handleEmpty } from './metabaseAPIHelpers';
44
import { getAndFormatOutputTable, getSqlErrorMessage } from './operations';
55
import { DashboardInfo } from './dashboard/types';
66
import { getDashboardAppState } from './dashboard/appState';
@@ -116,13 +116,23 @@ export async function convertDOMtoStateSQLQueryV2(pageType: MetabasePageType, cu
116116
const isEmbedded = getParsedIframeInfo().isEmbedded
117117
const sqlQuery = get(currentCard, 'dataset_query.native.query', '') || ''
118118
const dbId = currentDBId
119-
const limitedEntities = await getLimitedEntities(sqlQuery, currentDBId);
120-
const selectedDatabaseInfo = dbId ? await getDatabaseInfo(dbId) : undefined;
119+
const selectedDatabaseInfo = dbId ? await handleEmpty(getDatabaseInfo(dbId), {
120+
name: 'Unknown Database',
121+
description: '',
122+
id: dbId,
123+
dialect: 'unknown',
124+
dbms_version: {
125+
flavor: 'unknown',
126+
version: 'unknown',
127+
semantic_version: [0, 0, 0]
128+
}
129+
}) : undefined;
130+
const limitedEntities = await handleEmpty(getLimitedEntities(sqlQuery, currentDBId), []);
121131
const sqlErrorMessage = await getSqlErrorMessage();
122132
const appStateType = pageType === 'sql' ? MetabaseAppStateType.SQLEditor : MetabaseAppStateType.RandomPage;
123133
let relevantTablesWithFields: FormattedTable[] = []
124134
{
125-
const dbTables = await getAllRelevantTablesForSelectedDb(dbId || 0)
135+
const dbTables = await handleEmpty(getAllRelevantTablesForSelectedDb(dbId || 0), [])
126136
const sqlTables = await getTablesFromSqlRegex(sqlQuery)
127137
const defaultSchema = selectedDatabaseInfo?.default_schema
128138
// Apply default schema to tables if needed
@@ -158,9 +168,19 @@ export async function convertDOMtoStateSQLQuery(currentDBId: number) {
158168
// const dbId = _.get(hashMetadata, 'dataset_query.database');
159169
const fullUrl = await RPCs.queryURL();
160170
const url = new URL(fullUrl).origin;
161-
const availableDatabases = (await getDatabases())?.data?.map(({ name }) => name);
171+
const availableDatabases = (await handleEmpty(getDatabases(), { total: 0, data: [] }))?.data?.map(({ name }) => name);
162172
const dbId = currentDBId;
163-
const selectedDatabaseInfo = dbId ? await getDatabaseInfo(dbId) : undefined;
173+
const selectedDatabaseInfo = dbId ? await handleEmpty(getDatabaseInfo(dbId), {
174+
name: 'Unknown Database',
175+
description: '',
176+
id: dbId,
177+
dialect: 'unknown',
178+
dbms_version: {
179+
flavor: 'unknown',
180+
version: 'unknown',
181+
semantic_version: [0, 0, 0]
182+
}
183+
}) : undefined;
164184
const defaultSchema = selectedDatabaseInfo?.default_schema;
165185
let sqlQuery = await getCurrentQuery();
166186
const appSettings = RPCs.getAppSettings();
@@ -173,17 +193,17 @@ export async function convertDOMtoStateSQLQuery(currentDBId: number) {
173193
}
174194
})
175195
}
176-
let relevantTablesWithFields = await getTablesWithFields(appSettings.tableDiff, appSettings.drMode, false, sqlTables, [], dbId);
196+
let relevantTablesWithFields = await handleEmpty(getTablesWithFields(appSettings.tableDiff, appSettings.drMode, false, sqlTables, [], dbId), []);
177197
// add defaultSchema back to relevantTablesWithFields. kind of hacky but whatever
178198
relevantTablesWithFields = relevantTablesWithFields.map(table => {
179199
if (table.schema === undefined || table.schema === '') {
180200
table.schema = defaultSchema || 'unknown'
181201
}
182202
return table
183203
})
184-
const allModels = dbId ? await getAllRelevantModelsForSelectedDb(dbId) : []
204+
const allModels = dbId ? await handleEmpty(getAllRelevantModelsForSelectedDb(dbId), []) : []
185205
const relevantModels = await getSelectedAndRelevantModels(sqlQuery || "", appSettings.selectedModels, allModels)
186-
const relevantModelsWithFields = await getModelsWithFields(relevantModels)
206+
const relevantModelsWithFields = await handleEmpty(getModelsWithFields(relevantModels), [])
187207
const allFormattedTables = [...relevantTablesWithFields, ...relevantModelsWithFields]
188208
const tableContextYAML = getTableContextYAML(allFormattedTables, null, appSettings.drMode);
189209
sqlQuery = modifySqlForMetabaseModels(sqlQuery || "", relevantModels)

‎apps/src/metabase/helpers/dashboard/appState.ts‎

Lines changed: 31 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import { DashboardInfo, DashboardMetabaseState } from './types';
22
import _, { forEach, reduce, template, values } from 'lodash';
33
import { MetabaseAppStateDashboard} from '../DOMToState';
4-
import { getAllRelevantModelsForSelectedDb, getDatabaseInfo, getFieldResolvedName } from '../metabaseAPIHelpers';
4+
import { getAllRelevantModelsForSelectedDb, getDatabaseInfo, getFieldResolvedName, handleEmpty } from '../metabaseAPIHelpers';
55
import { getDashboardState, getSelectedDbId } from '../metabaseStateAPI';
66
import { getParsedIframeInfo, RPCs } from 'web';
77
import { getSQLFromMBQL, fetchCard } from '../metabaseAPI';
@@ -218,10 +218,10 @@ function substituteParameterMappings(
218218
return sql
219219
}
220220

221-
async function getDashcardwithOutputTableMd(
221+
function getDashcardwithOutputTableMd(
222222
dashboardMetabaseState: DashboardMetabaseState,
223223
dashcardId: number,
224-
dashboardId: number): Promise<DashboardInfoForModelling['cards'][number] | null> {
224+
dashboardId: number): DashboardInfoForModelling['cards'][number] | null {
225225
const dashcard = dashboardMetabaseState.dashcards[dashcardId].card;
226226
if (!dashcard) {
227227
return null;
@@ -335,7 +335,17 @@ export async function getDashboardAppState(currentDBId: number): Promise<Metabas
335335
const fullUrl = await RPCs.queryURL();
336336
const url = new URL(fullUrl).origin;
337337
const dbId = currentDBId
338-
const selectedDatabaseInfo = dbId ? await getDatabaseInfo(dbId) : undefined
338+
const selectedDatabaseInfo = dbId ? await handleEmpty(getDatabaseInfo(dbId), {
339+
name: 'Unknown Database',
340+
description: '',
341+
id: dbId,
342+
dialect: 'unknown',
343+
dbms_version: {
344+
flavor: 'unknown',
345+
version: 'unknown',
346+
semantic_version: [0, 0, 0]
347+
}
348+
}) : undefined
339349
const dashboardMetabaseState: DashboardMetabaseState = await getDashboardState() as DashboardMetabaseState;
340350
if (!dashboardMetabaseState || !dashboardMetabaseState.dashboards || !dashboardMetabaseState.dashboardId) {
341351
console.warn('Could not get dashboard info');
@@ -355,23 +365,23 @@ export async function getDashboardAppState(currentDBId: number): Promise<Metabas
355365
}
356366
const selectedTabDashcardIds = getSelectedTabDashcardIds(dashboardMetabaseState);
357367
// const dashboardParameters = _.get(dashboardMetabaseState, ['dashboards', dashboardId, 'parameters'], [])
358-
const cards = await Promise.all(selectedTabDashcardIds.map(async dashcardId => await getDashcardwithOutputTableMd(dashboardMetabaseState, dashcardId, dashboardId)))
368+
const cards = selectedTabDashcardIds.map(dashcardId => getDashcardwithOutputTableMd(dashboardMetabaseState, dashcardId, dashboardId))
359369
let filteredCards = _.compact(cards);
360-
const limitedEntitiesSQL = await getLimitedEntitiesFromQueries(
361-
filteredCards.flatMap(card =>
362-
card?.dataset_query?.native?.query ? [card.dataset_query.native.query] : []
363-
),
364-
dbId
365-
);
366-
const limitedEntitiesMBQL = await getLimitedEntitiesFromMBQLQueries(
367-
filteredCards.flatMap(card =>
368-
card?.dataset_query?.query ? [card.dataset_query.query] : []
369-
),
370-
dbId
371-
);
372-
const limitedEntities = [...limitedEntitiesSQL, ...limitedEntitiesMBQL];
373-
// remove duplicates based on id and type
374-
const uniqueEntities = Array.from(new Map(limitedEntities.map(entity => [entity.id, entity])).values());
370+
const limitedEntitiesSQL = await handleEmpty(getLimitedEntitiesFromQueries(
371+
filteredCards.flatMap(card =>
372+
card?.dataset_query?.native?.query ? [card.dataset_query.native.query] : []
373+
),
374+
dbId
375+
), []);
376+
const limitedEntitiesMBQL = await handleEmpty(getLimitedEntitiesFromMBQLQueries(
377+
filteredCards.flatMap(card =>
378+
card?.dataset_query?.query ? [card.dataset_query.query] : []
379+
),
380+
dbId
381+
), []);
382+
const limitedEntities = [...limitedEntitiesSQL, ...limitedEntitiesMBQL];
383+
// remove duplicates based on id and type
384+
const uniqueEntities = Array.from(new Map(limitedEntities.map(entity => [entity.id, entity])).values());
375385
const dashboardAppState: MetabaseAppStateDashboard = {
376386
...dashboardInfo,
377387
type: MetabaseAppStateType.Dashboard,
@@ -421,7 +431,7 @@ export async function getDashboardAppState(currentDBId: number): Promise<Metabas
421431
}
422432

423433
filteredCards = await Promise.all(
424-
filteredCards.map(card => processMBQLCard(card, dbId, cardDetailsMap))
434+
filteredCards.map(card => handleEmpty(processMBQLCard(card, dbId, cardDetailsMap), card))
425435
);
426436

427437
} catch (error) {

‎apps/src/metabase/helpers/mbql/appState.ts‎

Lines changed: 17 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { MetabaseAppStateMBQLEditor} from '../DOMToState';
22
import { getParsedIframeInfo, RPCs } from 'web';
33
import { MBQLInfo, getSourceTableIds } from './utils';
44
import { getMBQLState, getSelectedDbId } from '../metabaseStateAPI';
5-
import { getAllRelevantModelsForSelectedDb, getDatabaseInfo } from '../metabaseAPIHelpers';
5+
import { getAllRelevantModelsForSelectedDb, getDatabaseInfo, handleEmpty } from '../metabaseAPIHelpers';
66
import { get, find } from 'lodash';
77
import { getTableContextYAML } from '../catalog';
88
import { getTablesWithFields } from '../getDatabaseSchema';
@@ -18,21 +18,31 @@ export async function getMBQLAppState(currentDBId: number): Promise<MetabaseAppS
1818
const appSettings = RPCs.getAppSettings();
1919
const selectedCatalog = get(find(appSettings.availableCatalogs, { name: appSettings.selectedCatalog }), 'content')
2020
const dbId = currentDBId
21-
const selectedDatabaseInfo = dbId ? await getDatabaseInfo(dbId) : undefined
21+
const selectedDatabaseInfo = dbId ? await handleEmpty(getDatabaseInfo(dbId), {
22+
name: 'Unknown Database',
23+
description: '',
24+
id: dbId,
25+
dialect: 'unknown',
26+
dbms_version: {
27+
flavor: 'unknown',
28+
version: 'unknown',
29+
semantic_version: [0, 0, 0]
30+
}
31+
}) : undefined
2232
const defaultSchema = selectedDatabaseInfo?.default_schema;
2333
const mbqlState = await getMBQLState();
2434
const outputTableMarkdown = await getAndFormatOutputTable()
2535
const mbqlInfo: MBQLInfo = {
2636
mbqlQuery: mbqlState.dataset_query.query,
2737
outputTableMarkdown
2838
}
29-
39+
3040
const sourceTableModelIds = getSourceTableIds(mbqlState?.dataset_query?.query);
31-
const allModels = dbId ? await getAllRelevantModelsForSelectedDb(dbId) : []
41+
const allModels = dbId ? await handleEmpty(getAllRelevantModelsForSelectedDb(dbId), []) : []
3242
const relevantModels = await getSelectedAndRelevantModels('', appSettings.selectedModels, allModels, sourceTableModelIds)
33-
const relevantModelsWithFields = await getModelsWithFields(relevantModels)
34-
35-
let relevantTablesWithFields = await getTablesWithFields(appSettings.tableDiff, appSettings.drMode, !!selectedCatalog, [], sourceTableModelIds, dbId);
43+
const relevantModelsWithFields = await handleEmpty(getModelsWithFields(relevantModels), [])
44+
45+
let relevantTablesWithFields = await handleEmpty(getTablesWithFields(appSettings.tableDiff, appSettings.drMode, !!selectedCatalog, [], sourceTableModelIds, dbId), []);
3646
relevantTablesWithFields = relevantTablesWithFields.map(table => {
3747
if (table.schema === undefined || table.schema === '') {
3848
table.schema = defaultSchema || 'unknown'

‎apps/src/metabase/helpers/metabaseAPIHelpers.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -385,6 +385,10 @@ export async function getDatabaseTablesAndModelsWithoutFields(db_id: number, for
385385
};
386386
}
387387

388+
export function handleEmpty<T>(promise: Promise<T>, defaultValue: T): Promise<T> {
389+
return Promise.resolve(promise).catch(() => defaultValue);
390+
}
391+
388392
export async function getDatabaseInfo(dbId: number) {
389393
const jsonResponse = await fetchDatabaseInfo({ db_id: dbId });
390394
const defaultSchema = getDefaultSchema(jsonResponse);

‎web/src/planner/planner.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ startListening({
9898
// check if we need to continue tool calls. maybe should have a counter
9999
// here to limit the number of iterations?
100100
let _steps = 0
101-
const MAX_STEPS = 8
101+
const MAX_STEPS = 12
102102
while (shouldContinue(getState)) {
103103
_steps += 1
104104
if (_steps > MAX_STEPS) {

‎web/src/state/store.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -591,7 +591,7 @@ const BLACKLIST = ['billing', 'cache', userStateApi.reducerPath, atlasApi.reduce
591591

592592
const persistConfig = {
593593
key: 'root',
594-
version: 65,
594+
version: 64,
595595
storage,
596596
blacklist: BLACKLIST,
597597
// @ts-ignore

0 commit comments

Comments
 (0)