Cacti: Improved Javascript error message handling

Created on 12 Jan 2018  路  21Comments  路  Source: Cacti/cacti

When click on a tab, if there is an error 500 (for example) then the console side bar. For example, I was modifying the quicktree plugin, and missed a semi-colon (that age old developer issue) and then the tab was just collapsing and expanding the side bar. Only when I opened the tab in a new window did the error actually appear.

enhancement

Most helpful comment

I like this better.

callbackerrordisplay

All 21 comments

Well, I'm not sure we should do anything about that one. It involves changing our wrappers loadPageNoHeader() and loadPage() from using the $.get() shortcut to using the $.ajax() function call.

It can be done, but it's not the highest on my priority. You would have to search out all the $.get() and $.getJSON() and $.post() calls on all the pages and rework them. Then have a common function, likely in layout.js to handle presenting some error message like "Your code broke the web server" :)

Well, it's comments like this post:
https://forums.cacti.net/viewtopic.php?p=270671#p270671

That make me think we should. If we allow plugin's to directly add UI elements, we should at least make sure they respond correctly if we are using AJAX to switch to them. If it had been a direct HTTP request via the location bar, the error would be seen (for example, open it in a new tab). Without a change, it just looks weird, broken and confusing to a normal end user.

I'm just about to commit something that will trap a large number of these errors. You can use it as a template to get the other ones. I will add it as a feature, and we will keep this bug open. There are at least 37 more of these in the various base directory files, and likely some in lib.

This could be beautified by using jQueryUI dialog I guess. What do you think?

callbackerrordisplay

As a first pass it works, the beautifying can come later. Once we got the basic functionality in, it's just a simply mod to getFormatHTTPError() to make it look really pretty, maybe with a sad cacti?

I like this better.

callbackerrordisplay

Hello.
I like last version :)

lol

Slight typo in the commit message, but as the screenshot looks a lot better, I didn't notice it ;-)

I'm more looking at the wording now, thinking that you are saying it's a warning and an error... slight confusion there. Also, do we need the "Reason:" or "Error:" parts since they are self explainitory? Not sure if I would change it or not slightly.

Feel free to word smith it and do a pull. Remember the global_session.php change too. Rony won't release for a few hours.

image

Don't know why but I expected a green bar

Global session change? Have I forgotten something?

Take a look at global_session.php in the include directory, it includes translated strings into variables.

Relative to colors, we have a project to implement jQueryUI 1.12.1 where we have to rebuild all the themes jQueryUI css. That'll be a good project for someone. As Paul Gevers stated in the other thread, all the jQueryUI css is pretty much based off the smoothness theme. I just have been reluctant to go down that path. It got too creepy until I found that you can not just use the old css, you have to start over.

If we could get some help on that, we would have it placed into the feature 1.2 branch.

Oh yeah, the JavaScript variables are primed from global_session.

I may create a branch, pull in jQuery 1.12.1 and see what the big diff is ;-)

Cool. How bout the word smithing?

OK, Give me a minute.

OK, I have created a commit https://github.com/Cacti/cacti/commit/977559a61bc145dd6747115e7e197a04a8626598 which changes the layout slight to:

image

Still wondering if we should change the title to be a static title rather than the returning value so that we don't overrun the title. The styling is all in the JS at the moment, so you may want that to be in the css.

In the end, stuck with mostly what you had as once I started playing, it didn't look right.

Well, this looks much better. I'd say stick a fork in it.

Just added a bunch of other changes for the .get and .getJSON calls. Haven't had a chance to test it all so haven't created a PR yet but feel free to try it out. And .get that already had a .always on it, I haven't added a .fail to

So the above commit has now been incorporated into the #1269 patch so it can be fully tested.

Marking closed.

Was this page helpful?
0 / 5 - 0 ratings