witoldsz / angular-http-auth

MIT License
2.38k stars 417 forks source link

What/where is the $http circular dependency? #42

Closed stereokai closed 10 years ago

stereokai commented 10 years ago

I saw these lines in the buffer code:

/*
 * Service initialized later because of circular dependency problem.
 */
var $http;

And then:

$http = $http || $injector.get('$http');

What exactly is this circular dependency? Isn't $http a stock Angular service?

I'm just trying to understand exactly how your code works. Thank you in advance!

witoldsz commented 10 years ago

$httpProvider is responsible for configuring and creating the $http service. In order to do so, it needs httpBuffer, and httpBuffer needs $http to operate. Can you see the circular dependency now?

stereokai commented 10 years ago

I still haven't gotten around the concept of Providers in Angular, but I assume it is so because you're defining a response interceptor within $httpProvider on the config stage?

Allow me please to refine my question, if you're not too busy to answer: why does $httpProvider need httpBuffer to configurate $http?

laurelnaiad commented 10 years ago

I believe the $http service depends on any interceptors that are configured for it, and thus you can't inject it as part of the normal cycle if you want to interact with the service from the interceptor.

Side note: with all of us wondering why other response.status values aren't handled and proposing ways to handle them, perhaps it's worth considering that this is a 401 response interceptor only. There is probably room for a more generic and configurable status interceptor module that can handle status of 0 or 403 or 500 for that matter. Many things could result in responses require app-specific logic. I think the idea of using an interceptor on the status is great because it gives you the opportunity to handle it at a higher level than just, "every time I make a request I need to worry about all the possible outcomes."

I guess what I'm saying is it seems this module is purely about handling 401 in a straightforward manner and it does that. Those of us who want something more or different might want to build it.

stereokai commented 10 years ago

@stu-salsbury I wasn't implying something was missing, I am only trying to learn the ins and outs of Angular :)

I'd like to add it sounds logical to me to provide an unique interceptor for different server responses.

witoldsz commented 10 years ago

Allow me please to refine my question, if you're not too busy to answer: why does $httpProvider need httpBuffer to configurate $http?

As @stu-salsbury said, $httpProvider has nothing to my httpBuffer, it is the interceptor which needs it. It needs it, because I wrote it that way, so it can retransmit the rejected requests.

Does it make sense to you? Keep asking if you are in doubt. This is public, everyone can ask and provide the answers.

stereokai commented 10 years ago

But since the interceptor only uses httpBuffer, when a response is intercepted, can't the httpBuffer module simply be instantialized after the interceptor?

witoldsz commented 10 years ago

I am not sure if I understand correctly: you suggesting I could do this: var interceptor = function($rootScope, $q, $injector) {...} factory('httpBuffer', function($http) { ...} instead of: var interceptor = function($rootScope, $q, httpBuffer) {...} factory('httpBuffer', function($injector) {...}

Instead of using httpBuffer inside my interceptor, I would have to ask $injector to create it for me (postponed), so I would not need $injector to create a $http service inside the httpBuffer... Would that change anything?

stereokai commented 10 years ago

Yes, that is what I was thinking about. I guess it wouldn't change anything really, simply work around that small circular dependency. I am only trying to grasp how exactly Angular works - it is not the simplest of software...

witoldsz commented 10 years ago

Well, that would not work around the circular dependency problem, it would just move it from one place to the other. If I were to inject $injector to the interceptor (instead of to the service as it is now), I would have to write a stuff like:

var httpBuffer; //initialized later because of circular dependency problem
[...]
httpBuffer = httpBuffer || $injector.get('httpBufer');

Than you would ask me what it is all about that circular dependency problem, and would end up exactly in the same place as we are here now :)

stereokai commented 10 years ago

Talk about circular dependency ;)

Let me try and refine my suggestion, perhaps we are not on the same page yet: Interceptor and httpBuffer are two separate modules. Interceptor depends on httpBuffer, which depends on $http, which in turn depends on Interceptor for configuration as it is instantialized.

So far so good, right?

However, Interceptor doesn't require httpBuffer to instantialize - because its append method is only called once a response is processed. Do you see where I'm going with this?

Interjection! While I was writing this I had an epiphany: the circular dependency stems from Angular's dependency injection, which takes place on instantiation, did I get it right this time? :D

witoldsz commented 10 years ago

All you said about the interceptor which doesn't require httpBuffer to instantiate, you can say about the httpBuffer - which does not need $http to instantiate. The $http service is only called once a response is processed, isn't it? How is that different from what you have described above?

stereokai commented 10 years ago

You are right, that's true. So why is there a circular dependency if neither objects require each other to instantiate?

witoldsz commented 10 years ago

Because the truth is: it does not matter. More than that: object dependencies are supposed to be used later, not at build time, because all the provider is supposed to do is to put all the pieces together, not to execute anything against the objects dependencies. This is what makes DI pattern useful: explicit dependencies (you look at the dependencies and you know there are no more surprises hidden between lines of the method code) and easy testing, because there are no assumptions as to what the object really needs to operate.

I had to break the rule, to use $injector directly, which converts Di to Lookup pattern, but I had no choice... You can "close" the issue if you think we have talked it over, or... :)

stereokai commented 10 years ago

Thanks for taking the time to discuss this with me :) I am still missing the link in the chain that would make me understand why you needed to break the rule and go the lookup pattern way, since, as we already said, neither interceptor nor httpBuffer require the other object to instantiate.

witoldsz commented 10 years ago

Try to fix the code without looking for the dependency via $lookup explicitly. Circular dependency is circular dependency. Configuring the thing by adding a feature which needs the thing to operate will always introduce circular dependency, won't it? Hymmm.... Maybe you are right, show me the code :)

witoldsz commented 10 years ago

Well, I think - maybe I've got it this way: httpBuffer does not ask for $http when it gets created, it will accept $http as a parameter in #retryAll method instead.

Now, the authService - which is an actual API of the module, will get $http injected and it will give it to httpBuffer.

Any other ideas?

witoldsz commented 10 years ago

The only problem is, the solution above is also flawed in some way, because authService is now requesting $http service which it does not need. Asking for something which is not needed is against the DI principals (always ask only for things you need).

stereokai commented 10 years ago

That's what I'm suggesting:

  .factory('httpBuffer', ['$http', function($http) {
    /** Holds all the requests, so they can be re-requested in future. */
    var buffer = [];

    function retryHttpRequest(config, deferred) {
      function successCallback(response) {
        deferred.resolve(response);
      }
      function errorCallback(response) {
        deferred.reject(response);
      }
      $http(config).then(successCallback, errorCallback);
    }

I am testing it right now.

witoldsz commented 10 years ago

Let me guess without launching it..... Circular dependency error? How do you think it can work after all I have written here in this subject... How can one create httpBuffer with $http service as a dependency of $httpProvider?

Also, maybe we should move to the AngularJS user group. There are much more people who could benefit or contribute to the topic.

stereokai commented 10 years ago

Yeah, I was just typing, but had to do something else before I could finish:

I am getting a Uncaught Error: Circular dependency: httpBuffer <- $http <- $compile error.

You have indeed written it all out in great detail, but there's one fundamental thing we didn't touch that I think is the reason why nothing like I described would ever work with Angular. I believe DI instantiates dependencies as soon as the module or handler function referring them is called. Hence the immediate circular dependency. It seems so obvious now, haha... of course. I see why you needed to use $injector here. I feel educated yet dumb all in the same time!

Sorry if I bothered you too much. I am not only new to Angular, but to many of the concepts it builds upon too. Anyway, later today I will take this discussion into the user group.