Request a nonexistent data by id still push to the store using RestfulAdapter

[29x10]

While request a nonexistent data by id, the server return 404.

for example, GET localhost:5002/#/products/some_id_not_exists return 404

but the store still got one product with the id some_id_not_exists, the other fields are undefined.

[tomdale]

What behavior would you expect?

[jaaksarv]

@luthur Seems related to my problem (and other problems related to that):
#1810

@tomdale Expected behavior should be that there is a proper error handling mechanism for not found errors (error thrown or some hook to handle this) and Ember-Data should not create any empty zombie records automatically.

[tomdale]

@jaaksarv Is the record not going into an isError state?

[29x10]

@tomdale at least I can handle that error and the wrong undefined data should not be pushed into store

[jaaksarv]

@tomdale My case is that API (that is not under my control) returns empty response instead of 404. I can still detect when record is not found, but there is no public API for solving the this case. I use record.send('unloadRecord'); to fix it, but this solution has some side-effects. My main concern is not directly loading the record but abandoned belongsTo relationships that will create zombie records that will hang around and show up in lists.
Even with API that returns 404 there will be zombie records. Yes, they are in Error state, but they still, show up in lists etc I think?

[tomdale]

Maybe I am misunderstanding the behavior here, but it sounds like it's working as intended. My view is that errored records absolutely should be pushed into the store—if you request a person with ID of 1 and the server returns a 404, there's no reason to keep hammering the server. If multiple components of your app request person with ID of 1, they should get the same errored record. This is definitionally how an identity map works.

@jaaksarv It sounds like there's a secondary issue here, which is that errored records are showing up in queries. If that's happening, please file a separate issue describing what you're seeing.

[29x10]

@tomdale suppose you request product with ID 1, then you hit the back button to return to your app, visit the product list page, the error record is in the list.

[tomdale]

@luthur It sounds like that's a bug with fetching all records then.

[genkgo]

locks/ember-localstorage-adapter#12

localstorage also has this issue. the non existing record remains is inserted into the store and remains in loading state.

I think this issue is not a fetching-all-bug but a fetching-one-bug.

[chnn]

I can confirm that requesting a record by id that doesn't exist is caught in the loading state, even after the server returns a 404.

It's inconvenient that it's pushed in the store, but I can see how that might be the desired behavior. It just means that now something like

this.store.find('product')

has to become

this.store.find(product).filter(function(p) { 
    return !p.get('isError');  # or 'isLoading' until the loading bug is fixed
})

before being passed off to controllers or similar. Otherwise errors pop up when you get attributes of the errored model that are undefined and so on. It seems like having to filter through errored models is a bit unnecessary.

[end-user]

I too am having an issue with this. For me, the reason it's undesirable is because I'm testing to see if I have an id, on error (it didn't exist), I'd like to create a record and push it. However, since I just searched for that id, it now exists (in error state) in the store. I still can't use it, because find() still returns an error.

[vampolo]

I'm also affected by this. Doing this.store.find('myModel', 1); with a promise error pushes the record in the store anyway.
This sounds a bit weird to me since another route that has return this.store.find('myModel'); will get also the model which is in error state.

For now I'm doing:

   error: function(error, transition) {
            var watch;
            if (error.status > 300) {
                watch = this.modelFor('myModel').get('lastObject');
                this.store.unloadRecord(watch);
                this.transitionTo('myRoute');
            } else {
                this._super(error, transition);
            }
        }

[vampolo]

I also tried following the approach form @Chenn :

  model: function() {
        return this.store.filter('myModel', undefined, function(item) {
            return item.get('isError') === false && item.get('isLoading') === false;
        });
    },

But it didn't work probably to locks/ember-localstorage-adapter#12

[end-user]

So, what I ended up with is hasRecordForId to check for a record.

[vampolo]

@end-user thank for your observation. I've now solved with:

    model: function(params) {
        return this.store.find('myModel', params.id);
    },

    beforeModel: function(transition) {
        var id = transition.params['myModel.selection'].id;
        //
        // Check if watch is present using hasRecordForId instead of
        // find because find adds the record to the store anyway. More
        // on this topic at
        // https://github.com/emberjs/data/issues/1931
        //
        if (id !== undefined) {
            var hasRecord = this.store.hasRecordForId('myModel', id);

            if (hasRecord === false) {
                this.transitionTo('out');
            }
        }
    }

Looks better but still i would prefer a find() that does only a find and a findOrCreate() that adds the record if not found in the same way other ORMs do (django anyone ?)

[dgaus]

Is this supposed to be fixed on 1.0.0-beta.16.1? After a 404 I am getting phantom records which appear when doing store.all, but not via hasRecordForId

[jsangilve]

@dgaus @igorT @wecc I'm also having the same problem with 1.0.0-beta.16.1. I think #2467 was merged into 1.0.0-beta.15, but after a 404 (rejected promise), the non-existent record stays in the model.

[igorT]

Does references branch solve this for you?
When you say the non-existent record stays in the model. what do you mean by that?

[jsangilve]

@igorT, I mean, as detailed by the OP, if a route model's promise returns 404 (after requesting a nonexistent id), a nonexistent record is added to the store:

Then, If I go to a route that requests store.all, the nonexistent record is still there.

I'll try with references branch.

[jsangilve]

@igorT references branch solves the problem. The record is unloaded once the promise is rejected. I also tried with 1.0.0-beta-1.5 and it also unloads the record, so I think the problem is a regression on 1.0.0-beta.1.6.x.

Should I open an issue?

[igorT]

Hoping to get references merged soon, does your app fully work with it?

[igorT]

@jsangilve do you want to write a failing test? That would be super helpful

[jsangilve]

@igorT 1.0.0-beta-1.7 successfully unload the record. It seems to be solved by #3043

[jsangilve]

references branch didn't have var record = store.getById(typeClass, id); within _find method while 1.0.0-beta-1.6.x have it. That's the reason why I only had the bug with 1.6.x