[GitHub] [cordova-js] mathiasscheffe commented on a change in pull request #222: Stable error handling for loaded Cordova plugins

[GitBox <gi...@apache.org>]

mathiasscheffe commented on a change in pull request #222: Stable error handling for loaded Cordova plugins
URL: https://github.com/apache/cordova-js/pull/222#discussion_r406134203
 
 

 ##########
 File path: src/common/pluginloader.js
 ##########
 @@ -79,19 +79,63 @@ function onScriptLoadingComplete (moduleList, finishPluginLoading) {
 function handlePluginsObject (path, moduleList, finishPluginLoading) {
     // Now inject the scripts.
     var scriptCounter = moduleList.length;
+    var modulesWithProblems = [];
 
     if (!scriptCounter) {
         finishPluginLoading();
         return;
     }
+
+    function callOnScriptLoadingComplete () {
+        modulesWithProblems.forEach(function (elem) {
+            var foundIndex = -1;
+            var modulesLength = moduleList.length;
+            var i = 0;
+            // find module with load problems in module list. Use while loop to exit iteration early.
+            while (i < modulesLength && foundIndex === -1) {
+                if (moduleList[i].id === elem) {
+                    foundIndex = i;
+                }
+                i++;
+            }
+            if (foundIndex !== -1) {
+                // delete module with load problem from module list
+                moduleList.splice(foundIndex, 1);
+            }
+        });
+        onScriptLoadingComplete(moduleList, finishPluginLoading);
+    }
+
     function scriptLoadedCallback () {
         if (!--scriptCounter) {
-            onScriptLoadingComplete(moduleList, finishPluginLoading);
+            callOnScriptLoadingComplete();
+        }
+    }
+
+    function scriptLoadedErrorCallback (id, message, source, lineno, colno, error) {
+        modulesWithProblems.push(id);
+        if (typeof message !== 'undefined') {
+            var messageString = message;
+            if (typeof message !== 'string') {
+                messageString = JSON.stringify(message);
+            }
+            messageString = 'Could not load all functions. Please confirm or restart your application. \n \n' +
+            'Details: Error while loading module: "' + id + '". Module will be skipped. ' + messageString;
+            console.error(messageString);
+            alert(messageString);
+        }
+        if (!--scriptCounter) {
+            callOnScriptLoadingComplete();
         }
     }
 
     for (var i = 0; i < moduleList.length; i++) {
-        injectIfNecessary(moduleList[i].id, path + moduleList[i].file, scriptLoadedCallback);
+        var moduleId = moduleList[i].id;
+        var me = this;
+        // bound function to have the module id when the error occurs.
+        var boundErrorCallback = scriptLoadedErrorCallback.bind(me, moduleId);
 
 Review comment:
   This looks fine for me as well.
   My intention was to create as little surprises as possible for future developers. Therefore I wanted to keep the this pointer intact. But it is not needed now and will also work in the way you suggested.

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
users@infra.apache.org


With regards,
Apache Git Services

---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cordova.apache.org
For additional commands, e-mail: commits-help@cordova.apache.org