Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Router, RESTAdapter.find, 401 Unauthorized and DS.LoadPromise not playing nicely #838

Closed
emk opened this Issue Mar 27, 2013 · 5 comments

Comments

Projects
None yet
5 participants

emk commented Mar 27, 2013

This is a spinoff from a comment on emberjs/ember.js#2041, now that I've narrowed down the problem a bit.

The context

We have a global App.get("user") value which is initialized either using deferReadiness/advanceReadiness (on application load) or via a login route (once the application is loaded).

App.AuthenticationRequiredMixin = Ember.Mixin.create
  redirect: ->
    unless App.get("user")
      @transitionTo("login")

App.WidgetRoute = Ember.Route.extend App.AuthenticationRequiredMixin,
  model: (params) ->
    App.Widget.find(params.widget_id)

I don't know whether this is a good or bad design, but it's what I came up with after reading the docs.

The weirdness

The problem occurs when somebody goes directly to #/widgets/3 without having been logged in. In this case, model gets called, and the server returns 401 Unauthorized. Then I just sit there looking at a completely blank page with no layouts until I type:

w=App.Widget.find(3)
w.get("name")

At this point, my long-delayed redirect function finally gets called, and I snap to the login route.

What's going on

The code for RESTAdapater find never bothers to report errors to anybody:

  find: function(store, type, id) {
    var root = this.rootForType(type);

    this.ajax(this.buildURL(root, id), "GET", {
      success: function(json) {
        Ember.run(this, function(){
          this.didFindRecord(store, type, json, id);
        });
      }
    });
  },

So when the server returns 401 Unauthorized, our Widget remains in an unloaded state, because there's no AJAX error callback. But DS.Model mixes in DS.LoadPromise, so the router is perfectly happy to wait around forever, showing a blank page, until something happens. The key is the one containing then(proceed):

      if (object && typeof object.then === 'function') {
        loading(router);

        // The chained `then` means that we can also catch errors that happen in `proceed`
        object.then(proceed).then(null, function(error) {
          failure(router, error);
        });
      } else {
        proceed(object);
      }

Then, for reasons which I haven't investigated, when we manually call the following:

w=App.Widget.find(3)
w.get("name")

…we force the object into an isLoaded state (which is pretty weird, actually, since there's no data and it's not really a valid object, which is possibly a bug in and of itself). At this point, the router notices that the LoadPromise can be resolved, and it finally gets around to calling redirect. At this point we finally notice we don't have a user, and transition to our login screen.

Contributor

workmanw commented Mar 27, 2013

I think the same could really be said for most errors, 403 and 404 specifically. It would be helpful if the router had a callback to handle when a model's promise was rejected. Presently there isn't much flexibility and to make it work you typically have to circumventing the natural work flow.

Contributor

jasonkriss commented Mar 27, 2013

Take a look at this proposal. Seems like the path forward.

emk commented Mar 27, 2013

@workmanw If you look at the code sample, I think the router actually tries to handle rejected promises. But find(ID) and findAll() never reject their promises, so the router hangs hard, even if a later redirect would know how to deal with the situation.

@jasonkriss That seems like an interesting proposal, and it would help with issue (1) below. But that still leaves (2) and (3).

  1. Why don't find and findAll reject their promises on AJAX errors?
  2. Why does calling get("name") on the unloaded object returned by a failed find resolve the associated promise successfully? That can't possibly be a good thing. I'd make a Fiddle to demonstrate this, but I think it requires a real backend server to demonstrate.
  3. What should ember-data and find do with transient errors like 401 Unauthorized that will presumably go away a few seconds later? We're eventually going to want to try another find and get back a valid object, so we don't want to leave the model in an error state indefinitely.

This is a pretty interesting set of issues, and I don't have any good answers yet.

Owner

wagenet commented Sep 6, 2013

Does this issue still apply to ED 1.0?

Owner

wycats commented Sep 7, 2013

Nope.

@wycats wycats closed this Sep 7, 2013

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment