scottschiller / SoundManager2

A JavaScript Sound API supporting MP3, MPEG4 and HTML5 audio + RTMP, providing reliable cross-browser/platform audio control in as little as 12 KB. BSD licensed.
http://www.schillmania.com/projects/soundmanager2/
Other
4.99k stars 769 forks source link

onerror event fails during initial load if buffer state changes #160

Open robotnealan opened 7 years ago

robotnealan commented 7 years ago

Per your release notes it looks like the onerror method now fires when there's an initial failure to connect (much appreciated addition!). In testing it looks like this works properly if the source returns a 50x error immediately, but if the server takes some time to respond with this error then SM2 will change buffer states on the audio object and throw its own errors.

I've included a screenshot of the console below (it eventually retries again and repeats the errors in the protonstream1 section), but unfortunately I'm not sure how to best provide a repro case as it's only occurring temporarily due to a DNS issue with our host. Happy to provide any additional info or notes that might be of help.

Screenshot of Chrome Dev Console: screen shot 2017-06-28 at 5 00 50 pm

scottschiller commented 7 years ago

Whoa, this is weird - I think I'd seen the occasional null case with _onerror myself, but I'm not sure exactly what triggers it. I'm not sure if it's connected to the promise mentioned in console, or something else.

In any case, it's best if I make sure an object is provided to the event handler I use to listen for errors. It's strange that an error event would fire and not provide the sound scope. Maybe there's something else going on that I'm missing. 🤔

scottschiller commented 7 years ago

Oh, hang on - looking at the sequence of events, this is probably stemming from the destruct() and unload() calls firing before the error handler(s), which should also be removed.

I'll look into this!

jayepoch commented 7 years ago

Sweet - thanks for looking into this for us @scottschiller - really appreciate it.

Please let us know if we can help in anyway? Good luck!

scottschiller commented 7 years ago

Just pushed a change which checks for this._s within HTML5 event handlers, and exits if the _s (SMSound) reference is not defined.

When destroying sound objects, they unload themselves and it's possible for HTML5 events to fire after the SMSound object has been detached from the HTML5 audio object. These errors should be non-fatal, but they are avoidable and this is a good catch, thanks! 🙇

This is on the "dev" branch which I create for each release, it's fine to use this until the next major version release at which point the dev branch will be merged back into master and the version will be V2.97a.2017xxxx.

robotnealan commented 7 years ago

Holy quick turnaround Batman!

The specific situation we were encountering is unfortunately difficult to replicate as its dependent on our Shoutcast stream's process quasi-crashing, but in all the situations I have been able to test the onerror event handler is working perfectly, and SM2 isn't blowing up when this._s is undefined.

Thanks for the quick update, and as always your work is immensely appreciated!