fix: handle clone errors gracefully (#28451)
## Description This PR introduces custom transmission (clone) error handlers. #### PR fixes following issue(s) Fixes #28449 #### Media > A video or a GIF is preferred. when using Loom, don’t embed because it looks like it’s a GIF. instead, just link to the video > > #### Type of change > Please delete options that are not relevant. - Bug fix (non-breaking change which fixes an issue) - New feature (non-breaking change which adds functionality) - Breaking change (fix or feature that would cause existing functionality to not work as expected) - Chore (housekeeping or task changes that don't impact user perception) - This change requires a documentation update > > > ## Testing > #### How Has This Been Tested? > Please describe the tests that you ran to verify your changes. Also list any relevant details for your test configuration. > Delete anything that is not relevant - [ ] Manual - [ ] JUnit - [ ] Jest - [ ] Cypress > > #### Test Plan > Add Testsmith test cases links that relate to this PR > > #### Issues raised during DP testing > Link issues raised during DP testing for better visiblity and tracking (copy link from comments dropped on this PR) > > > ## Checklist: #### Dev activity - [ ] My code follows the style guidelines of this project - [ ] I have performed a self-review of my own code - [ ] I have commented my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [ ] My changes generate no new warnings - [ ] I have added tests that prove my fix is effective or that my feature works - [ ] New and existing unit tests pass locally with my changes - [ ] PR is being merged under a feature flag #### QA activity: - [ ] [Speedbreak features](https://github.com/appsmithorg/TestSmith/wiki/Guidelines-for-test-plans#speedbreakers-) have been covered - [ ] Test plan covers all impacted features and [areas of interest](https://github.com/appsmithorg/TestSmith/wiki/Guidelines-for-test-plans#areas-of-interest-) - [ ] Test plan has been peer reviewed by project stakeholders and other QA members - [ ] Manually tested functionality on DP - [ ] We had an implementation alignment call with stakeholders post QA Round 2 - [ ] Cypress test cases have been added and approved by SDET/manual QA - [ ] Added `Test Plan Approved` label after Cypress tests were reviewed - [ ] Added `Test Plan Approved` label after JUnit tests were reviewed
This commit is contained in:
parent
f497a4d7f8
commit
215adcf3a3
|
|
@ -141,7 +141,7 @@ export function getDependenciesFromInverseDependencies(
|
||||||
deps: DependencyMap,
|
deps: DependencyMap,
|
||||||
entityName: string | null,
|
entityName: string | null,
|
||||||
) {
|
) {
|
||||||
if (!entityName) return null;
|
if (!entityName || !deps) return null;
|
||||||
|
|
||||||
const directDependencies = new Set<string>();
|
const directDependencies = new Set<string>();
|
||||||
const inverseDependencies = new Set<string>();
|
const inverseDependencies = new Set<string>();
|
||||||
|
|
|
||||||
|
|
@ -251,7 +251,7 @@ function* handleEachUpdateJSCollection(update: JSUpdate) {
|
||||||
export function* makeUpdateJSCollection(
|
export function* makeUpdateJSCollection(
|
||||||
action: ReduxAction<Record<string, JSUpdate>>,
|
action: ReduxAction<Record<string, JSUpdate>>,
|
||||||
) {
|
) {
|
||||||
const jsUpdates: Record<string, JSUpdate> = action.payload;
|
const jsUpdates: Record<string, JSUpdate> = action.payload || {};
|
||||||
|
|
||||||
yield all(
|
yield all(
|
||||||
Object.keys(jsUpdates).map((key) =>
|
Object.keys(jsUpdates).map((key) =>
|
||||||
|
|
|
||||||
|
|
@ -296,7 +296,7 @@ export const TypeErrorModifier: Modifier = (
|
||||||
errorMessage.message = `${
|
errorMessage.message = `${
|
||||||
possibleCausesArr.length === 1
|
possibleCausesArr.length === 1
|
||||||
? `${possibleCausesArr[0]} is undefined`
|
? `${possibleCausesArr[0]} is undefined`
|
||||||
: `${Array.from(possibleCauses).join(", ")} `
|
: `${Array.from(possibleCauses).join(", ")} could be undefined`
|
||||||
} . Please fix ${source || "the binding"}.`;
|
} . Please fix ${source || "the binding"}.`;
|
||||||
|
|
||||||
return {
|
return {
|
||||||
|
|
|
||||||
|
|
@ -1,7 +1,11 @@
|
||||||
// Workers do not have access to log.error
|
// Workers do not have access to log.error
|
||||||
/* eslint-disable no-console */
|
/* eslint-disable no-console */
|
||||||
import type { EvalWorkerASyncRequest, EvalWorkerSyncRequest } from "./types";
|
import type { EvalWorkerASyncRequest, EvalWorkerSyncRequest } from "./types";
|
||||||
import { syncHandlerMap, asyncHandlerMap } from "./handlers";
|
import {
|
||||||
|
syncHandlerMap,
|
||||||
|
asyncHandlerMap,
|
||||||
|
transmissionErrorHandlerMap,
|
||||||
|
} from "./handlers";
|
||||||
import type { TMessage } from "utils/MessageUtil";
|
import type { TMessage } from "utils/MessageUtil";
|
||||||
import { MessageType } from "utils/MessageUtil";
|
import { MessageType } from "utils/MessageUtil";
|
||||||
import { WorkerMessenger } from "./fns/utils/Messenger";
|
import { WorkerMessenger } from "./fns/utils/Messenger";
|
||||||
|
|
@ -20,8 +24,14 @@ function syncRequestMessageListener(
|
||||||
if (typeof messageHandler !== "function") return;
|
if (typeof messageHandler !== "function") return;
|
||||||
const responseData = messageHandler(body);
|
const responseData = messageHandler(body);
|
||||||
if (!responseData) return;
|
if (!responseData) return;
|
||||||
|
const transmissionErrorHandler = transmissionErrorHandlerMap[method];
|
||||||
const endTime = performance.now();
|
const endTime = performance.now();
|
||||||
WorkerMessenger.respond(messageId, responseData, endTime - startTime);
|
WorkerMessenger.respond(
|
||||||
|
messageId,
|
||||||
|
responseData,
|
||||||
|
endTime - startTime,
|
||||||
|
transmissionErrorHandler,
|
||||||
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
async function asyncRequestMessageListener(
|
async function asyncRequestMessageListener(
|
||||||
|
|
@ -38,7 +48,13 @@ async function asyncRequestMessageListener(
|
||||||
const data = await messageHandler(body);
|
const data = await messageHandler(body);
|
||||||
if (!data) return;
|
if (!data) return;
|
||||||
const end = performance.now();
|
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);
|
self.addEventListener("message", syncRequestMessageListener);
|
||||||
|
|
|
||||||
|
|
@ -26,6 +26,42 @@ async function responseHandler(requestId: string): Promise<TPromiseResponse> {
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
|
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 {
|
export class WorkerMessenger {
|
||||||
static async request(payload: any) {
|
static async request(payload: any) {
|
||||||
const messageId = uniqueId(`request-${payload.method}-`);
|
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 {
|
try {
|
||||||
sendMessage.call(self, {
|
sendMessage.call(self, {
|
||||||
messageId,
|
messageId,
|
||||||
|
|
@ -71,28 +112,12 @@ export class WorkerMessenger {
|
||||||
body: { data, timeTaken },
|
body: { data, timeTaken },
|
||||||
});
|
});
|
||||||
} catch (e) {
|
} catch (e) {
|
||||||
// TODO: Remove hardcoded error handling.
|
const errorHandler = onErrorHandler || defaultErrorHandler;
|
||||||
console.error(e);
|
try {
|
||||||
sendMessage.call(self, {
|
errorHandler(messageId, timeTaken, data, e);
|
||||||
messageId,
|
} catch {
|
||||||
messageType: MessageType.RESPONSE,
|
defaultErrorHandler(messageId, timeTaken, data, e);
|
||||||
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),
|
|
||||||
},
|
|
||||||
],
|
|
||||||
},
|
|
||||||
},
|
|
||||||
});
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -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)),
|
||||||
|
]);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
@ -24,6 +24,8 @@ import { getJSVariableCreatedEvents } from "../JSObject/JSVariableEvents";
|
||||||
import { errorModifier } from "../errorModifier";
|
import { errorModifier } from "../errorModifier";
|
||||||
import { generateOptimisedUpdatesAndSetPrevState } from "../helpers";
|
import { generateOptimisedUpdatesAndSetPrevState } from "../helpers";
|
||||||
import DataStore from "../dataStore";
|
import DataStore from "../dataStore";
|
||||||
|
import type { TransmissionErrorHandler } from "../fns/utils/Messenger";
|
||||||
|
import { MessageType, sendMessage } from "utils/MessageUtil";
|
||||||
|
|
||||||
export let replayMap: Record<string, ReplayEntity<any>> | undefined;
|
export let replayMap: Record<string, ReplayEntity<any>> | undefined;
|
||||||
export let dataTreeEvaluator: DataTreeEvaluator | undefined;
|
export let dataTreeEvaluator: DataTreeEvaluator | undefined;
|
||||||
|
|
@ -223,6 +225,19 @@ export default function (request: EvalWorkerSyncRequest) {
|
||||||
return evalTreeResponse;
|
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() {
|
export function clearCache() {
|
||||||
dataTreeEvaluator = undefined;
|
dataTreeEvaluator = undefined;
|
||||||
clearAllIntervals();
|
clearAllIntervals();
|
||||||
|
|
|
||||||
|
|
@ -7,7 +7,10 @@ import { EVAL_WORKER_ACTIONS } from "@appsmith/workers/Evaluation/evalWorkerActi
|
||||||
import type { EvalWorkerSyncRequest, EvalWorkerASyncRequest } from "../types";
|
import type { EvalWorkerSyncRequest, EvalWorkerASyncRequest } from "../types";
|
||||||
import evalActionBindings from "./evalActionBindings";
|
import evalActionBindings from "./evalActionBindings";
|
||||||
import evalExpression from "./evalExpression";
|
import evalExpression from "./evalExpression";
|
||||||
import evalTree, { clearCache } from "./evalTree";
|
import evalTree, {
|
||||||
|
clearCache,
|
||||||
|
evalTreeTransmissionErrorHandler,
|
||||||
|
} from "./evalTree";
|
||||||
import evalTrigger from "./evalTrigger";
|
import evalTrigger from "./evalTrigger";
|
||||||
import initFormEval from "./initFormEval";
|
import initFormEval from "./initFormEval";
|
||||||
import { installLibrary, loadLibraries, uninstallLibrary } from "./jsLibrary";
|
import { installLibrary, loadLibraries, uninstallLibrary } from "./jsLibrary";
|
||||||
|
|
@ -17,6 +20,7 @@ import setupEvaluationEnvironment, {
|
||||||
} from "./setupEvalEnv";
|
} from "./setupEvalEnv";
|
||||||
import validateProperty from "./validateProperty";
|
import validateProperty from "./validateProperty";
|
||||||
import updateActionData from "./updateActionData";
|
import updateActionData from "./updateActionData";
|
||||||
|
import type { TransmissionErrorHandler } from "../fns/utils/Messenger";
|
||||||
|
|
||||||
const syncHandlerMap: Record<
|
const syncHandlerMap: Record<
|
||||||
EVAL_WORKER_SYNC_ACTION,
|
EVAL_WORKER_SYNC_ACTION,
|
||||||
|
|
@ -47,4 +51,13 @@ const asyncHandlerMap: Record<
|
||||||
[EVAL_WORKER_ACTIONS.INSTALL_LIBRARY]: installLibrary,
|
[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 };
|
||||||
|
|
|
||||||
Loading…
Reference in New Issue
Block a user