Better validation failure handling about backbone HOT 8 CLOSED

I completely agree with you. Take a good look at this patch:

http://github.com/documentcloud/backbone/commit/a09bcbca9d660b09dd010e6fc92c5c6c32f10bbb

Ideally, you add an "error" event listener to a model or entire collection as a way to provide a general catch-all case for failed validations and server requests. But if you have a particular view that can better display the error, being able to override and suppress the event for specific cases is ideal. The patch standardizes the arguments for error callbacks ((model, error)), and makes it so that if you pass an error callback to set or save, the callback will be used instead of the general event. Your example should now look like this:

my_model.save(null, {
  error: function(model, error) {
    alert(error);
  }
});

Sound like a plan?

from backbone.

So...this means that #bind has a different signature for validation errors? The callback has an eventName as an argument, unless it's a validation error and then has model, error as an argument? I'm a little skeptical about that...I guess I was expecting one of the two:

model.bind("error", function(eventName, model, error){...};
// or
model.bind("error", function(eventName, errObj){...});
// where errObj = {model: foo, value: "invalid name provided"};

Anything, so long as it's "backwards compatible" with the existing bind method. Changing signature by event type is a bad idea!

I still sort of prefer setting an error property on the model itself, but the solution you've provided is sufficient as well. The error property way is a little simpler, IMO, but with the downside of polluting the model object. You'd only ever check it if save or set failed (returned false) anyway, so I don't see any actual /problems/.

Lastly, why is the model passed into the error callback in the #save example above? Shouldn't the error callback already be getting called in the context of the model? Perhaps for consistency with the #bind-error version..?

from backbone.

Nope, the signatures should be roughly the same (the error objects may differ):

model.bind("error", function(model, error){ ... });

model.save(null, {error: function(model, error){...}});

Is that better? I agree that it's slightly more verbose than your solution, but I don't think that setting a magic property that sticks around after the fact to clutter things up is really an acceptable API for this.

Models are the first argument to error callbacks because you may have a generic error handler that's not bound to a specific model...

from backbone.

Alright, I'll buy your model-should-be-passed-in argument :)

What I meant about differing callback signatures was now you have
model.bind("error", function(model, error){ ... });
but also
model.bind("change", function(eventName){ ... });

Do we want to say "the arguments to the callback depend on the event"?

from backbone.

The arguments to the callback do get passed depending not just on the event, but how you trigger it. Any subsequent arguments to trigger are forwarded along.

However, in terms of built-in Backbone events, the argument signatures are relatively similar. eventName is never passed as the first argument, and model always is, for every event. The second argument is passed in two cases:

• For "error" events, the second argument is the error object.
• For "change:key" events, the second argument is the new value of the key.

from backbone.

Hmm, okay. I withdraw my statement then :)

from backbone.

I still prefer setting a top-level attribute on the model instance. For example, an extra attribute is_valid on a model instance obj would be set after validation and is accessible via obj.is_valid. Since it is not stored in the internal attributes attribute, and not accessible from obj.get('is_valid'), I doubt this would be considered as polluting the model object. The plus side is that then you can filter objects in a collection easily before, say, sending to the persistence layer.

from backbone.

An is_valid flag doesn't make much sense on a model that has a validate method -- because models are always valid. They don't allow you to set invalid properties...

from backbone.