[Phpmyadmin-devel] AJAX error reporting system (catching errors)

[Mohamed Ashraf]

On Sat, Jun 22, 2013 at 8:26 PM, Rouslan Placella <rouslan at placella.com> wrote:
> On 06/22/2013 05:53 PM, Mohamed Ashraf wrote:
>> On Sat, Jun 22, 2013 at 5:30 PM, Rouslan Placella <rouslan at placella.com> wrote:
>>> On 06/22/2013 11:10 AM, Mohamed Ashraf wrote:
>>>> On Sat, Jun 22, 2013 at 10:27 AM, Rouslan Placella <rouslan at placella.com> wrote:
>>>>> On 06/22/2013 01:21 AM, Mohamed Ashraf wrote:
>>>>>> On Saturday, June 22, 2013 at 12:49 AM, Rouslan Placella wrote:
>>>>>>> On 06/16/2013 12:59 PM, Mohamed Ashraf wrote:
>>>>>>>> On Wed, Jun 12, 2013 at 9:58 AM, Rouslan Placella
>>>>>>>> <rouslan at placella.com> wrote:
>>>>>>>>
>>>>>>>>> Some of my thoughts are:
>>>>>>>>>
>>>>>>>>> * Try/catch has a performance penalty
>>>>>>>> it is not too big as can be seen here
>>>>>>>> http://jsperf.com/try-catch-performance-overhead
>>>>>>>
>>>>>>> Really? On some browsers it's about 20 times slower on the very
>>>>>>> benchmark that you are linking to!
>>>>>> Really!! That is strange I tested it multiple times and everytime it
>>>>>> showed they are all equal in time. Which test case was 20 times slower?
>>>>>
>>>>> Opera 11 would be one of them. IE 10 is also quite bad, but by a lower
>>>>> magnitude...
>>>>
>>>> the reduction in performance is usually in wrapping the inside of a
>>>> function with try and catch. I was thinking of doing the other way and
>>>> wrapping the functions from outside like this
>>>>
>>>> var temp  = PMA_foo
>>>> PMA_foo = function() {
>>>> try{
>>>> temp.apply(window, arguments);
>>>> } catch(e) {
>>>> my_method(e);
>>>> }
>>>> }
>>>>
>>>> I am thinking of doing this to all global functions starting with PMA_
>>>> and according to the benchmark this should have an equivalent
>>>> performance to the no try catch  performance. also if we don't do
>>>> anything of the sort then we would never get a stacktrace.
>>>>
>>>> the javascript in phpmyadmin is just for aesthetics and caching but it
>>>> is not really very processor intensive anyway it is mostly about doing
>>>> things to the page and sending requests to the server and most of the
>>>> heavy lifting is done by jquery itself which I shall not change. The
>>>> benchmark is to show what happens in the extreme case which may be
>>>> problematic in node.js where the js code does heavy lifting and
>>>> performance hits are problematic. however for the simple javascript we
>>>> have, the negligible loss in performance cannot be perceived by the
>>>> end users. but the stack traces we get could significantly better
>>>> their experience in the long run.
>>>>
>>>> I will be using an automated function to add the try and catch at
>>>> runtime so that there would be minimum modification to the codebase
>>>> and it can be easily removed if required.
>>>>
>>>>>
>>>>>>>>> * We need to be able to easily turn off the error reporting feature
>>>>>>>> You can do it simply by setting a config option I have already set it
>>>>>>>> up in the Header.class.php
>>>>>>>>> * Some/most function names in the stack trace are going to be useless as
>>>>>>>>> the production js code is minified.
>>>>>>>>> * The line numbers in the file will not be of much help either, I guess,
>>>>>>>>> unless we change the way that the files are minified (like forcing line
>>>>>>>>> breaks every few hundred characters).
>>>>>>>> since uglified js provides little to no information about the location
>>>>>>>> of the error. what should we do. I checked up on source maps however
>>>>>>>> the browser support is not enough. so what do you think should happen.
>>>>>>>> I think the only way to go is to add try and catch to all the
>>>>>>>> different functions if you need to find out where the error has
>>>>>>>> occured. however adding try and catch statements to the entire
>>>>>>>> codebase will decrease code readability considerably.
>>>
>>> Thank you for the explanation, it makes perfect sense. By the way, in
>>> the above code snippet, to retain the correct context, it should be:
>>>
>>> temp.apply(this, arguments);
>>
>> Ah yes you are correct. I was thinking about how will I get the
>> context and just assumed window. but you are correct "this" will refer
>> to the correct context. there is a problem with this approach since
>> these functions are only a small subset of the global functions used
>> by phpmyadmin.
>>
>>>
>>> Also, out of curiosity, would it be possible to use such a solution for
>>> the event handlers that we register with AJAX.registerOnload?
>>> What about jquery event handlers (e.g: $('#foo').click(somefunction) ) and jquery
>>> ajax callbacks?
>>
>> sadly no since they are anonymous functions mostly and I do not have
>> access to them unless I manually insert a try and catch in the code
>> itself
>
> Well, you can hook into AJAX.registerOnload, using your code snippet
> above. There you can reference the callback as "func". I just tested
> this and it works.
>
>> also keep in mind this is just to get the most out of the error and as
>> much as we do will still have exceptions that are thrown in parts of
>> code that are not surrounded by try and catch statements and that is
>> ok since there is a second line of defense with the "window.onerror".
>> I am just trying to catch most errors without changing alot of the
>> codebase and anything not caught we will still get an error report but
>> just without a stacktrace. Also you should keep in mind that a
>> stacktrace is not available in some browsers so even try and catch may
>> not do any better than window.error
>
> I have a few thoughts on the matter.
>
> First of all, it may be useful to do a bit of feature detection. You
> could deliberately throw an error and catch if, then check if the stack
> trace is available inside it. Then, based on the result of this test you
> could decide to wrap the code in a try/catch using your code snippet, if
> the stack trace will be available, and if the stack trace will not be
> available there would be no point in trying to catch the error that way
> and you could leave the functions unwrapped to improve performance.
>
yes I will install many different browsers and try to test it on a
browser without a stacktrace

> Another issue that I found is that, in my tests, the stack traces appear
> to be very cluttered as the JS files are concatenated by the server.

can you give me an example of what you mean.

> Not sure what we could do about this (keep track of number of lines offset
> for each source file? something else?)...
>
yes this is a problem which I have thought about but have not yet
figured out the solution. At first I just thought the developers can
figure it out since they get the url and the phpmyadmin version so
they can simulate the situation and see where a specific line is,
however it turns out its not so easy at all. then I thought my
webserver can do it for them and extract that part of the code and
show it with the report but then I will have to interface with git
from the web server and get the actual commit from whatever info that
I have which would not have been easy. The thing that comes to mind
now but I am still looking for a better solution is putting line
markers in the generated js file. for example between each file I can
output a js comment showing the current line number allowing the
developer to narrow down his search but this is far from optimal and
would appreciate any ideas

> Bye,
> Rouslan
>
>
> ------------------------------------------------------------------------------
> This SF.net email is sponsored by Windows:
>
> Build for Windows Store.
>
> http://p.sf.net/sfu/windows-dev2dev
> _______________________________________________
> Phpmyadmin-devel mailing list
> Phpmyadmin-devel at lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/phpmyadmin-devel
>