From e1701eea1601e7fe1fb58132d47a9f2d70551b65 Mon Sep 17 00:00:00 2001 From: hainenber Date: Tue, 18 Feb 2025 14:12:45 +0700 Subject: [PATCH 1/6] fix(embed): prevent random crash for filters bar in embedded dashboard Text containing single quote might not be JSON-parseable. If consumed as bootstrap data, it'll crash over. Signed-off-by: hainenber --- .../components/Header/Header.test.tsx | 2 +- .../components/RefreshIntervalModal.test.tsx | 24 +++++++++---------- superset/config.py | 2 +- 3 files changed, 14 insertions(+), 14 deletions(-) diff --git a/superset-frontend/src/dashboard/components/Header/Header.test.tsx b/superset-frontend/src/dashboard/components/Header/Header.test.tsx index e6d526f4089e..278ea446fb78 100644 --- a/superset-frontend/src/dashboard/components/Header/Header.test.tsx +++ b/superset-frontend/src/dashboard/components/Header/Header.test.tsx @@ -41,7 +41,7 @@ const initialState = { common: { conf: { DASHBOARD_AUTO_REFRESH_INTERVALS: [ - [0, "Don't refresh"], + [0, 'Do not refresh'], [10, '10 seconds'], ], }, diff --git a/superset-frontend/src/dashboard/components/RefreshIntervalModal.test.tsx b/superset-frontend/src/dashboard/components/RefreshIntervalModal.test.tsx index 6ef0299665cd..39cd00b5fbaf 100644 --- a/superset-frontend/src/dashboard/components/RefreshIntervalModal.test.tsx +++ b/superset-frontend/src/dashboard/components/RefreshIntervalModal.test.tsx @@ -41,7 +41,7 @@ const createProps = () => ({ common: { conf: { DASHBOARD_AUTO_REFRESH_INTERVALS: [ - [0, "Don't refresh"], + [0, 'Do not refresh'], [10, '10 seconds'], [30, '30 seconds'], [60, '1 minute'], @@ -113,7 +113,7 @@ const openRefreshIntervalModal = async () => { const displayOptions = async () => { // Click default refresh interval option to display other options - userEvent.click(screen.getByText(/don't refresh/i)); + userEvent.click(screen.getByText(/do not refresh/i)); }; const defaultRefreshIntervalModalProps = { @@ -148,9 +148,9 @@ test('renders refresh interval options', async () => { await openRefreshIntervalModal(); await displayOptions(); - // Assert that both "Don't refresh" instances exist + // Assert that both "Do not refresh" instances exist // - There will be two at this point, the default option and the dropdown option - const dontRefreshInstances = screen.getAllByText(/don't refresh/i); + const dontRefreshInstances = screen.getAllByText(/do not refresh/i); expect(dontRefreshInstances).toHaveLength(2); dontRefreshInstances.forEach(option => { expect(option).toBeInTheDocument(); @@ -177,9 +177,9 @@ test('should change selected value', async () => { render(setup(editModeOnProps)); await openRefreshIntervalModal(); - // Initial selected value should be "Don't refresh" - const selectedValue = screen.getByText(/don't refresh/i); - expect(selectedValue.title).toMatch(/don't refresh/i); + // Initial selected value should be "Do not refresh" + const selectedValue = screen.getByText(/do not refresh/i); + expect(selectedValue.title).toMatch(/do not refresh/i); // Display options and select "10 seconds" await displayOptions(); @@ -187,16 +187,16 @@ test('should change selected value', async () => { // Selected value should now be "10 seconds" expect(selectedValue.title).toMatch(/10 seconds/i); - expect(selectedValue.title).not.toMatch(/don't refresh/i); + expect(selectedValue.title).not.toMatch(/do not refresh/i); }); test('should change selected value to custom value', async () => { render(setup(editModeOnProps)); await openRefreshIntervalModal(); - // Initial selected value should be "Don't refresh" - const selectedValue = screen.getByText(/don't refresh/i); - expect(selectedValue.title).toMatch(/don't refresh/i); + // Initial selected value should be "Do not refresh" + const selectedValue = screen.getByText(/do not refresh/i); + expect(selectedValue.title).toMatch(/do not refresh/i); // Display options and select "Custom interval" await displayOptions(); @@ -204,7 +204,7 @@ test('should change selected value to custom value', async () => { // Selected value should now be "Custom interval" expect(selectedValue.title).toMatch(/Custom interval/i); - expect(selectedValue.title).not.toMatch(/don't refresh/i); + expect(selectedValue.title).not.toMatch(/do not refresh/i); }); test('should save a newly-selected value', async () => { diff --git a/superset/config.py b/superset/config.py index 6362e39aec7e..668409b10d87 100644 --- a/superset/config.py +++ b/superset/config.py @@ -983,7 +983,7 @@ class D3TimeFormat(TypedDict, total=False): DASHBOARD_AUTO_REFRESH_MODE: Literal["fetch", "force"] = "force" # Dashboard auto refresh intervals DASHBOARD_AUTO_REFRESH_INTERVALS = [ - [0, "Don't refresh"], + [0, "Do not refresh"], [10, "10 seconds"], [30, "30 seconds"], [60, "1 minute"], From fb020b60fc83765556755edab3705fa3a750047f Mon Sep 17 00:00:00 2001 From: hainenber Date: Tue, 18 Feb 2025 15:09:18 +0700 Subject: [PATCH 2/6] fix(embed): repair broken bootstrap data before parsing as json Signed-off-by: hainenber --- superset-frontend/package-lock.json | 10 ++++++++++ superset-frontend/package.json | 1 + superset-frontend/src/utils/getBootstrapData.ts | 5 ++++- 3 files changed, 15 insertions(+), 1 deletion(-) diff --git a/superset-frontend/package-lock.json b/superset-frontend/package-lock.json index 02aa9563cd93..bda69142ad3d 100644 --- a/superset-frontend/package-lock.json +++ b/superset-frontend/package-lock.json @@ -90,6 +90,7 @@ "js-yaml-loader": "^1.2.2", "json-bigint": "^1.0.0", "json-stringify-pretty-compact": "^2.0.0", + "jsonrepair": "^3.12.0", "lodash": "^4.17.21", "luxon": "^3.5.0", "mapbox-gl": "^2.10.0", @@ -30368,6 +30369,15 @@ "node": ">=0.10.0" } }, + "node_modules/jsonrepair": { + "version": "3.12.0", + "resolved": "https://registry.npmjs.org/jsonrepair/-/jsonrepair-3.12.0.tgz", + "integrity": "sha512-SWfjz8SuQ0wZjwsxtSJ3Zy8vvLg6aO/kxcp9TWNPGwJKgTZVfhNEQBMk/vPOpYCDFWRxD6QWuI6IHR1t615f0w==", + "license": "ISC", + "bin": { + "jsonrepair": "bin/cli.js" + } + }, "node_modules/JSONStream": { "version": "1.3.5", "resolved": "https://registry.npmjs.org/JSONStream/-/JSONStream-1.3.5.tgz", diff --git a/superset-frontend/package.json b/superset-frontend/package.json index a5853a86474c..bff7fe1eae65 100644 --- a/superset-frontend/package.json +++ b/superset-frontend/package.json @@ -157,6 +157,7 @@ "js-yaml-loader": "^1.2.2", "json-bigint": "^1.0.0", "json-stringify-pretty-compact": "^2.0.0", + "jsonrepair": "^3.12.0", "lodash": "^4.17.21", "luxon": "^3.5.0", "mapbox-gl": "^2.10.0", diff --git a/superset-frontend/src/utils/getBootstrapData.ts b/superset-frontend/src/utils/getBootstrapData.ts index e426498e5738..05fb424fe13f 100644 --- a/superset-frontend/src/utils/getBootstrapData.ts +++ b/superset-frontend/src/utils/getBootstrapData.ts @@ -17,11 +17,14 @@ * under the License. */ +import { jsonrepair } from 'jsonrepair'; import { BootstrapData } from 'src/types/bootstrapTypes'; import { DEFAULT_BOOTSTRAP_DATA } from 'src/constants'; export default function getBootstrapData(): BootstrapData { const appContainer = document.getElementById('app'); const dataBootstrap = appContainer?.getAttribute('data-bootstrap'); - return dataBootstrap ? JSON.parse(dataBootstrap) : DEFAULT_BOOTSTRAP_DATA; + return dataBootstrap + ? JSON.parse(jsonrepair(dataBootstrap)) + : DEFAULT_BOOTSTRAP_DATA; } From 6265fff111dab9c69f8432499d27242816287613 Mon Sep 17 00:00:00 2001 From: hainenber Date: Tue, 18 Feb 2025 15:22:30 +0700 Subject: [PATCH 3/6] fix(ci): increase timeout for slow UploadDataModal Jest test Signed-off-by: hainenber --- .../features/databases/UploadDataModel/UploadDataModal.test.tsx | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/superset-frontend/src/features/databases/UploadDataModel/UploadDataModal.test.tsx b/superset-frontend/src/features/databases/UploadDataModel/UploadDataModal.test.tsx index bf83445dbf29..60540f4ba290 100644 --- a/superset-frontend/src/features/databases/UploadDataModel/UploadDataModal.test.tsx +++ b/superset-frontend/src/features/databases/UploadDataModel/UploadDataModal.test.tsx @@ -567,7 +567,7 @@ test('Columnar, does not render the rows', () => { }); test('database and schema are correctly populated', async () => { - jest.setTimeout(10000); + jest.setTimeout(20000); render(, { useRedux: true, }); From cd8bc60feca5b98014b68fe1b99d539b4da21eff Mon Sep 17 00:00:00 2001 From: hainenber Date: Sat, 22 Feb 2025 00:05:28 +0700 Subject: [PATCH 4/6] feat(frontend/utils): parse potentially invalid bootstrap data before reparse the fixed version Signed-off-by: hainenber --- superset-frontend/src/utils/getBootstrapData.ts | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/superset-frontend/src/utils/getBootstrapData.ts b/superset-frontend/src/utils/getBootstrapData.ts index 05fb424fe13f..d34fd51c8a26 100644 --- a/superset-frontend/src/utils/getBootstrapData.ts +++ b/superset-frontend/src/utils/getBootstrapData.ts @@ -23,8 +23,13 @@ import { DEFAULT_BOOTSTRAP_DATA } from 'src/constants'; export default function getBootstrapData(): BootstrapData { const appContainer = document.getElementById('app'); - const dataBootstrap = appContainer?.getAttribute('data-bootstrap'); - return dataBootstrap - ? JSON.parse(jsonrepair(dataBootstrap)) - : DEFAULT_BOOTSTRAP_DATA; + const dataBootstrapString = appContainer?.getAttribute('data-bootstrap'); + if (!dataBootstrapString) { + return DEFAULT_BOOTSTRAP_DATA; + } + try { + return JSON.parse(dataBootstrapString); + } catch (error) { + return JSON.parse(jsonrepair(dataBootstrapString)); + } } From fe0ff836909fa16a6a7d948150412c7afbfd49c0 Mon Sep 17 00:00:00 2001 From: hainenber Date: Thu, 27 Feb 2025 08:28:46 +0700 Subject: [PATCH 5/6] feat(frontend/utils): wrap json-repaired bootstrap data in try-catch and return default if errored Signed-off-by: hainenber --- superset-frontend/src/utils/getBootstrapData.ts | 9 ++++++++- 1 file changed, 8 insertions(+), 1 deletion(-) diff --git a/superset-frontend/src/utils/getBootstrapData.ts b/superset-frontend/src/utils/getBootstrapData.ts index d34fd51c8a26..65e25e82b29c 100644 --- a/superset-frontend/src/utils/getBootstrapData.ts +++ b/superset-frontend/src/utils/getBootstrapData.ts @@ -20,6 +20,7 @@ import { jsonrepair } from 'jsonrepair'; import { BootstrapData } from 'src/types/bootstrapTypes'; import { DEFAULT_BOOTSTRAP_DATA } from 'src/constants'; +import { logging } from '@superset-ui/core'; export default function getBootstrapData(): BootstrapData { const appContainer = document.getElementById('app'); @@ -30,6 +31,12 @@ export default function getBootstrapData(): BootstrapData { try { return JSON.parse(dataBootstrapString); } catch (error) { - return JSON.parse(jsonrepair(dataBootstrapString)); + try { + return JSON.parse(jsonrepair(dataBootstrapString)); + } catch (error) { + logging.error('Malformed JSON in bootstrap data', error); + logging.info('Using default bootstrap data'); + return DEFAULT_BOOTSTRAP_DATA; + } } } From e19fa11088bec61265c6b9103e08b00141010b61 Mon Sep 17 00:00:00 2001 From: hainenber Date: Thu, 27 Feb 2025 08:30:06 +0700 Subject: [PATCH 6/6] chore(frontend/utils): reduce duplicate logging Signed-off-by: hainenber --- superset-frontend/src/utils/getBootstrapData.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/superset-frontend/src/utils/getBootstrapData.ts b/superset-frontend/src/utils/getBootstrapData.ts index 65e25e82b29c..8163c10153ed 100644 --- a/superset-frontend/src/utils/getBootstrapData.ts +++ b/superset-frontend/src/utils/getBootstrapData.ts @@ -34,8 +34,10 @@ export default function getBootstrapData(): BootstrapData { try { return JSON.parse(jsonrepair(dataBootstrapString)); } catch (error) { - logging.error('Malformed JSON in bootstrap data', error); - logging.info('Using default bootstrap data'); + logging.error( + 'Malformed JSON in bootstrap data. Using default data instead', + error, + ); return DEFAULT_BOOTSTRAP_DATA; } }