Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Unable to override sync error behavior in 0.9.9 #2031

Closed
kpdecker opened this Issue · 9 comments

4 participants

@kpdecker

From 0.9.2 -> 0.9.9 it's no longer possible to override the behavior of the the error handler, only augment it. As a consequence it's not possible to prevent error responses from triggering on the model (yes in the ideal world you would probably want this to happen but not all services tiers are created equal :) )

Worse the error handler signature changed between the two versions making this a breaking change for any code that used Backbone.sync directly and provided an error handler.

@braddunbar
Collaborator

Hi @kpdecker! Would you mind elaborating on your use case a bit? In particular, why do you need to suppress the "error" event?

You're right about the handler signature, but I think that the change is for the best as the previous signature did not include the model like most other events.

@kpdecker

Error:
Through a custom error handler we added the concept of an ignored error for background processes, etc that would not trigger the user facing error handlers.

Signature:
Success doesn't even match this pattern of adding the model...

options.success = function(resp, status, xhr) {
  if (success) success(resp, status, xhr);

While the error callback does not echo the AJAX callbacks and actually masks a few of the parameters from the AJAX-layer.

options.error = function(xhr, status, thrown) {
  if (error) error(model, xhr, options);

And complete is left with the raw AJAX callbacks. I agree that there should be consistency here, but I'd argue that it should mirror the AJAX callback APIs rather than trying to do your own thing.

@braddunbar
Collaborator

I think you can probably still ignore the error via a custom option (which I think is probably the best way). It will be passed through to the error callback/handler and you can check it there.

model.save({...}, {ui: false});

As for the callback signature, please check out #2003 for the updated version (not what you posted above) and post comments/question there.

@tgriesser
Collaborator

What if the success and error (and maybe "beforeSend") were split out as static properties on the sync object:

Backbone.sync = function () { 

   .... 

   Backbone.sync.error.call(this, ...)
   Backbone.sync.success.call(this, ...)
}
Backbone.sync.error = function () { ... }
Backbone.sync.success = function () { ... };

That way they could easily be replaced by the user

@braddunbar
Collaborator

The success and error handlers are really doing very little. They just trigger an event on the model/collection and normalize the arguments to fit Backbone's patterns (passing the model to the callback). It's fairly easy to ignore the events and get to whatever options you need. I'd rather not complicate this with more customization hooks if it's not completely necessary.

@kpdecker

I guess the use case can be hacked together using Backbone.ajax but it feels a lot fuglier rather than overriding this.sync only on instances where it mattered.

@eastridge

@kpdecker does #2051 provide the needed hook?

@kpdecker

@eastridge Those methods could be used to implement the behavior that was there before but I've frankly moved on as for the use case we are dealing with it was simpler to just rewrite the sync method without all of the emulation crap that somehow made it into a merged pull request. This is kind of becoming the norm for backbone projects since PR are too much of a fight to get anything in.... :(

@tgriesser
Collaborator

Closing this one out, since the main issues should be addressed with the changes in #2221.

@tgriesser tgriesser closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.