-
Notifications
You must be signed in to change notification settings - Fork 31
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
wrapError stripping ampersand-sync's err.message. #51
Comments
This might be fixed by new |
I'm not sure how it could be.. We're calling options.error in
Now, wrapError is defined like this... var wrapError = function (model, options) {
var error = options.error;
options.error = function (resp) {
if (error) error(model, resp, options);
model.trigger('error', model, resp, options);
};
}; But that's expecting a totally different signature. It's assuming our What if we want to get access to ampersand-sync's {
// this is how it would look if we were submitting options.error
// straight to ampersand sync for use.. (no wrapError)
// and we wanted to debug with options.message
options: function (resp, errorOrSucess, messsage) {
console.log("Error message is "+ message")
}
} We can't.. What we want is something like this... var wrapError = function (model, options) {
var error = options.error;
options.error = function (resp) {
options['ampersand-sync-error'] = arguments[2];
if (error) error(model, resp, options);
model.trigger('error', model, resp, options);
};
}; Which is what my local forked copy does... Or, change the signature.. We just barely document the error callback anyway..
Our callback should either
|
Thanks for the detailed response. I believe most things here are modeled after Backbone: Does |
function wrapError
is stripping ampersand-sync's err.msg. This is unfortunate as accessing it is often necessary. What about a patch to add the original contents of the err.message asoptions['ampersand-sync-error']
(setting that to arguments[2])?The text was updated successfully, but these errors were encountered: