Feature Request: Adding "onerror" option to Tone.Buffers

[mklasinc]

Hey,

I love Tone.js, but it's really frustrating that it's impossible to add an onerror callback when loading audio buffers (e.g. with Tone.Buffers, Tone.Sampler). That basically means there's no way of knowing if anything went wrong during loading.

Solution:
I poked through the source code and I see that the onerror is in fact implemented, but only on the Tone.Buffer class, but it's not included in classes that extend it. So the only thing that needs to be done is to make sure the onerror callback is included in classes that extend/build on top of Tone.Buffer, like Buffers, Sampler etc. Should be a super quick fix, but it would make a huge difference in using Tone.js, especially for production.

[rodw]

Allow me to piggy-back on this request. I was about to submit a bug on this topic, noting that there's no way to handle an error loading a sample file in Tone.Sampler. Right now if you provide an invalid URL for one of the samples the new Tone.Sampler() constructor hangs indefinitely.

Specifically, if I'm reading the code correctly:

1. The Tone.Buffer constructor supports new Tone.Buffer(/*string*/ url, /*function*/ onload, /*function*/ onerror), which should invoke onerror(err) if there's a problem loading the buffer from the given url.

2. But currently there's no way to pass the onerror callback thru Tone.Buffers (and therefore no way to pass it thru Tone.Sampler either.

3. Naively it looks like passing a map as the second parameter to the Tone.Buffers constructor, as in new Tone.Buffers(samples, {onload:onload, onerror:onerror, baseUrl:baseUrl}) (or adding a third parameter as in new Tone.Buffers(samples, onload, baseUrl, onerror), but that signature is inconsistent with the Tone.Buffer constructor) would work, but (a) the onerror callback isn't being passed to Tone.Buffer and (b) I guess there's some ambiguity about how many times onerror should be called. (For what it is worth in my opinion either stopping after the first error (such that either onerror or onload is called exactly once) or calling onerror for each failure (and maybe onload when done) would be fine.).

4. I think the Tone.Sampler constructor also supports the second-param-as-map idiom and so the same adjustment could be made there but to be honest I'm not 100% sure I am following the Tone.defaults logic correctly.

That is, I think tweaking Buffers.js and Sampler.js to use:

var options = Tone.defaults(args, ["onload", "baseUrl", "onerror"], ...);

instead of:

var options = Tone.defaults(args, ["onload", "baseUrl"], ...);

would be nearly sufficient to address this.

[rodw]

Awesome. Thanks for the quick fix!

[mklasinc]

Same here, thanks a lot!