diff --git a/app/client/src/components/editorComponents/Debugger/helpers.tsx b/app/client/src/components/editorComponents/Debugger/helpers.tsx index 1ac95392f8..b085d68222 100644 --- a/app/client/src/components/editorComponents/Debugger/helpers.tsx +++ b/app/client/src/components/editorComponents/Debugger/helpers.tsx @@ -141,7 +141,7 @@ export function getDependenciesFromInverseDependencies( deps: DependencyMap, entityName: string | null, ) { - if (!entityName) return null; + if (!entityName || !deps) return null; const directDependencies = new Set(); const inverseDependencies = new Set(); diff --git a/app/client/src/sagas/JSPaneSagas.ts b/app/client/src/sagas/JSPaneSagas.ts index 1d96884888..69dc8aa822 100644 --- a/app/client/src/sagas/JSPaneSagas.ts +++ b/app/client/src/sagas/JSPaneSagas.ts @@ -251,7 +251,7 @@ function* handleEachUpdateJSCollection(update: JSUpdate) { export function* makeUpdateJSCollection( action: ReduxAction>, ) { - const jsUpdates: Record = action.payload; + const jsUpdates: Record = action.payload || {}; yield all( Object.keys(jsUpdates).map((key) => diff --git a/app/client/src/workers/Evaluation/errorModifier.ts b/app/client/src/workers/Evaluation/errorModifier.ts index 118be74049..a74ec29827 100644 --- a/app/client/src/workers/Evaluation/errorModifier.ts +++ b/app/client/src/workers/Evaluation/errorModifier.ts @@ -296,7 +296,7 @@ export const TypeErrorModifier: Modifier = ( errorMessage.message = `${ possibleCausesArr.length === 1 ? `${possibleCausesArr[0]} is undefined` - : `${Array.from(possibleCauses).join(", ")} ` + : `${Array.from(possibleCauses).join(", ")} could be undefined` } . Please fix ${source || "the binding"}.`; return { diff --git a/app/client/src/workers/Evaluation/evaluation.worker.ts b/app/client/src/workers/Evaluation/evaluation.worker.ts index 3847a99cae..587f8a1527 100644 --- a/app/client/src/workers/Evaluation/evaluation.worker.ts +++ b/app/client/src/workers/Evaluation/evaluation.worker.ts @@ -1,7 +1,11 @@ // Workers do not have access to log.error /* eslint-disable no-console */ import type { EvalWorkerASyncRequest, EvalWorkerSyncRequest } from "./types"; -import { syncHandlerMap, asyncHandlerMap } from "./handlers"; +import { + syncHandlerMap, + asyncHandlerMap, + transmissionErrorHandlerMap, +} from "./handlers"; import type { TMessage } from "utils/MessageUtil"; import { MessageType } from "utils/MessageUtil"; import { WorkerMessenger } from "./fns/utils/Messenger"; @@ -20,8 +24,14 @@ function syncRequestMessageListener( if (typeof messageHandler !== "function") return; const responseData = messageHandler(body); if (!responseData) return; + const transmissionErrorHandler = transmissionErrorHandlerMap[method]; const endTime = performance.now(); - WorkerMessenger.respond(messageId, responseData, endTime - startTime); + WorkerMessenger.respond( + messageId, + responseData, + endTime - startTime, + transmissionErrorHandler, + ); } async function asyncRequestMessageListener( @@ -38,7 +48,13 @@ async function asyncRequestMessageListener( const data = await messageHandler(body); if (!data) return; const end = performance.now(); - WorkerMessenger.respond(messageId, data, end - start); + const transmissionErrorHandler = transmissionErrorHandlerMap[method]; + WorkerMessenger.respond( + messageId, + data, + end - start, + transmissionErrorHandler, + ); } self.addEventListener("message", syncRequestMessageListener); diff --git a/app/client/src/workers/Evaluation/fns/utils/Messenger.ts b/app/client/src/workers/Evaluation/fns/utils/Messenger.ts index 434764c53f..39a327475f 100644 --- a/app/client/src/workers/Evaluation/fns/utils/Messenger.ts +++ b/app/client/src/workers/Evaluation/fns/utils/Messenger.ts @@ -26,6 +26,42 @@ async function responseHandler(requestId: string): Promise { }); } +export type TransmissionErrorHandler = ( + messageId: string, + timeTaken: number, + responseData: unknown, + e: unknown, +) => void; + +const defaultErrorHandler: TransmissionErrorHandler = ( + messageId: string, + timeTaken: number, + responseData: unknown, + e: unknown, +) => { + console.error(e); + sendMessage.call(self, { + messageId, + messageType: MessageType.RESPONSE, + body: { + timeTaken: timeTaken.toFixed(2), + data: { + errors: [ + { + type: WorkerErrorTypes.CLONE_ERROR, + message: (e as Error)?.message, + errorMessage: getErrorMessage( + e as Error, + WorkerErrorTypes.CLONE_ERROR, + ), + context: JSON.stringify(responseData), + }, + ], + }, + }, + }); +}; + export class WorkerMessenger { static async request(payload: any) { const messageId = uniqueId(`request-${payload.method}-`); @@ -63,7 +99,12 @@ export class WorkerMessenger { } } - static respond(messageId: string, data: unknown, timeTaken: number) { + static respond( + messageId: string, + data: unknown, + timeTaken: number, + onErrorHandler?: TransmissionErrorHandler, + ) { try { sendMessage.call(self, { messageId, @@ -71,28 +112,12 @@ export class WorkerMessenger { body: { data, timeTaken }, }); } catch (e) { - // TODO: Remove hardcoded error handling. - console.error(e); - sendMessage.call(self, { - messageId, - messageType: MessageType.RESPONSE, - body: { - timeTaken: timeTaken.toFixed(2), - data: { - errors: [ - { - type: WorkerErrorTypes.CLONE_ERROR, - message: (e as Error)?.message, - errorMessage: getErrorMessage( - e as Error, - WorkerErrorTypes.CLONE_ERROR, - ), - context: JSON.stringify(data), - }, - ], - }, - }, - }); + const errorHandler = onErrorHandler || defaultErrorHandler; + try { + errorHandler(messageId, timeTaken, data, e); + } catch { + defaultErrorHandler(messageId, timeTaken, data, e); + } } } } diff --git a/app/client/src/workers/Evaluation/handlers/__tests__/evalTree.test.ts b/app/client/src/workers/Evaluation/handlers/__tests__/evalTree.test.ts new file mode 100644 index 0000000000..83dd9655fc --- /dev/null +++ b/app/client/src/workers/Evaluation/handlers/__tests__/evalTree.test.ts @@ -0,0 +1,63 @@ +import { WorkerMessenger } from "workers/Evaluation/fns/utils/Messenger"; +import { evalTreeTransmissionErrorHandler } from "../evalTree"; +import { isFunction } from "lodash"; + +const mockEvalErrorHandler = jest.fn(); +const mockSendMessage = jest.fn(); + +jest.mock("workers/Evaluation/handlers/evalTree", () => { + const actualExports = jest.requireActual( + "workers/Evaluation/handlers/evalTree", + ); + return { + __esModule: true, + ...actualExports, + evalTreeTransmissionErrorHandler: (...args: unknown[]) => { + mockEvalErrorHandler(); + actualExports.evalTreeTransmissionErrorHandler(...args); + }, + }; +}); +jest.mock("utils/MessageUtil", () => { + const actualExports = jest.requireActual("utils/MessageUtil"); + return { + __esModule: true, + ...actualExports, + sendMessage: (...args: any[]) => { + mockSendMessage(args[0].body.data); + const { + body: { data }, + } = args[0]; + if (isFunction(data.response)) { + throw new Error("unserializable data"); + } + }, + }; +}); + +describe("test", () => { + it("calls custom evalTree error handler", () => { + const UNSERIALIZABLE_DATA = { + response: () => {}, + logs: { + depedencies: { name: ["test", "you"] }, + }, + }; + WorkerMessenger.respond( + "TEST", + UNSERIALIZABLE_DATA, + 4, + evalTreeTransmissionErrorHandler, + ); + // Since response is unserializable, expect EvalErrorHandler to be called + expect(mockEvalErrorHandler).toBeCalledTimes(1); + // Error in the first attempt, then, a successfully second attempt + expect(mockSendMessage).toBeCalledTimes(2); + + expect(mockSendMessage.mock.calls[0]).toEqual([UNSERIALIZABLE_DATA]); + // The error handler should convert data to a serializable form + expect(mockSendMessage.mock.calls[1]).toEqual([ + JSON.parse(JSON.stringify(UNSERIALIZABLE_DATA)), + ]); + }); +}); diff --git a/app/client/src/workers/Evaluation/handlers/evalTree.ts b/app/client/src/workers/Evaluation/handlers/evalTree.ts index 58d1c4e629..64b278140a 100644 --- a/app/client/src/workers/Evaluation/handlers/evalTree.ts +++ b/app/client/src/workers/Evaluation/handlers/evalTree.ts @@ -24,6 +24,8 @@ import { getJSVariableCreatedEvents } from "../JSObject/JSVariableEvents"; import { errorModifier } from "../errorModifier"; import { generateOptimisedUpdatesAndSetPrevState } from "../helpers"; import DataStore from "../dataStore"; +import type { TransmissionErrorHandler } from "../fns/utils/Messenger"; +import { MessageType, sendMessage } from "utils/MessageUtil"; export let replayMap: Record> | undefined; export let dataTreeEvaluator: DataTreeEvaluator | undefined; @@ -223,6 +225,19 @@ export default function (request: EvalWorkerSyncRequest) { return evalTreeResponse; } +export const evalTreeTransmissionErrorHandler: TransmissionErrorHandler = ( + messageId: string, + timeTaken: number, + responseData: unknown, +) => { + const sanitizedData = JSON.parse(JSON.stringify(responseData)); + sendMessage.call(self, { + messageId, + messageType: MessageType.RESPONSE, + body: { data: sanitizedData, timeTaken }, + }); +}; + export function clearCache() { dataTreeEvaluator = undefined; clearAllIntervals(); diff --git a/app/client/src/workers/Evaluation/handlers/index.ts b/app/client/src/workers/Evaluation/handlers/index.ts index b02811d970..6f7bd15272 100644 --- a/app/client/src/workers/Evaluation/handlers/index.ts +++ b/app/client/src/workers/Evaluation/handlers/index.ts @@ -7,7 +7,10 @@ import { EVAL_WORKER_ACTIONS } from "@appsmith/workers/Evaluation/evalWorkerActi import type { EvalWorkerSyncRequest, EvalWorkerASyncRequest } from "../types"; import evalActionBindings from "./evalActionBindings"; import evalExpression from "./evalExpression"; -import evalTree, { clearCache } from "./evalTree"; +import evalTree, { + clearCache, + evalTreeTransmissionErrorHandler, +} from "./evalTree"; import evalTrigger from "./evalTrigger"; import initFormEval from "./initFormEval"; import { installLibrary, loadLibraries, uninstallLibrary } from "./jsLibrary"; @@ -17,6 +20,7 @@ import setupEvaluationEnvironment, { } from "./setupEvalEnv"; import validateProperty from "./validateProperty"; import updateActionData from "./updateActionData"; +import type { TransmissionErrorHandler } from "../fns/utils/Messenger"; const syncHandlerMap: Record< EVAL_WORKER_SYNC_ACTION, @@ -47,4 +51,13 @@ const asyncHandlerMap: Record< [EVAL_WORKER_ACTIONS.INSTALL_LIBRARY]: installLibrary, }; -export { syncHandlerMap, asyncHandlerMap }; +const transmissionErrorHandlerMap: Partial< + Record< + EVAL_WORKER_SYNC_ACTION | EVAL_WORKER_ASYNC_ACTION, + TransmissionErrorHandler + > +> = { + [EVAL_WORKER_ACTIONS.EVAL_TREE]: evalTreeTransmissionErrorHandler, +}; + +export { syncHandlerMap, asyncHandlerMap, transmissionErrorHandlerMap };