Modified ErrorHandler to call notify onerror event so uncaught except…

…ions are displayed, revised exception handling to only catch expected errors, and fixed test randomly failing because of onerror notify.
mdenet · Mar 28, 2024 · 2d10c79 · 2d10c79
1 parent ed278ac
commit 2d10c79
Show file tree

Hide file tree

Showing 6 changed files with 41 additions and 17 deletions.
diff --git a/platform/src/ActivityManager.js b/platform/src/ActivityManager.js
@@ -132,7 +132,7 @@ class ActivityManager {
         try {
             let file = this.fileHandler.fetchFile( this.activitiesUrl , urlParamPrivateRepo() );
             fileContent = file.content;
-        } catch (e) {
+        } catch (err) {
             errors.push( new EducationPlatformError(`The activity configuration file was not accessible at: ${this.activitiesUrl}. 
                                                     Check the activity file is available at the given url and you have the correct access rights.`) );
         }

diff --git a/platform/src/EducationPlatformApp.js b/platform/src/EducationPlatformApp.js
@@ -526,7 +526,11 @@ class EducationPlatformApp {
             } 
         }).catch( (err) => {
             this.errorHandler.notify("There was an error translating action function parameter types.", err);
-        } );
+
+            if (!(err instanceof EducationPlatformError)) {
+                throw err;
+            }
+        });
 
     }
 
@@ -705,8 +709,13 @@ class EducationPlatformApp {
         Promise.all(fileStorePromises).then( () => {
             this.successNotification("The activity panel contents have been saved.");
 
-        }).catch(() => {
-            this.errorHandler.notify("An error occurred while trying to save the panel contents.");
+        }).catch( (err) => {
+            this.errorHandler.notify("An error occurred while trying to save the panel contents.", err);
+
+            if (!(err instanceof EducationPlatformError)) {
+                throw err;
+            }
+            this.errorHandler.notify();
         });
     }
 

diff --git a/platform/src/ErrorHandler.js b/platform/src/ErrorHandler.js
@@ -11,15 +11,18 @@ class ErrorHandler {
 
         this.displayError = notifier;
 
-        window.onerror = () => {
+        window.onerror = (event, source, lineno, colno, err) => {
+
+            this.notify("An unexpected error has occurred", err);
+
             // TODO log unhandled exceptions/errors to remote server
         };
     }
 
     /**
      * Displays the given error 
-     * @param message - The message to display
-     * @param {EducationPlatformError} error - The error to display
+     * @param message - The message to display if provided
+     * @param {EducationPlatformError} error - The error to display if provided
      */
     notify(message, error){
         let displayMessage = "";
@@ -32,11 +35,16 @@ class ErrorHandler {
             displayMessage += "<br><br>"
         }
 
-        if (error instanceof EducationPlatformError){
-            displayMessage += `<i>${error.message}</i>`;
-        } else {
-            // Other errors mark as unknown
-            displayMessage += `<i>Unknown - ${error.message}</i>`;
+        if (error) {
+            if (error instanceof EducationPlatformError){
+                displayMessage += `<i>${error.message}</i>`;
+            } else if (error instanceof Error){ 
+                // Other errors mark as unknown
+                displayMessage += `<i>${error.constructor.name} - ${error.message}</i>`;
+            } else {
+                // Anything else mark as unknown
+                displayMessage += `<i>value - ${String(error)}</i>`;
+            }
         }
 
         this.displayError(displayMessage);

diff --git a/platform/src/ToolsManager.js b/platform/src/ToolsManager.js
@@ -59,11 +59,13 @@ class ToolManager {
 
             try{
                 xhr.send();
-            } catch (e) {
-                if (e instanceof DOMException){
+            } catch (err) {
+                if (err instanceof DOMException){
                     errors.push( new EducationPlatformError(`A tool configuration file was not accessible at: ${toolUrl.url}. 
                                                             Check the tool's url given in the activity file is correct and the tool 
                                                             service is still available.`) );
+                } else {
+                    throw err;
                 }
             }
 

diff --git a/platform/src/Utility.js b/platform/src/Utility.js
@@ -200,9 +200,11 @@ export function parseConfigFile(contents, extension="yml"){
                 console.log("Cannot parse unsupported configuration file type '" + extension + "'.");
                 configObject = null;
         }
-    } catch(e){
-        if (e instanceof YAMLParseError || e instanceof SyntaxError){
-            configObject = e;
+    } catch(err){
+        if (err instanceof YAMLParseError || err instanceof SyntaxError){
+            configObject = err;
+        } else {
+            throw err;
         }
     }
 

diff --git a/platform/test/spec/testEducationPlatformAppSpec.js b/platform/test/spec/testEducationPlatformAppSpec.js
@@ -140,6 +140,9 @@ describe("EducationPlatformApp", () => {
                 reject(new TypeError("test type error"));
             })
 
+            window.onerror = () => {}; /* Disable notifications for unknown errors so that the uncaught exceptions 
+                                          that this test triggers do not cause random failures */
+
             // Call the target object
             platform.handleResponseActionFunction({}, invokeReturnedPromiseError);