witoldsz / angular-http-auth

MIT License
2.38k stars 418 forks source link

Issue using ng-file-upload and http-auth-interceptor #113

Closed xeor closed 8 years ago

xeor commented 8 years ago

I created https://github.com/danialfarid/ng-file-upload/issues/1010 yesterday for what I didnt knew if was a bug in ng-file-upload (which I need to use), or http-auth-interceptor.

Basically, I cant just use $http for calling a REST resource, because I need a file-upload capability with a progress-bar (and some other features ng-file-upload provides).

When http-auth-interceptor intercepts the 401 that ng-file-upload is triggering, I am hitting an TypeError: Cannot read property 'addEventListener' of undefined in ng-file-upload, because:

It seems like http-auth-interceptor is overwriting the XMLHttpRequest but it does
not implement the xhr.upload causing this error, so you can open a bug on their side.

Any idea how I can solve this problem? It happens when loginConfirmed is triggered, so I guess I need to update $http(config) at that point, but I dont know the internals of how this works, so a little push might be useful. ng-file-upload is very useful, so it would be a nice thing for http-auth-interceptor to support (or have a little doc on how to support)..

alex-training commented 8 years ago

Have the same issue. Guys have you found any solution?

witoldsz commented 8 years ago

Hi guys, I am little of out scope right now, sorry for this, but may I suggest you look at the code of http-auth-interceptor to find answers for your questions? It's all just few lines of code in one file.

xeor commented 8 years ago

Hi

I have looked, but I'm not familiar with how AngularJS deals with all this internally, (and I'm still kinda new to js). Didn't find any clues first look. I can look again, but don't think I will get any further. Any hints for what it can be?

alex-training commented 8 years ago

Hi guys! I have looked at the code of http-auth-interceptor and I think that the problem is on ng-file-upload side

https://github.com/danialfarid/ng-file-upload/blob/master/dist/ng-file-upload.js If i comment out lines 66-83 it fixes the issue

alex-training commented 8 years ago

IMHO here is buggy code

if (window.XMLHttpRequest && !(window.FileAPI && FileAPI.shouldLoad)) {
  window.XMLHttpRequest.prototype.setRequestHeader = (function (orig) {
    return function (header, value) {
      if (header === '__setXHR_') {
        var val = value(this);
        // fix for angular < 1.2.0
        if (val instanceof Function) {
          val(this);
        }
      } else {
        orig.apply(this, arguments);
      }
    };
  })(window.XMLHttpRequest.prototype.setRequestHeader);
}

and

config.headers.__setXHR_ = function () {
  return function (xhr) {
    if (!xhr) return;
    config.__XHR = xhr;
    if (config.xhrFn) config.xhrFn(xhr);
    xhr.upload.addEventListener('progress', function (e) {
      e.config = config;
      notifyProgress(getNotifyEvent(e));
    }, false);
    //fix for firefox not firing upload progress end, also IE8-9
    xhr.upload.addEventListener('load', function (e) {
      if (e.lengthComputable) {
        e.config = config;
        notifyProgress(getNotifyEvent(e));
      }
    }, false);
  };
};

Now i'm going to create new issue on ng-file-upload git

xeor commented 8 years ago

Nice find :+1:

danialfarid commented 8 years ago

@alex-training that's how the plugin listen to the progress event if you remove those lines file progress feature will be removed too

danialfarid commented 8 years ago

If you guys can create a small jsfiddle showing the error I can debug it and find out where the issue is. You can POST to this url to simulate 401 error: https://angular-file-upload-cors-srv.appspot.com/upload?errorCode=401&errorMessage=error

alex-training commented 8 years ago

@danialfarid Unfortunately I understand that the feature will be removed too...

OK, I will create jsfiddle project to reproduce this behavior

alex-training commented 8 years ago

@danialfarid @xeor @witoldsz

Guys please have a look at jsfiddle

https://jsfiddle.net/alexedu/4na60fpd/

You can see all errors in browser console

xeor commented 8 years ago

Thanks @alex-training for making the jsfiddle. I have nothing to add, this is the one :)

danialfarid commented 8 years ago

This is fixed at ng-file-upload#8.0.4, you can close this issue now.

xeor commented 8 years ago

Very nice! Thank you @danialfarid for the quick fix!

alex-training commented 8 years ago

Thank you @danialfarid