witoldsz / angular-http-auth

MIT License
2.38k stars 417 forks source link

Add dont retry option #68

Closed jameskleeh closed 10 years ago

jameskleeh commented 10 years ago

I have a situation where I want an http request to trigger the login page on 401, but I don't want it to be retried. Adding this check will allow someone to write $http({... , dontRetry: true})

witoldsz commented 10 years ago

Is it for real? The thing you have just implemented is implemented already 2 lines above your change. Also, why would you create create deferred object just to make a decision one line later that you will never use it?

jameskleeh commented 10 years ago

ignoreAuthModule: true will not trigger the event event:auth-loginRequired. I want this event to be triggered, however I don't want the call to be retried. The deferred is returned via return deferred.promise, so it is used. Maybe you need to look at it closer.

witoldsz commented 10 years ago

I still don't get it: why would you create and return a promise if you are never going to resolve or reject it? Doesn't it look awkward to have one ignoreAuthModule which makes the request to be ignored by the module and second flag dontRetry which does... what it does?

What is your use case? If your login endpoint returns 401 on invalid login (which is pretty usual), then why won't you just use ignoreAuthModule flag? If you have a special need to call auth:loginRequired on 401 of your login endpoint (this seems odd, why do you need it?), then just add an error handler to your request and broadcast this message. That would be so much more expressive and easier to understand, wouldn't it? Also, there would be no need to tweak the module.

$http.post('/login', credentials, {ignoreAuthModule: true}).then(
  function() { /** user logged in */ }, 
  function(err) { err.status === 401 && $rootScope.$broadcast('auth:loginRequired');}
);

Or I can just accept it, then answer hundred times what is this for, then someone will find it extremely useful to add rejectButDontRetry flag by adding one more if somewhere in between, and so on and so forth...

I hope you can see my point of view?

jameskleeh commented 10 years ago

I see your point of view, however I don't feel it is an option that is difficult to understand, given it is documented correctly. To me it is pretty simple, on a per $http basis:

  1. Use the module as normal
  2. Don't use the module at all {ignoreAuthModule: true}
  3. Use the module, but don't retry the request {dontRetry (whatever you want to call this): true}

There really isn't any other functionality in your module to add flags to. If you feel it is too much for you to manage, I will simply fork your repo. I just thought some users might be able to use this feature. Of course, if you see improvements that can be made, go ahead and make them.

Either way, here is my use case:

When a user switches views in Angular (ng-route or ui.router), the html is retrieved from cache if the view was accessed previously. If the view is retrieved from cache and there are no ajax calls to trigger a 401, the user will see the view even though he or she has an expired session. My solution was to register an event handler on view change to make a simple $http call to trigger the 401 from the server and thus, show the log in page. I do not wish to retry that simple call because it has already served its purpose of triggering the 401.

witoldsz commented 10 years ago

As far as I can see, there is no reasonable use case other than the very special one you have provided. However:

You said:

What is different in your case?

jameskleeh commented 10 years ago

The difference in my case is that in some views (templates), there are no calls to retrieve secured data. The template is pulled from cache, displayed to the user, and he/she is unaware the session is expired.

But really if you don't get it by now - Just close this. I'll use my fork.

witoldsz commented 10 years ago

I do get it, I just don't find it suitable to add such a thing to the module. You can use your fork, you can also just use the ignoreAuthModule:true and handle the 401's manually.

Also, if the view does not load secured data, then you can just leave it. Why to bother user if there is no need. Once user do something that actually requires to fetch sensitive data, the auth module act accordingly.