gund / ng-http-interceptor

Http Interceptor library for Angular
MIT License
104 stars 16 forks source link

Timeout interceptor #131

Closed jogelin closed 7 years ago

jogelin commented 7 years ago

I would like to add a timeout configuration on the request. If I extends the Http service, it's easy to add a timeout configuration on the Observable returned by the request method.

But I would like to use your library and on the request, we don't have any Observable on the request interceptors...is there any other way ?

Thx

gund commented 7 years ago

Hi, thanks for the question.

In request interceptor you do not have Observable but you can return Observable of data:

httpInterceptorService.request().addInterceptor(data =>
  Observable.of(data).delay(1000));
jogelin commented 7 years ago

Hi, sorry just misunderstandings on my side. I need clarifications, I would like to put a timeout on the request and be able to catch the error.

My interceptor

httpInterceptor.request(/.*\/api\/.*/).addInterceptor(data =>
  Observable.of(data).timeoutWith(1000, Observable.throw(new Error('Timeout')))
);

And if I do

this._http.get('...')
    .catch(error => {
        console.log(error);
        return Observable.of([]);
     });

But if I put the timeout with the request interceptor, the catch is not called. if I put the timeout on the response, it's called after the response is received...

gund commented 7 years ago

Ok, I see now what you mean.

The problem is that Observable chain from request interceptors is replaced by http request so there is no real connection between them and your timeout simply not applied on final Observable.

Also I'm not sure if it's ever possible to apply timeoutWith operator before actual http request issued.

I would say that it in theory should work if you set it in response interceptor, but you said it's called after response received... That is something I need to investigate, particularly how timeoutWhen works and why it's waiting on http response.

Despite of that can you please provide a plunkr so I can have a look in your particular case?

Thanks

jogelin commented 7 years ago

Have a look : plunker

I would like to simulate the timeout but it's never thrown

jogelin commented 7 years ago

this is working :

@Injectable()
export class HttpInterceptorService extends Http {

    constructor(backend: ConnectionBackend, defaultOptions: RequestOptions) {
        super(backend, defaultOptions);
    }

    request(url: string | Request, options?: RequestOptionsArgs): Observable<Response> {
        return super.request(url, options).timeoutWith(1000, Observable.throw(new Error('TIMEOUT')));
    }
}
gund commented 7 years ago

Okay, so actually you can make it work. As I was thinking the key lies within how timeout operator works - it measures time between events, but when you apply it on http stream it does not fire event before request but only when it resolves so timeout just does not start.

What you need is initial event to occur on your stream just before sending an http request so that timer can start and wait for response.

See implementation here: http://plnkr.co/edit/1U7qHM1UgAsZJPcwxfL8?P=preview

gund commented 7 years ago

More info about timeout operator: http://reactivex.io/documentation/operators/timeout.html

jogelin commented 7 years ago

Ok thank you very much for your help. It's works in this case but....it doesn't work in real case....If you remove the delay and you simulate a real delay using chrome tools network, the error is not thrown.

I think because the response interceptor with the timeout config is called only when the response is received true ? We have : 1) Before request 2) do request 3) receive response and set timeout on Observable

We should have something like : 1) Before request -> set timeout on Observable 2) do request 3) receive response and the timeout is checked

jogelin commented 7 years ago

It's is not possible to overwrite the Observable in the request interceptor ?

gund commented 7 years ago

Okay I see your point.

So currently it's not possible by design because response interceptors chain is executed once http response received (I don't remember now why I did it exactly this way (: ).

So I will have a look again into chain calls and evaluate if it is possible to call them before request resolves.

Otherwise I can add something like beforeResponse() which will be called immediately on the request Observable.

jogelin commented 7 years ago

Ok, I'll do it in another way ;-)

I think it could be good that the request allows to return an Observable which is the same way as the Http service request method. Or provide it in parameter.

Anyway, thx for your time !

gund commented 7 years ago

Well I actually was able to change the way interceptors called (right after HTTP request) so that stuff like timeout can work as expected and all unit tests as well as e2e tests just passed.

Do not know why I did not do like that in first place and thank you for pointing me with this use case =) Now will deploy new fixed version expected in v2.0.3.

Thanks

gund commented 7 years ago

Fix landed in v.2.0.3 and 3.0.0-beta.3