Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Uncaught TypeError: Cannot read property dismiss of null (alert.js) #1240

Closed
E3V3A opened this issue Mar 31, 2018 · 12 comments
Closed

Uncaught TypeError: Cannot read property dismiss of null (alert.js) #1240

E3V3A opened this issue Mar 31, 2018 · 12 comments

Comments

@E3V3A
Copy link
Contributor

E3V3A commented Mar 31, 2018

I keep getting random events where the error is:

Uncaught TypeError: Cannot read property 'dismiss' of null

alert_bug

The error is coming from the Alert default module on Line 113, in alert.js:

	hide_alert: function(sender) {
		//Dismiss alert and remove from this.alerts
		this.alerts[sender.name].dismiss();  // <<========= LINE  113 !
		this.alerts[sender.name] = null;
		//Remove overlay
		var overlay = document.getElementById("overlay");
		overlay.parentNode.removeChild(overlay);
},

@MichMich
Does this have to be null or can it also be set to "" or something else?
Any ideas how to fix or work around this?

I'm surprised nobody else has reported this issue, so I want to eliminate that it is a problem with my module.

@E3V3A
Copy link
Contributor Author

E3V3A commented Mar 31, 2018

I tried to fix with some variations of this:

        //Dismiss alert and remove from this.alerts
//        if (this.alerts[sender.name] != null) {
            this.alerts[sender.name].dismiss();
            this.alerts[sender.name] = "";        // was null
 //       }

But it didn't work...and seem to need something else.

@MichMich
Copy link
Collaborator

MichMich commented Apr 1, 2018

It's because a alert is dismissed which doesn't exist. Which module is firing the alerts? Might be worth checking that code as well.

@E3V3A
Copy link
Contributor Author

E3V3A commented Apr 1, 2018

It's our module MMM-Assistant, that seem to use Alerts to show pop-up screen results for Snowboy requests...

But, why didn't my if() statement work then? (Actually it seem to have worked, but then got another error message instead.)

@fewieden
Copy link
Contributor

fewieden commented Apr 2, 2018

if (this.alerts[sender.name]) { would cover null, undefined empty string and more

@E3V3A
Copy link
Contributor Author

E3V3A commented Apr 2, 2018

@fewieden Yeah, but I already tried that before the comments shown above, and it just propagated the error elsewhere... Seem to be some kind of race condition...

  • Why would something else be closing it? (Should a module close it's own?)

Or maybe I need to enclose the whole thing before the closing bracket?

@fewieden
Copy link
Contributor

fewieden commented Apr 2, 2018

what is the other error?

I think you need to put the dom actions into your if statement

var overlay = document.getElementById("overlay");
overlay.parentNode.removeChild(overlay);

if there is no alert to dismiss, there should also be no dom action.
if the overlay is not there, you're trying to access parentNode of null

@MichMich
Copy link
Collaborator

MichMich commented Apr 2, 2018

Might be a good idea to check with @paviro, he made the alert module.

@paviro
Copy link
Contributor

paviro commented Apr 2, 2018

Oh man :D that was ages ago. I will have a look later today :)

E3V3A added a commit to E3V3A/MagicMirror that referenced this issue Apr 2, 2018
Make sure there is something to remove, before we attempt to remove the notifications. 
- This fixes MagicMirrorOrg#1240
@MichMich
Copy link
Collaborator

MichMich commented Apr 2, 2018

@paviro It will hunt you for the rest of you life. ;)

@paviro
Copy link
Contributor

paviro commented Apr 2, 2018

I really hoped someone would replace this 💩 code really fast 😅

@MichMich
Copy link
Collaborator

MichMich commented Apr 2, 2018

@E3V3A If that fix seems to work for you, feel free to send a PR. I'll merge it after all test's are finished.

@E3V3A
Copy link
Contributor Author

E3V3A commented Apr 2, 2018

Just tested, @fewieden suggestion fixed it. Now the question is why this happened in the first place.

Done, while you guys were chatting... ;)
#1248

@E3V3A E3V3A closed this as completed Apr 2, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants