From 12ff4675cfec530c068eddc9e4b3f05aa1693c6d Mon Sep 17 00:00:00 2001 From: arunvjn <32433245+arunvjn@users.noreply.github.com> Date: Tue, 4 Jul 2023 10:01:55 +0530 Subject: [PATCH] fix: Log uncaught errors to debugger (#25030) ## Description All errors from synchronous user code are handled by evaluation engine. But if a user action schedules a timer and the callback code contains an error, this error is never caught causing sentry to log it. This PR adds a global "error" event listener on the worker that handles all such uncaught errors, by logging it to console and in-app debugger. #### PR fixes following issue(s) Fixes #24039 #### Type of change - Bug fix (non-breaking change which fixes an issue) > > ## Testing > #### How Has This Been Tested? - [x] Manual - [x] Cypress - [ ] Jest > > #### 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 - [x] My code follows the style guidelines of this project - [x] I have performed a self-review of my own code - [x] I have commented my code, particularly in hard-to-understand areas - [ ] I have made corresponding changes to the documentation - [x] My changes generate no new warnings - [x] I have added tests that prove my fix is effective or that my feature works - [x] 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 (cherry picked from commit 02c9e25f6e9d1d7ad054bf825095d403ed65f0d8) --- .../ClientSide/OtherUIFeatures/Logs2_spec.js | 27 +++++++++++++++++++ .../workers/Evaluation/evaluation.worker.ts | 8 ++++++ 2 files changed, 35 insertions(+) diff --git a/app/client/cypress/e2e/Regression/ClientSide/OtherUIFeatures/Logs2_spec.js b/app/client/cypress/e2e/Regression/ClientSide/OtherUIFeatures/Logs2_spec.js index e4f9b5369c..479f76bee3 100644 --- a/app/client/cypress/e2e/Regression/ClientSide/OtherUIFeatures/Logs2_spec.js +++ b/app/client/cypress/e2e/Regression/ClientSide/OtherUIFeatures/Logs2_spec.js @@ -291,4 +291,31 @@ describe("Debugger logs", function () { cy.get(".t--js-action-name-edit-field").should("exist"); }); + + it("10. Bug #24039 - Logs errors from setInterval callback into debugger", () => { + _.entityExplorer.NavigateToSwitcher("Widgets"); + _.entityExplorer.DragDropWidgetNVerify("buttonwidget", 400, 600); + _.entityExplorer.SelectEntityByName("Button1", "Widgets"); + _.propPane.SelectPlatformFunction("onClick", "Set interval"); + _.agHelper.EnterActionValue( + "Callback function", + `{{() => { + try { + Test.run(); + } catch (e) { + clearInterval('myInterval'); + throw e; + } + } + }}`, + ); + _.agHelper.EnterActionValue("Id", "myInterval"); + _.agHelper.Sleep(); + _.agHelper.GetNClick(_.jsEditor._logsTab); + _.debuggerHelper.ClearLogs(); + _.agHelper.ClickButton("Submit"); + _.debuggerHelper.DoesConsoleLogExist( + "Uncaught ReferenceError: Test is not defined", + ); + }); }); diff --git a/app/client/src/workers/Evaluation/evaluation.worker.ts b/app/client/src/workers/Evaluation/evaluation.worker.ts index 53c3795d40..dc157e392a 100644 --- a/app/client/src/workers/Evaluation/evaluation.worker.ts +++ b/app/client/src/workers/Evaluation/evaluation.worker.ts @@ -44,6 +44,14 @@ async function asyncRequestMessageListener( self.addEventListener("message", syncRequestMessageListener); self.addEventListener("message", asyncRequestMessageListener); +self.addEventListener("error", (e) => { + if (e instanceof ErrorEvent) { + console.error(e.message); + } else { + console.error(e); + } +}); + self.addEventListener("unhandledrejection", (e) => { // We might want to send this error to the main thread in the future. // console error will log the error to the logs tab against trigger field.