fix: [sc-54864] Adds safety check to provide near term fix to save query (#21034)

* [sc-54864] Adds safety check to provide near term fix for Queries not saving

There is an error when casting the columns array to String() for saving queries where the objects in the array are missing the toString method.   This is a near term rapid patch to fix workflow in production which will have a follow up to identify root cause.

* fix typo

Co-authored-by: Elizabeth Thompson <eschutho@gmail.com>

* Adjusted chekc to be explicit for undefined instead of truthy

This fixes issue caught by unit test where the if statement was using a truthy check where it should be explicitly checking for value of undefined

* Adds new unit test to get 100% coverage for callApi

Co-authored-by: Elizabeth Thompson <eschutho@gmail.com>

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;

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', () => {

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)));
 }