Improve error reporting in Ajax installer/upgrader

[Aaron Wells]

We use an Ajax-based page to install & upgrade Mahara. For each upgrade task it:

- fires off a separate Ajax request to run the task
- shows a "progress" spinner while it waits for a response
- and then once it gets a response it updates the page and fires off the next task

The problem is that this system doesn't handle errors well. If the Ajax script errors out, it continues showing the "progress" spinner forever. Consequently we get a LOT of error reports where people complain that "installation takes forever", when in fact it errored out after 30 seconds or whatever and it just never told them.

We need to update this process so that it actually lets you know when it failed, and why.

The simplest solution, suggested by Robert, is that we put a timeout on waiting for the Ajax response. Mahara sets max_execution_time itself, so we could:

1. Make the ajax time out after max_execution_time
2. Show an error icon
3. Fire off a second ajax response to a script that gets any error messages from the session and displays those on the page.

That should be pretty robust against any type of error.

[Nigel Cunningham (nigelc-g)]

I've spent some time looking into this this afternoon. It seems to me that some of the issue is already resolved:

If I set up my IDE to catch the invocation of an installation in admin/upgrade.json.php, line 42 ($fun()) and just sit on the breakpoint until the request times out, the spinner gets changed to "Failed to upgrade".

The problem I saw was where the failure is not an failure of the ajax call itself (like the above), but a successful completion of the request which returns an error. The simplest way to reproduce this was to add

throw new Exception("Failure!");

prior to the $fun() invocation mentioned above. The ajax call thus completes, but neither the success or the failure handler passed to sendjsonrequest in line 145 of admin/upgrade.php get invoked. This is because sendjsonrequest uses its own success and
error handlers, and there is a path (which we're hitting) wherein neither of our handlers will be invoked by sendjsonrequest's handlers:

If an ajax request succeeds, but the data in the result indicates an error (data.error = true), the code in lines 233-236 will be invoked, calling globalErrorHandler. In this case, errtype has not been set, so the successcallback function will rightly not be called, but neither will the errorcallback.

My solution is to simply call errorcallback() after globalErrorHandler, as well as after setting errortype = 'error' in the block above.

I'm not submitting it as a patch yet, though, because I'm aware that sendjsonrequest is called from many other places, and I'd like to have a discussion with others about it first. The code seems messy to me, and I'm not sure I fully comprehend yet the implications of this change as well as others might (particularly since you're all way more familiar with the code!).

Thanks for any feedback!

Nigel

[Mahara Bot (dev-mahara)]

Patch for "master" branch: https://reviews.mahara.org/3510

[Mahara Bot (dev-mahara)]

Reviewed: https://reviews.mahara.org/3510
Committed: http://gitorious.org/mahara/mahara/commit/2e873d2a5251b905ddd2543ba53f40d19749b28c
Submitter: Aaron Wells (<email address hidden>)
Branch: master

commit 2e873d2a5251b905ddd2543ba53f40d19749b28c
Author: Nigel Cunningham <email address hidden>
Date: Mon Jul 14 16:04:00 2014 +1000

(Bug 1318432) Ensure errorhandler called

In sendjsonrequest, if the request completes but returns an error,
we should invoke the caller's error handling function.

Change-Id: I52ef8ae900796c5db4532fa11c2a152d8a75b838
Signed-off-by: Nigel Cunningham <email address hidden>

[Mahara Bot (dev-mahara)]

Patch for "master" branch: https://reviews.mahara.org/4375

[Aaron Wells (u-aaronw)]

I've merged Nigel's patch, but it doesn't really resolve the problem I'm describing, which I guess is what happens when the script errors out without returning properly formed AJAX.

So, I've pushed another patch to fix that problem. The core of it was that we use the MochiKit JSON parsing method, which does no JSON validation beforehand at all. It just does an eval() of whatever it gets sent.

I've swapped it out for the JQuery JSON parser, which is smart enough to throw an exception when it receives malformed JSON. Then we can catch that exception and call the error callback method properly.

I think it'll be good to get this in for 15.04, because it'll help with debugging installation issues.

https://reviews.mahara.org/4375

[Mahara Bot (dev-mahara)]

Reviewed: https://reviews.mahara.org/4375
Committed: http://gitorious.org/mahara/mahara/commit/01467168a15d501f9767b9de66853b02e50db32d
Submitter: Robert Lyon (<email address hidden>)
Branch: master

commit 01467168a15d501f9767b9de66853b02e50db32d
Author: Aaron Wells <email address hidden>
Date: Tue Mar 10 18:00:14 2015 +1300

Make the JSON request handler better able to handle malformed responses

Bug 1318432

Change-Id: I161df25067515a4c85b7058df7b2e7dbebe32a33