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

can.Model Fails Silently When Server Doesn't Return an Array for findAll #384

Merged
merged 1 commit into from Jul 24, 2013

Conversation

Projects
None yet
4 participants
@daffl
Contributor

daffl commented Jul 24, 2013

No deferred method is executed if invalid data is returned. For example:

myModel.findAll({myData: 'myValue'}, function(result) {
    // do something here
}).fail(function(xhr, textStatus, errorThrown) {
  // do something here
});

Success callback or fail callback not called. This hold true for 'then'.

@daffl

This comment has been minimized.

Show comment
Hide comment
@daffl

daffl May 10, 2013

Contributor

Could you verify this happening in all cases or just for cross domain requests as mentioned in #385? How did you implement findAll, update etc. in the cross domain case? Afaik, JSONP requests don't explicitly fail, you actually have to examine the response and fail the deferred yourself.

Contributor

daffl commented May 10, 2013

Could you verify this happening in all cases or just for cross domain requests as mentioned in #385? How did you implement findAll, update etc. in the cross domain case? Afaik, JSONP requests don't explicitly fail, you actually have to examine the response and fail the deferred yourself.

@adrianmaurer

This comment has been minimized.

Show comment
Hide comment
@adrianmaurer

adrianmaurer May 13, 2013

We made a fiddle to demonstrate this. Looks like the deferred is called with an ajax but not the model.

http://jsfiddle.net/mattseburn/vTnDS/1/

We aren't doing any cross domain requests. It was actually a coincidence that we noticed this behaviour because a local apache proxies wasn't set up. But it would be nice to know if a JSONP requests possible with can.Model to inspect the response?

adrianmaurer commented May 13, 2013

We made a fiddle to demonstrate this. Looks like the deferred is called with an ajax but not the model.

http://jsfiddle.net/mattseburn/vTnDS/1/

We aren't doing any cross domain requests. It was actually a coincidence that we noticed this behaviour because a local apache proxies wasn't set up. But it would be nice to know if a JSONP requests possible with can.Model to inspect the response?

daffl added a commit that referenced this pull request Jul 24, 2013

Merge pull request #384 from bitovi/reject-findalls-384
can.Model Fails Silently When Server Doesn't Return an Array for findAll

@daffl daffl merged commit bf64247 into master Jul 24, 2013

1 check passed

default The Travis CI build passed
Details

@daffl daffl deleted the reject-findalls-384 branch Jul 24, 2013

@daffl

This comment has been minimized.

Show comment
Hide comment
@daffl

daffl Jul 24, 2013

Contributor

Allright, the findAll and findOne deferreds now reject when an exception gets thrown during .model or .models conversion and the default .models throws an error if it can't convert the data (i.e. the data is undefined).

Contributor

daffl commented Jul 24, 2013

Allright, the findAll and findOne deferreds now reject when an exception gets thrown during .model or .models conversion and the default .models throws an error if it can't convert the data (i.e. the data is undefined).

@justinbmeyer

This comment has been minimized.

Show comment
Hide comment
@justinbmeyer

justinbmeyer Jul 29, 2013

Contributor

Can you weigh in on: #454. Seems like this was added for a reason.

Contributor

justinbmeyer commented on model/model.js in 1dfc161 Jul 29, 2013

Can you weigh in on: #454. Seems like this was added for a reason.

This comment has been minimized.

Show comment
Hide comment
@scorphus

scorphus Jul 29, 2013

Contributor

And the reason was #384.

Contributor

scorphus replied Jul 29, 2013

And the reason was #384.

@justinbmeyer

This comment has been minimized.

Show comment
Hide comment
@justinbmeyer

justinbmeyer Jul 29, 2013

Contributor

So was can.Model truly silently failing (no error in the console), or was the fail not called?

IMO, an error is a different case than a ajax request fails to load. I'm not sure the try-catch was ever appropriate.

Contributor

justinbmeyer commented Jul 29, 2013

So was can.Model truly silently failing (no error in the console), or was the fail not called?

IMO, an error is a different case than a ajax request fails to load. I'm not sure the try-catch was ever appropriate.

daffl added a commit that referenced this pull request Aug 30, 2013

Merge pull request #455 from scorphus/issue-454-canjs-swallowing-errors
Catch only exceptions thrown by model[func]() (fix #454 and re #384)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment