Skip to content
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

{"data":null} throwing errors #4424

Closed
denisbetsi opened this issue Jun 7, 2016 · 11 comments
Closed

{"data":null} throwing errors #4424

denisbetsi opened this issue Jun 7, 2016 · 11 comments

Comments

@denisbetsi
Copy link

denisbetsi commented Jun 7, 2016

Looks like this issue was fixed already in

#3790

However it appears to be still happening.

Here's my setup

DEBUG: -------------------------------
ember.debug.js:6440DEBUG: Ember             : 2.5.1
ember.debug.js:6440DEBUG: Ember Data        : 2.5.3
ember.debug.js:6440DEBUG: jQuery            : 2.2.4
ember.debug.js:6440DEBUG: Ember Simple Auth : 1.1.0
ember.debug.js:6440DEBUG: -------------------------------


Here's the query:
this.store.findRecord('campaign',1);

Here's the response from server:
{"data":null}

Here's the error from ember:

ember.debug.js:32096 TypeError: Cannot read property '_internalModel' of null
    at finders.js:38
    at Object.run (ember.debug.js:678)
    at Class._adapterRun (store.js:1736)
    at finders.js:33
    at tryCatch (ember.debug.js:53806)
    at invokeCallback (ember.debug.js:53821)
    at publish (ember.debug.js:53789)
    at ember.debug.js:32054
    at invoke (ember.debug.js:333)
    at Object.flush (ember.debug.js:397)

Using
export default DS.JSONAPIAdapter.extend(DataAdapterMixin, ...

JSON API specs indicate "data": null to be a valid not found response.

@dmitrymatveev
Copy link

I have the same issue..
My setup is:

Ember : 2.6.0 Ember Data : 2.6.0

Looking into ember-data's system/store.js -> store.push(data) method and its consumer in system/store/finders.js on line:42, it seems like _find method is expecting push to always return and instance of _internalModel (which is a model factory?). However, when result of a find operation is empty as per JSON API specification the return type in null which breaks on the following line in finder.js -> _find() file.

@bmac
Copy link
Member

bmac commented Jun 24, 2016

@denisbetsi, @dmitrymatveev what is the http status code your server is returning in response to the findRecord call?

@denisbetsi
Copy link
Author

I believe it was 200. I had to do a workaround and modify that behavior to stop this error from happening, should it be 404?

@bmac
Copy link
Member

bmac commented Jul 5, 2016

Yes, Ember Data expects the server to return a 404 when there is no record for an id.

@dmitrymatveev
Copy link

dmitrymatveev commented Jul 7, 2016

@bmac
This does not quite match what JSON API specification states in the corresponding section:
jsonapi#fetching-resources

Responses

200 OK
...
A server MUST respond to a successful request to fetch an individual resource with a resource object or null provided as the response document’s primary data.

null is only an appropriate response when the requested URL is one that might correspond to a single resource, but doesn’t currently.

I got a working change that fixes it, will do a pull request when I get time to get back to it. I am just not sure how hacky is my solution.

@bmac
Copy link
Member

bmac commented Jul 7, 2016

My understanding of the line:

null is only an appropriate response when the requested URL is one that might correspond to a single resource, but doesn’t currently.

Is it is talking about the response for fetching a related resource (For example: articles/1/author). The note below the line uses a to-one related resource link in the example.

The section below http://jsonapi.org/format/#fetching-resources-responses-404 says:

A server MUST respond with 404 Not Found when processing a request to fetch a single resource that does not exist, except when the request warrants a 200 OK response with null as the primary data (as described above).

Which I believe means when fetching a particular resource like campaigns/1 the server should use a 404 status code.

@denisbetsi
Copy link
Author

It's either OR, so
404 status response

OR

200 Status with JSON body {"data":null}

Either way, current code handles 404 status but not 200 + NULL response

@dgeb
Copy link
Member

dgeb commented Jul 9, 2016

Just chiming in to confirm @bmac's understanding about the appropriateness of a 200 response with {data: null}. See the note immediately below the quoted line in the spec for an example of when this response is appropriate:

Note: Consider, for example, a request to fetch a to-one related resource link. This request would respond with null when the relationship is empty (such that the link is corresponding to no resources) but with the single related resource’s resource object otherwise.

If findRecord() is only used to fetch resources at their canonical URLs (i.e. via self links), and never related resources, then I don't see the need for a code path in this method to handle 200 responses with {data: null}.

@dmitrymatveev
Copy link

@dgeb, in my example there is a need to fetch a resource from a server which responds with null instead of 404 (self link not the relationship) for a given resource URL (unfortunately nothing about the back-end in use is canonical and it is not possible to change this). To my knowledge I can only signal to Ember that the resource does not exist using the null formatted data object returned from the custom DS.Adapter in this case.

P.S. As a side note... it seems that choosing to ignore parts of the core of the specification defeats the purpose of following specification entirely.

@wecc
Copy link
Contributor

wecc commented Oct 21, 2016

200 responses with data: null is OK for relationship related links but that's not the case here so a 404 should be expected.

A server MUST respond to a successful request to fetch an individual resource with a resource object or null provided as the response document’s primary data.

null is only an appropriate response when the requested URL is one that might correspond to a single resource, but doesn’t currently.

Note: Consider, for example, a request to fetch a to-one related resource link. This request would respond with null when the relationship is empty (such that the link is corresponding to no resources) but with the single related resource’s resource object otherwise.

I disagree with the statement that we ignore parts of the specification. I'd argue that we do follow the specification and our interpretation of it is correct, as confirmed by @dgeb.

@bmac
Copy link
Member

bmac commented Oct 21, 2016

I'm going to close this issue. If you would like your adapter to reject a response of {"data":null} with a 200 status code for a store.findRecord request you will need to define a custom adapter . By default the JSONAPIAdapter will assume a 200 status code means the record was successfully returned. This is consistent with our understanding of the spec and confirmed by @dgeb.

(Note: {"data":null} with a 200 status code is still supported for requests on relationship links to signify the relationship is empty).

@bmac bmac closed this as completed Oct 21, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants