diff --git a/superset-frontend/packages/superset-ui-core/src/connection/callApi/callApi.ts b/superset-frontend/packages/superset-ui-core/src/connection/callApi/callApi.ts index 7c3fe21fdb8..7a19e94c032 100644 --- a/superset-frontend/packages/superset-ui-core/src/connection/callApi/callApi.ts +++ b/superset-frontend/packages/superset-ui-core/src/connection/callApi/callApi.ts @@ -146,10 +146,23 @@ export default async function callApi({ Object.keys(payload).forEach(key => { const value = (payload as JsonObject)[key] as JsonValue; if (typeof value !== 'undefined') { - formData.append( - key, - stringify ? JSON.stringify(value) : String(value), - ); + let valueString; + try { + // We have seen instances where casting to String() throws error + // This check allows all valid attributes to be appended to the formData + // while logging error to console for any attribute that fails the cast to String + valueString = stringify ? JSON.stringify(value) : String(value); + } catch (e) { + // eslint-disable-next-line no-console + console.error( + `Unable to convert attribute '${key}' to a String(). '${key}' was not added to the formData in request.body for call to ${url}`, + value, + e, + ); + } + if (valueString !== undefined) { + formData.append(key, valueString); + } } }); request.body = formData; diff --git a/superset-frontend/packages/superset-ui-core/test/connection/callApi/callApi.test.ts b/superset-frontend/packages/superset-ui-core/test/connection/callApi/callApi.test.ts index 81467ce2393..81c8e2d1505 100644 --- a/superset-frontend/packages/superset-ui-core/test/connection/callApi/callApi.test.ts +++ b/superset-frontend/packages/superset-ui-core/test/connection/callApi/callApi.test.ts @@ -23,6 +23,12 @@ import callApi from '../../../src/connection/callApi/callApi'; import { LOGIN_GLOB } from '../fixtures/constants'; +// missing the toString function causing method to error out when casting to String +class BadObject {} +const corruptObject = new BadObject(); +/* @ts-expect-error */ +BadObject.prototype.toString = undefined; + describe('callApi()', () => { beforeAll(() => { fetchMock.get(LOGIN_GLOB, { result: '1234' }); @@ -178,6 +184,44 @@ describe('callApi()', () => { expect(jsonRequestBody[key]).toEqual(value); }); }); + + it('removes corrupt value when building formData with stringify = false', async () => { + /* + There has been a case when 'stringify' is false an object value on one of the + attributes was missing a toString function making the cast to String() fail + and causing entire method call to fail. The new logic skips corrupt values that fail cast to String() + and allows all valid attributes to be added as key / value pairs to the formData + instance. This test case replicates a corrupt object missing the .toString method + representing a real bug report. + */ + const postPayload = { + string: 'value', + number: 1237, + array: [1, 2, 3], + object: { a: 'a', 1: 1 }, + null: null, + emptyString: '', + // corruptObject has no toString method and will fail cast to String() + corrupt: [corruptObject], + }; + jest.spyOn(console, 'error').mockImplementation(); + + await callApi({ + url: mockPostUrl, + method: 'POST', + postPayload, + stringify: false, + }); + + const calls = fetchMock.calls(mockPostUrl); + expect(calls).toHaveLength(1); + const unstringified = (calls[0][1] as RequestInit).body as FormData; + const hasCorruptKey = unstringified.has('corrupt'); + expect(hasCorruptKey).toBeFalsy(); + // When a corrupt attribute is encountred, a console.error call is made with info about the corrupt attribute + // eslint-disable-next-line no-console + expect(console.error).toHaveBeenCalledTimes(1); + }); }); describe('PUT requests', () => { diff --git a/superset-frontend/src/SqlLab/actions/sqlLab.js b/superset-frontend/src/SqlLab/actions/sqlLab.js index 1435e0d796e..6391ce82c98 100644 --- a/superset-frontend/src/SqlLab/actions/sqlLab.js +++ b/superset-frontend/src/SqlLab/actions/sqlLab.js @@ -908,9 +908,12 @@ export function updateSavedQuery(query) { dispatch(addSuccessToast(t('Your query was updated'))); dispatch(queryEditorSetTitle(query, query.name)); }) - .catch(() => - dispatch(addDangerToast(t('Your query could not be updated'))), - ) + .catch(e => { + const message = t('Your query could not be updated'); + // eslint-disable-next-line no-console + console.error(message, e); + dispatch(addDangerToast(message)); + }) .then(() => dispatch(updateQueryEditor(query))); }