fix: show js function execution errors in debugger (#14555)

* Show js function execution error logs

* remove unused function

* improve check for async functions

* clear errors for deleted jsActions

* fix typescript error

* modify js function execution error logging

* test that execution parse errors are logged in the debugger

* Add test to show that js execution errors are logged in the debugger

* re-order js execution tests

* Add type to jsObj variable

* update cypress tests

* update cypress test
This commit is contained in:
Favour Ohanekwu 2022-06-30 08:21:20 +01:00 committed by GitHub
parent 703b0efda6
commit 41789c71bc
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
11 changed files with 164 additions and 36 deletions

View File

@ -10,7 +10,8 @@ const jsEditor = ObjectsRegistry.JSEditor,
let onPageLoadAndConfirmExecuteFunctionsLength: number, let onPageLoadAndConfirmExecuteFunctionsLength: number,
getJSObject: any, getJSObject: any,
functionsLength: number, jsObj: any; functionsLength: number,
jsObj: string;
describe("JS Function Execution", function() { describe("JS Function Execution", function() {
interface IFunctionSettingData { interface IFunctionSettingData {
@ -50,6 +51,22 @@ describe("JS Function Execution", function() {
ee.DragDropWidgetNVerify("tablewidget", 300, 300); ee.DragDropWidgetNVerify("tablewidget", 300, 300);
ee.NavigateToSwitcher("explorer"); ee.NavigateToSwitcher("explorer");
}); });
function assertAsyncFunctionsOrder(data: IFunctionSettingData[]) {
// sorts functions alphabetically
const sortFunctions = (data: IFunctionSettingData[]) =>
data.sort((a, b) => a.name.localeCompare(b.name));
cy.get(jsEditor._asyncJSFunctionSettings).then(function($lis) {
const asyncFunctionLength = $lis.length;
// Assert number of async functions
expect(asyncFunctionLength).to.equal(functionsLength);
Object.values(sortFunctions(data)).forEach((functionSetting, idx) => {
// Assert alphabetical order
expect($lis.eq(idx)).to.have.id(
jsEditor._getJSFunctionSettingsId(functionSetting.name),
);
});
});
}
it("1. Allows execution of js function when lint warnings(not errors) are present in code", function() { it("1. Allows execution of js function when lint warnings(not errors) are present in code", function() {
jsEditor.CreateJSObject( jsEditor.CreateJSObject(
@ -170,8 +187,75 @@ describe("JS Function Execution", function() {
assertInvalidJSObjectStart(jsObjectStartingWithANewLine, jsObjectStartLine); assertInvalidJSObjectStart(jsObjectStartingWithANewLine, jsObjectStartLine);
assertInvalidJSObjectStart(jsObjectStartingWithASpace, jsObjectStartLine); assertInvalidJSObjectStart(jsObjectStartingWithASpace, jsObjectStartLine);
}); });
it("5. Verify that js function execution errors are logged in debugger and removed when function is deleted", () => {
const JS_OBJECT_WITH_PARSE_ERROR = `export default {
myVar1: [],
myVar2: {},
myFun1: () => {
//write code here
return Table1.unknown.name
}
}`;
it("5. Supports the use of large JSON data (doesn't crash)", () => { const JS_OBJECT_WITHOUT_PARSE_ERROR = `export default {
myVar1: [],
myVar2: {},
myFun1: () => {
//write code here
return Table1.unknown
}
}`;
const JS_OBJECT_WITH_DELETED_FUNCTION = `export default {
myVar1: [],
myVar2: {}
}`;
// Create js object
jsEditor.CreateJSObject(JS_OBJECT_WITH_PARSE_ERROR, {
paste: true,
completeReplace: true,
toRun: true,
shouldCreateNewJSObj: true,
});
// Assert that there is a function execution parse error
jsEditor.AssertParseError(true, true);
// click the debug icon
agHelper.GetNClick(jsEditor._debugCTA);
// Assert that errors tab is not empty
cy.contains("No signs of trouble here!").should("not.exist");
// Assert presence of typeError
cy.contains(
"TypeError: Cannot read properties of undefined (reading 'name')",
).should("exist");
// Fix parse error and assert that debugger error is removed
jsEditor.EditJSObj(JS_OBJECT_WITHOUT_PARSE_ERROR);
agHelper.GetNClick(jsEditor._runButton);
jsEditor.AssertParseError(false, true);
agHelper.GetNClick(locator._errorTab);
cy.contains(
"TypeError: Cannot read properties of undefined (reading 'name')",
).should("not.exist");
// Switch back to response tab
agHelper.GetNClick(locator._responseTab);
// Re-introduce parse errors
jsEditor.EditJSObj(JS_OBJECT_WITH_PARSE_ERROR);
agHelper.GetNClick(jsEditor._runButton);
// Assert that there is a function execution parse error
jsEditor.AssertParseError(true, true);
// Delete function
jsEditor.EditJSObj(JS_OBJECT_WITH_DELETED_FUNCTION);
// Assert that parse error is removed from debugger when function is deleted
agHelper.GetNClick(locator._errorTab);
cy.contains(
"TypeError: Cannot read properties of undefined (reading 'name')",
).should("not.exist");
});
it("6. Supports the use of large JSON data (doesn't crash)", () => {
const jsObjectWithLargeJSONData = `export default{ const jsObjectWithLargeJSONData = `export default{
largeData: ${JSON.stringify(largeJSONData)}, largeData: ${JSON.stringify(largeJSONData)},
myfun1: ()=> this.largeData myfun1: ()=> this.largeData
@ -211,7 +295,7 @@ describe("JS Function Execution", function() {
}); });
}); });
it("6. Doesn't cause cyclic dependency when function name is edited", () => { it("7. Doesn't cause cyclic dependency when function name is edited", () => {
const syncJSCode = `export default { const syncJSCode = `export default {
myFun1 :()=>{ myFun1 :()=>{
return "yes" return "yes"
@ -275,8 +359,7 @@ describe("JS Function Execution", function() {
jsEditor.EditJSObj(asyncJSCodeWithRenamedFunction2); jsEditor.EditJSObj(asyncJSCodeWithRenamedFunction2);
agHelper.AssertElementAbsence(locator._toastMsg); agHelper.AssertElementAbsence(locator._toastMsg);
}); });
it("8. Maintains order of async functions in settings tab alphabetically at all times", function() {
it("7. Maintains order of async functions in settings tab alphabetically at all times", function() {
functionsLength = FUNCTIONS_SETTINGS_DEFAULT_DATA.length; functionsLength = FUNCTIONS_SETTINGS_DEFAULT_DATA.length;
// Number of functions set to run on page load and should also confirm before execute // Number of functions set to run on page load and should also confirm before execute
onPageLoadAndConfirmExecuteFunctionsLength = FUNCTIONS_SETTINGS_DEFAULT_DATA.filter( onPageLoadAndConfirmExecuteFunctionsLength = FUNCTIONS_SETTINGS_DEFAULT_DATA.filter(
@ -341,7 +424,7 @@ describe("JS Function Execution", function() {
assertAsyncFunctionsOrder(FUNCTIONS_SETTINGS_DEFAULT_DATA); assertAsyncFunctionsOrder(FUNCTIONS_SETTINGS_DEFAULT_DATA);
}); });
it("8. Verify Asyn methods alphabetical order after clone page and after rename", () => { it("9. Verify Async methods have alphabetical order after cloning page and renaming it", () => {
const FUNCTIONS_SETTINGS_RENAMED_DATA: IFunctionSettingData[] = [ const FUNCTIONS_SETTINGS_RENAMED_DATA: IFunctionSettingData[] = [
{ {
name: "newGetId", name: "newGetId",
@ -379,7 +462,7 @@ describe("JS Function Execution", function() {
agHelper.Sleep(); agHelper.Sleep();
} }
ee.SelectEntityByName(jsObj as string, "QUERIES/JS"); ee.SelectEntityByName(jsObj, "QUERIES/JS");
agHelper.GetNClick(jsEditor._settingsTab); agHelper.GetNClick(jsEditor._settingsTab);
assertAsyncFunctionsOrder(FUNCTIONS_SETTINGS_DEFAULT_DATA); assertAsyncFunctionsOrder(FUNCTIONS_SETTINGS_DEFAULT_DATA);
@ -391,21 +474,4 @@ describe("JS Function Execution", function() {
agHelper.GetNClick(jsEditor._settingsTab); agHelper.GetNClick(jsEditor._settingsTab);
assertAsyncFunctionsOrder(FUNCTIONS_SETTINGS_RENAMED_DATA); assertAsyncFunctionsOrder(FUNCTIONS_SETTINGS_RENAMED_DATA);
}); });
function assertAsyncFunctionsOrder(data: IFunctionSettingData[]) {
// sorts functions alphabetically
const sortFunctions = (data: IFunctionSettingData[]) =>
data.sort((a, b) => a.name.localeCompare(b.name));
cy.get(jsEditor._asyncJSFunctionSettings).then(function($lis) {
const asyncFunctionLength = $lis.length;
// Assert number of async functions
expect(asyncFunctionLength).to.equal(functionsLength);
Object.values(sortFunctions(data)).forEach((functionSetting, idx) => {
// Assert alphabetical order
expect($lis.eq(idx)).to.have.id(
jsEditor._getJSFunctionSettingsId(functionSetting.name),
);
});
});
}
}); });

View File

@ -37,6 +37,7 @@ export class CommonLocators {
_uploadBtn = "button.uppy-StatusBar-actionBtn--upload" _uploadBtn = "button.uppy-StatusBar-actionBtn--upload"
_debuggerIcon = ".t--debugger svg" _debuggerIcon = ".t--debugger svg"
_errorTab = "[data-cy=t--tab-ERROR]" _errorTab = "[data-cy=t--tab-ERROR]"
_responseTab = "[data-cy=t--tab-response]"
_debugErrorMsg = ".t--debugger-message" _debugErrorMsg = ".t--debugger-message"
_debuggerLabel = "span.debugger-label" _debuggerLabel = "span.debugger-label"
_modal = ".t--modal-widget" _modal = ".t--modal-widget"

View File

@ -81,6 +81,7 @@ export class JSEditor {
_getJSFunctionSettingsId = (JSFunctionName: string) => _getJSFunctionSettingsId = (JSFunctionName: string) =>
`${JSFunctionName}-settings`; `${JSFunctionName}-settings`;
_asyncJSFunctionSettings = `.t--async-js-function-settings`; _asyncJSFunctionSettings = `.t--async-js-function-settings`;
_debugCTA = `button.js-editor-debug-cta`;
//#endregion //#endregion
//#region constants //#region constants

View File

@ -199,7 +199,6 @@ function JSResponseView(props: Props) {
}); });
dispatch(setCurrentTab(DEBUGGER_TAB_KEYS.ERROR_TAB)); dispatch(setCurrentTab(DEBUGGER_TAB_KEYS.ERROR_TAB));
}, []); }, []);
useEffect(() => { useEffect(() => {
setResponseStatus( setResponseStatus(
getJSResponseViewState( getJSResponseViewState(
@ -213,7 +212,7 @@ function JSResponseView(props: Props) {
}, [responses, isExecuting, currentFunction, isSaving, isDirty]); }, [responses, isExecuting, currentFunction, isSaving, isDirty]);
const tabs = [ const tabs = [
{ {
key: "body", key: "response",
title: "Response", title: "Response",
panelComponent: ( panelComponent: (
<> <>
@ -229,7 +228,10 @@ function JSResponseView(props: Props) {
fill fill
label={ label={
<FailedMessage> <FailedMessage>
<DebugButton onClick={onDebugClick} /> <DebugButton
className="js-editor-debug-cta"
onClick={onDebugClick}
/>
</FailedMessage> </FailedMessage>
} }
text={ text={

View File

@ -11,6 +11,7 @@ export enum ENTITY_TYPE {
export enum PLATFORM_ERROR { export enum PLATFORM_ERROR {
PLUGIN_EXECUTION = "PLUGIN_EXECUTION", PLUGIN_EXECUTION = "PLUGIN_EXECUTION",
JS_FUNCTION_EXECUTION = "JS_FUNCTION_EXECUTION",
} }
export type ErrorType = PropertyEvaluationErrorType | PLATFORM_ERROR; export type ErrorType = PropertyEvaluationErrorType | PLATFORM_ERROR;

View File

@ -314,6 +314,7 @@ function* logDebuggerErrorAnalyticsSaga(
getJSCollection, getJSCollection,
payload.entityId, payload.entityId,
); );
if (!action) return;
const plugin: Plugin = yield select(getPlugin, action.pluginId); const plugin: Plugin = yield select(getPlugin, action.pluginId);
const pluginName = plugin?.name?.replace(/ /g, ""); const pluginName = plugin?.name?.replace(/ /g, "");

View File

@ -47,6 +47,7 @@ import {
} from "actions/evaluationActions"; } from "actions/evaluationActions";
import { import {
evalErrorHandler, evalErrorHandler,
handleJSFunctionExecutionErrorLog,
logSuccessfulBindings, logSuccessfulBindings,
postEvalActionDispatcher, postEvalActionDispatcher,
updateTernDefinitions, updateTernDefinitions,
@ -350,7 +351,11 @@ export function* clearEvalCache() {
return true; return true;
} }
export function* executeFunction(collectionName: string, action: JSAction) { export function* executeFunction(
collectionName: string,
action: JSAction,
collectionId: string,
) {
const functionCall = `${collectionName}.${action.name}()`; const functionCall = `${collectionName}.${action.name}()`;
const { isAsync } = action.actionConfiguration; const { isAsync } = action.actionConfiguration;
let response: { let response: {
@ -381,7 +386,13 @@ export function* executeFunction(collectionName: string, action: JSAction) {
const { errors, result } = response; const { errors, result } = response;
const isDirty = !!errors.length; const isDirty = !!errors.length;
yield call(evalErrorHandler, errors); yield call(
handleJSFunctionExecutionErrorLog,
collectionId,
collectionName,
action,
errors,
);
return { result, isDirty }; return { result, isDirty };
} }

View File

@ -270,6 +270,10 @@ function* updateJSCollection(data: {
jsCollection, jsCollection,
createMessage(JS_FUNCTION_DELETE_SUCCESS), createMessage(JS_FUNCTION_DELETE_SUCCESS),
); );
// delete all execution error logs for deletedActions if present
deletedActions.forEach((action) =>
AppsmithConsole.deleteError(`${jsCollection.id}-${action.id}`),
);
} }
yield put( yield put(
@ -353,6 +357,7 @@ export function* handleExecuteJSFunctionSaga(data: {
executeFunction, executeFunction,
collectionName, collectionName,
action, action,
collectionId,
); );
yield put({ yield put({
type: ReduxActionTypes.EXECUTE_JS_FUNCTION_SUCCESS, type: ReduxActionTypes.EXECUTE_JS_FUNCTION_SUCCESS,

View File

@ -1,4 +1,9 @@
import { ENTITY_TYPE, Log, Severity } from "entities/AppsmithConsole"; import {
ENTITY_TYPE,
Log,
PLATFORM_ERROR,
Severity,
} from "entities/AppsmithConsole";
import { DataTree } from "entities/DataTree/dataTreeFactory"; import { DataTree } from "entities/DataTree/dataTreeFactory";
import { import {
DataTreeDiff, DataTreeDiff,
@ -31,6 +36,7 @@ import {
ERROR_EVAL_ERROR_GENERIC, ERROR_EVAL_ERROR_GENERIC,
JS_OBJECT_BODY_INVALID, JS_OBJECT_BODY_INVALID,
VALUE_IS_INVALID, VALUE_IS_INVALID,
JS_EXECUTION_FAILURE,
} from "@appsmith/constants/messages"; } from "@appsmith/constants/messages";
import log from "loglevel"; import log from "loglevel";
import { AppState } from "reducers"; import { AppState } from "reducers";
@ -40,6 +46,7 @@ import { dataTreeTypeDefCreator } from "utils/autocomplete/dataTreeTypeDefCreato
import TernServer from "utils/autocomplete/TernServer"; import TernServer from "utils/autocomplete/TernServer";
import { selectFeatureFlags } from "selectors/usersSelectors"; import { selectFeatureFlags } from "selectors/usersSelectors";
import FeatureFlags from "entities/FeatureFlags"; import FeatureFlags from "entities/FeatureFlags";
import { JSAction } from "entities/JSCollection";
const getDebuggerErrors = (state: AppState) => state.ui.debugger.errors; const getDebuggerErrors = (state: AppState) => state.ui.debugger.errors;
/** /**
@ -392,3 +399,31 @@ export function* updateTernDefinitions(
log.debug("Tern definitions updated took ", (end - start).toFixed(2)); log.debug("Tern definitions updated took ", (end - start).toFixed(2));
} }
} }
export function* handleJSFunctionExecutionErrorLog(
collectionId: string,
collectionName: string,
action: JSAction,
errors: any[],
) {
errors.length
? AppsmithConsole.addError({
id: `${collectionId}-${action.id}`,
logType: LOG_TYPE.EVAL_ERROR,
text: `${createMessage(JS_EXECUTION_FAILURE)}: ${collectionName}.${
action.name
}`,
messages: errors.map((error) => ({
message: error.errorMessage || error.message,
type: PLATFORM_ERROR.JS_FUNCTION_EXECUTION,
subType: error.errorType,
})),
source: {
id: action.id,
name: `${collectionName}.${action.name}`,
type: ENTITY_TYPE.JSACTION,
propertyPath: `${collectionName}.${action.name}`,
},
})
: AppsmithConsole.deleteError(`${collectionId}-${action.id}`);
}

View File

@ -404,12 +404,17 @@ export function isFunctionAsync(
}); });
try { try {
if (typeof userFunction === "function") { if (typeof userFunction === "function") {
const returnValue = userFunction(); if (userFunction.constructor.name === "AsyncFunction") {
if (!!returnValue && returnValue instanceof Promise) { // functions declared with an async keyword
self.IS_ASYNC = true;
}
if (self.TRIGGER_COLLECTOR.length) {
self.IS_ASYNC = true; self.IS_ASYNC = true;
} else {
const returnValue = userFunction();
if (!!returnValue && returnValue instanceof Promise) {
self.IS_ASYNC = true;
}
if (self.TRIGGER_COLLECTOR.length) {
self.IS_ASYNC = true;
}
} }
} }
} catch (e) { } catch (e) {

View File

@ -67,7 +67,7 @@ function messageEventListener(
errors: [ errors: [
{ {
type: EvalErrorTypes.CLONE_ERROR, type: EvalErrorTypes.CLONE_ERROR,
message: e, message: (e as Error)?.message,
context: JSON.stringify(rest), context: JSON.stringify(rest),
}, },
], ],