Uploaded image for project: 'Moodle'
  1. Moodle
  2. MDL-59312

Error object is discarded when handling failed AJAX requests (random "undefined" popup continues)

XMLWordPrintable

    • MOODLE_32_STABLE, MOODLE_33_STABLE
    • MOODLE_32_STABLE, MOODLE_33_STABLE
    • MDL-59312-master
    • Hide
      1. Make the following change to lib/ajax/service.php 

        diff --git a/lib/ajax/service.php b/lib/ajax/service.php
        		 
        index 3298c0e7a7..0f3a8e5aad 100644
        		 
        --- a/lib/ajax/service.php
        		 
        +++ b/lib/ajax/service.php
        		 
        @@ -64,4 +64,5 @@ foreach ($requests as $request) {
        		 
             }
        		 
         }
        		 
         
        		 
        +echo ".";
        		 
         echo json_encode($responses);

      2. Go to any page that makes an ajax request (all pages since they all check messaging and notifications)
      3. Wait for the request to fail
      4. Make sure you see a pop up with details about the error
      5. Remove that line you added
      6. Go to some AJAX-y part of moodle (e.g. Site administration > Plugins > Activity modules > External tool > Manage tools)
      7. Cause an error (e.g. Add a tool with the url http://lti.tools/test/tp.php and click the return "with an error" link)
      8. Make sure the error displays as a in-page notification
      9. Add an exception to a web service e.g. fetch_notifications() 

        diff --git a/lib/amd/src/notification.js b/lib/amd/src/notification.js
        		 
        index 601e50e..34d6bf9 100644
        		 
        --- a/lib/amd/src/notification.js
        		 
        +++ b/lib/amd/src/notification.js
        		 
        @@ -39,7 +39,7 @@ function(Y, $, log) {
        		 
                 fieldName: 'user-notifications',
        		 
         
                 fetchNotifications: function() {
        		 
        -            require(['core/ajax'], function(ajax) {
        		 
        +            require(['core/ajax', 'core/notification'], function(ajax, notif) {
        		 
                         var promises = ajax.call([{
        		 
                             methodname: 'core_fetch_notifications',
        		 
                             args: {
        		 
        @@ -48,8 +48,7 @@ function(Y, $, log) {
        		 
                         }]);
        		 
         
                         promises[0]
        		 
        -                    .done(notificationModule.addNotifications)
        		 
        -                    ;
        		 
        +                    .done(notificationModule.addNotifications).fail(notif.exception);
        		 
                     });
        		 
                 },
        		 
         
        diff --git a/lib/external/externallib.php b/lib/external/externallib.php
        		 
        index e33a709..f902cb8 100644
        		 
        --- a/lib/external/externallib.php
        		 
        +++ b/lib/external/externallib.php
        		 
        @@ -545,7 +545,7 @@ class core_external extends external_api {
        		 
         
                 $context = \context::instance_by_id($contextid);
        		 
                 self::validate_context($context);
        		 
        -
        		 
        +        throw new moodle_exception('adsf');
        		 
                 return \core\notification::fetch_as_array($PAGE->get_renderer('core'));
        		 
             }
        		 
         }

      10. Refresh the dashboard
      11. Make sure you get an error pop up modal with adsf
      12. Remove that code
      13. Add throw new moodle_exception('abc'); to lib/ajax/service.php
      14. Refresh the dashboard
      15. Ensure you get a abc error pop up (there may be a lot, this is expected - a lot of requests just failed)
      16. Remove that code
      17. Log in as a user
      18. Add sleep(10) to lib/ajax/service.php right before the echo json_encode($responses); (to simulate a lot of work to do)
      19. Click rapidly through a few different pages, before the messaging and notification icons/panels get enough time to load
      20. Make sure you don't get an error pop up modal. On chrome/firefox check that you see the error in the console. Make sure the error here makes sense No need to do this on multiple browsers
      21. Go to the dashboard
      22. Before it loads, click to go to site home (or anywhere else)
      23. Make sure no error shows
      24. After half a second, but still before the page loads, press stop on the browser (or press the hotkey... usually escape)
      25. Make sure no error pop up modal shows
      26. Remove the sleep(10) from lib/ajax/service.php
      Show
      Make the following change to lib/ajax/service.php diff --git a/lib/ajax/service.php b/lib/ajax/service.php index 3298c0e7a7..0f3a8e5aad 100644 --- a/lib/ajax/service.php +++ b/lib/ajax/service.php @@ -64,4 +64,5 @@ foreach ($requests as $request) { } }   +echo "."; echo json_encode($responses); Go to any page that makes an ajax request (all pages since they all check messaging and notifications) Wait for the request to fail Make sure you see a pop up with details about the error Remove that line you added Go to some AJAX-y part of moodle (e.g. Site administration > Plugins > Activity modules > External tool > Manage tools) Cause an error (e.g. Add a tool with the url http://lti.tools/test/tp.php and click the return "with an error" link) Make sure the error displays as a in-page notification Add an exception to a web service e.g. fetch_notifications() diff --git a/lib/amd/src/notification.js b/lib/amd/src/notification.js index 601e50e..34d6bf9 100644 --- a/lib/amd/src/notification.js +++ b/lib/amd/src/notification.js @@ -39,7 +39,7 @@ function(Y, $, log) { fieldName: 'user-notifications', fetchNotifications: function() { - require(['core/ajax'], function(ajax) { + require(['core/ajax', 'core/notification'], function(ajax, notif) { var promises = ajax.call([{ methodname: 'core_fetch_notifications', args: { @@ -48,8 +48,7 @@ function(Y, $, log) { }]); promises[0] - .done(notificationModule.addNotifications) - ; + .done(notificationModule.addNotifications).fail(notif.exception); }); }, diff --git a/lib/external/externallib.php b/lib/external/externallib.php index e33a709..f902cb8 100644 --- a/lib/external/externallib.php +++ b/lib/external/externallib.php @@ -545,7 +545,7 @@ class core_external extends external_api { $context = \context::instance_by_id($contextid); self::validate_context($context); - + throw new moodle_exception('adsf'); return \core\notification::fetch_as_array($PAGE->get_renderer('core')); } } Refresh the dashboard Make sure you get an error pop up modal with adsf Remove that code Add throw new moodle_exception('abc'); to lib/ajax/service.php Refresh the dashboard Ensure you get a abc error pop up (there may be a lot, this is expected - a lot of requests just failed) Remove that code Log in as a user Add sleep(10) to lib/ajax/service.php right before the echo json_encode($responses); (to simulate a lot of work to do) Click rapidly through a few different pages, before the messaging and notification icons/panels get enough time to load Make sure you don't get an error pop up modal. On chrome/firefox check that you see the error in the console. Make sure the error here makes sense No need to do this on multiple browsers Go to the dashboard Before it loads, click to go to site home (or anywhere else) Make sure no error shows After half a second, but still before the page loads, press stop on the browser (or press the hotkey... usually escape) Make sure no error pop up modal shows Remove the sleep(10) from lib/ajax/service.php

      We aren't passing the error object through so we can display more information about AJAX requests.

      Example:

      • An ill formatted response
      • A cancelled request (from page unload)

      In these occasions an Error is thrown by the JS (or native) code.

      The requestFail code in lib/amd/src/ajax.js is passed the Error, along with the name of the error that was thrown (e.g. parseerror).
      Unfortunately the requestFail code does not use the Error object, and instead passes just the error string into the promise rejection.

      In most cases a promise failure for our AJAX uses are passed straight to notification.exception.
      However, notification.exception expects an Error object rather than a string, and it passes the value, after minor modifications, to moodle-core-notification-exception.

      The YUI module (moodle-core-notification-exception) also expects an object, and it sets the content of the dialogue using that object, for example:

      '<h1 id="moodle-dialogue-' + config.COUNT + '-header-text">' + Y.Escape.html(config.name) + '</h1>',

      Since config should be an Error, but is actually a string, config.name returns as undefined - hence the confusing error message.

      In order to solve this we need to:

      1. take the additional argument into requestFail and pass it as an argument in the rejection; and
      2. take the additional argument into notification.exception and replace the first argument with it if the first arg is a string.

      If neither of the args are Error objects, then we should convert the string into an object with a value of name as a final fallback.

        1. image-2017-06-28-11-50-12-890.png
          image-2017-06-28-11-50-12-890.png
          2 kB

            dobedobedoh Andrew Lyons
            johno John Okely
            John Okely John Okely
            Jake Dallimore Jake Dallimore
            Jun Pataleta Jun Pataleta
            Votes:
            1 Vote for this issue
            Watchers:
            3 Start watching this issue

              Created:
              Updated:
              Resolved:

                Error rendering 'clockify-timesheets-time-tracking-reports:timer-sidebar'. Please contact your Jira administrators.