sensu / sensu-dashboard

A dashboard for Sensu, for displaying & managing events & clients.
http://sensuapp.org
MIT License
96 stars 44 forks source link

fix null object error when an event is removed between checks #127

Closed joemiller closed 11 years ago

joemiller commented 11 years ago

as discussed on irc, i believe this fixes an error that causes refreshing the events table to stop working if an event is removed between updates.

the error in the console is: Uncaught TypeError: Object #<Event> has no method 'isEmpty'

joemiller commented 11 years ago

ping. @amdprophet @portertech . would like to get this reviewed and merged if it's ok. or, if this is not the right fix then something else. But in our case we've been running it for a few weeks and it fixes the issue with results never clearing from the board.

amdprophet commented 11 years ago

I'll take a look at noon and get this dealt with today. I have a feeling there's a better fix but I haven't found it yet.

vitoravelino commented 11 years ago

Hey guys!

A friend of mine was having a similar issue and he asked me to help him debugging it. I found the bug and after I came to here and saw this PR.

Actually the problem is that in the initialize method the List is listening to the 'remove' event http://dl.dropbox.com/u/27144161/Screenshots/3r.png

and renderEmpty() has declared the wrong parameters order http://dl.dropbox.com/u/27144161/Screenshots/3s.png

as you can see from Backbone documentation. http://dl.dropbox.com/u/27144161/Screenshots/3t.png

And when the event is fired that exception occurs.

So, maybe the right fix is to create a new function with the correct parameters to listen to the remove event and then call renderEmpty() passing it the second parameter (collection).

(Dropbox is having issues by serving the files. Try to refresh the browser if the 3 images doesn't load.)

amdprophet commented 11 years ago

That is most definitely the issue. Working on a fix now.

amdprophet commented 11 years ago

@joemiller I think this should fix it - can you give it a shot? https://github.com/amdprophet/sensu-dashboard/commit/2d22d9a244de946761415c868f1048b09a81aa66

joemiller commented 11 years ago

still breaks when item is removed

On Mon, Apr 22, 2013 at 2:12 PM, Justin Kolberg notifications@github.comwrote:

@joemiller https://github.com/joemiller I think this should fix it - can you give it a shot? amdprophet@2d22d9ahttps://github.com/amdprophet/sensu-dashboard/commit/2d22d9a244de946761415c868f1048b09a81aa66

— Reply to this email directly or view it on GitHubhttps://github.com/sensu/sensu-dashboard/pull/127#issuecomment-16824204 .

jamesdphillips commented 11 years ago

I think @listenTo(@collection, "add", @render) will fail as well.

"add" (model, collection, options) — when a model is added to a collection.
joemiller commented 11 years ago

@amdprophet I think the patch you referenced fixes it. It's working for me now. I was mistaken before, I had a previously defined breakpoint in chrome from 3 weeks ago that I forgot about and was trying to signal an error to me :( Sorry for the confusion.

amdprophet commented 11 years ago

@joemiller no problem. I'm going to make a similar fix to the "add" listener as @jamesdphillips suggested and then I'll open a new PR.

portertech commented 11 years ago

@amdprophet @jamesdphillips Any fixes hanging around?

amdprophet commented 11 years ago

Pull Request #137 has a fix. @kdaniels - are you able to test #137 and see if it fixes the issue for you?

ryndaniels commented 11 years ago

@amdprophet I tested #137 and I no longer see the null object error in the dev console and the event count updates automatically, but the event list still does not.

amdprophet commented 11 years ago

@kdaniels if you have any dashboard settings (aside from credentials) can you gist them? What browser & browser version are you using? Do you have any extensions that could be affecting JavaScript and/or AJAX requests? Are you on the latest sensu & sensu-dashboard? I'll try to replicate the same scenario you're getting over the weekend.

ryndaniels commented 11 years ago

@amdprophet I'm seeing this in both Safari 6.0.5 and Chrome 28.0.1500.71, with all extensions disabled. I'm running sensu 0.10.0 and dashboard 0.9.9. The only settings I've got are https://gist.github.com/kdaniels/2a95ab40c04126d4b9d4