Add hook for ko.onBindingError

[brianmhunt]

This PR should exposes a function (ko.onBindingError) to hook into binding errors.

This should give greater control to users handling cases such as #1626

The function onBindingError(spec) takes one object, spec containing the following properties:

Property	Meaning
during	(string) The binding function name (init or update or apply)
errorCaptured	(Error) The error that was thrown by the binding function
element	(HTML Node) The node the binding was being applied to
bindingKey	(string) The name of the binding (init or update only)
valueAccessor	(function) As passed to the binding function (init or update only)
allBindings	(function) As passed to the binding function (init or update only)
bindingContext	(object) As passed to the binding function (contains $data, $rawData, $root, ...)

Tested on PhantomJS, Chrome, Safari, and Firefox.

[xavierzwirtz]

I like it, makes the error handlers you referenced in #1626 much easier to write.
+1

[brianmhunt]

Thanks @voiceofwisdom ... My thinking too is that there are lots of folks using error reporters like (from https://plus.google.com/+PaulIrish/posts/12BVL5exFJn):

• github.com/Offbeatmammal/jsErrLog
• github.com/occ/TraceKit
• bugsense.com
• jslogger.com
• qbaka.com
• muscula.com
• errorception.com
• exceptionhub.com
• bugsnag.com
• exceptional.io
• airbrake.io
• getsentry.com

Plus any number of variants the in-house. Writing a custom binding provider is a somewhat ominous barrier to work with these services, especially if you're new to Knockout.

I glean from their number that there is some popularity to them.

[brianmhunt]

@mbest, @rniemeyer @SteveSanderson — any thoughts on this PR?

[rniemeyer]

I think this is good functionality. A custom binding provider is harder to jump into than a hook like this one.

[brianmhunt]

A couple extra features:

• Errors on subsequent calls to the update function will now be captured and passed to ko.onBindingError
• Multiple applyBindings will call ko.onBindingError
• The default onBindingError error message will now print "Unable to process binding \"" + spec.bindingKey + "\" in binding \"" + bindingText + "\"\nMessage: " + error.message, where bindingText is the entire binding string (e.g. the data-bind attribute)

Tested on Chrome, Safari, Firefox, and PhantomJS.

[brianmhunt]

Is this "outer" try necessary if we have the "inner" try below inside the ko.dependentObservable?

[brianmhunt]

Thank you @rniemeyer !

@mbest, @SteveSanderson - I am tagging this for 3.3 - even though I expect it would not make it in. This PR seems a fairly straightforward patch, would be handy for lots of folks, and it would be good to get it into the "stream" for release (expecting it'll be 3.4 or later, but great if it were good enough for 3.3).

The one concern I have is that errors will now be thrown when handlerUpdateFn is called, not just when it is bound/initialized. i.e. this differs from the current handling.

I would be very grateful for thoughts on how “mergeable” this looks.

Cheers

[SteveSanderson]

@brianmhunt, could you let us know what (if anything) you think we should still do here, in light of #1715 having been merged now?

[brianmhunt]

@SteveSanderson – I've been mulling it.

I am guessing the easiest way to preserve the functionality would be to directly attach the metadata to the error instances. We just have to be wary of clobbering anything currently or possibly in the future native to the Error, but the spec for Errors is pretty bare.

We could just change the onBindingError spec and respective calls so the spec looks like this:

    ko.onBindingError = function(error, spec) {
        var bindingText;
        ko.utils.extend(error, spec);
        if (spec.bindingKey) {
            // During: 'init' or initial 'update'
            bindingText = ko.bindingProvider['instance']['getBindingsString'](spec.element);
            error.message = "Unable to process binding \"" + spec.bindingKey + 
                 "\" in binding \"" + bindingText + "\"\nMessage: " + error.message;
        } // else: During: 'apply'

        if (ko['onError']) {
          ko['onError'](error);
        } else {
          throw error;
        }
   }

Does that sound – roughly – like a reasonable tact?

Incidentally, I noted that ko.utils.catchFunctionErrors rethrows an error even if ko.onError exists. Might it be preferable to leave it up to the user whether they want the error to rethrow? They can easily add it to their ko.onError handler.