TypeError Cannot read property 'id' of null if API response { "data": null }

[jelhan]

If API response with { "data": null } to a find request, this warning throws a TypeError:

data/packages/store/addon/-private/system/store/finders.js

Lines 53 to 59 in 637d606

	warn(
	`You requested a record of type '${modelName}' with id '${id}' but the adapter returned a payload with primary data having an id of '${payload.data.id}'. Use 'store.findRecord()' when the requested id is the same as the one returned by the adapter. In other cases use 'store.queryRecord()' instead.`,
	coerceId(payload.data.id) === coerceId(id),
	{
	id: 'ds.store.findRecord.id-mismatch',
	}
	);

payload.data is null. Trying to access id on null throws.

Actually it doesn't make much sense to return { "data": null } by our API if the resource does not exist but it's explicitly be allowed by JSON:API spec in some cases:

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.

https://jsonapi.org/format/#fetching-resources

I'm facing this issue in Ember Data 3.14.0 but from reading changelog and inspecting the code that has triggered it, I'm quite sure it's still the same for 3.16.0-beta.0.

[runspired]

@jelhan this has been true for probably 2 major versions. The correct response here is to throw a 404 not-found error.

[jelhan]

Not saying that the API I'm fighting with is behaving correctly... 🤷‍♂️ But I think Ember Data should not throw a TypeError in that case but a more meaningful warning / assertion.

Supporting { data: null } instead of 404 is another topic. It might be good for long-term as it's explicitly allowed for some cases but I totally understand that it's not a priority.

[runspired]

@jelhan in this case not returning a 404 is incorrect per the spec. If you've explicitly said "I expect a record matching this type and id and then the API says "sorry no such record" the response should error.

[jelhan]

I'm not arguing that returning { "data": null } is a good practice for find requests. Asking to support it in Ember Data was also not my main intend when opening this ticket. I'm mostly caring about not throwing a TypeError` but a helpful error / assertion in that case.

Having that said I'm still tending to disagree that returning a 200 with { "data": null } is not allowed by the JSON:API spec at all.

@jelhan in this case not returning a 404 is incorrect per the spec. If you've explicitly said "I expect a record matching this type and id and then the API says "sorry no such record" the response should error.

I'm not sure if the spec is that strict. It allows to return 200 with { "data": null } in some cases:

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.

[...]

404 Not Found

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).

The spec gives an example for that case:

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.

That example is about a relationship link but I would argue that there are also cases in which a URL for a specific combination of type and ID may "correspond to a single resource, but doesn’t currently." One example I could imagine is a blog post that isn't published yet.

[runspired]

@jelhan looking at this again and totally agree we should assert here. Not sure why I was so opposed before. Thanks for surfacing!

[runspired]

Related to #7547