Improve error reporting in Ajax installer/upgrader
Affects | Status | Importance | Assigned to | Milestone | |
---|---|---|---|---|---|
Mahara |
Fix Released
|
High
|
Nigel Cunningham | ||
1.10 |
Won't Fix
|
Undecided
|
Unassigned | ||
15.04 |
Fix Released
|
High
|
Nigel Cunningham |
Bug Description
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.
Changed in mahara: | |
assignee: | nobody → Nigel Cunningham (nigelc-g) |
Changed in mahara: | |
status: | Confirmed → In Progress |
Changed in mahara: | |
status: | Fix Committed → Fix Released |
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