Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Requests that error remain in cache and cannot be cleared #9

Closed
jmannau opened this issue Apr 29, 2020 · 3 comments · Fixed by #10
Closed

Requests that error remain in cache and cannot be cleared #9

jmannau opened this issue Apr 29, 2020 · 3 comments · Fixed by #10

Comments

@jmannau
Copy link
Contributor

jmannau commented Apr 29, 2020

I'm submitting a...


[ ] Regression (a behavior that used to work and stopped working in a new release)
[x] Bug report  
[ ] Performance issue
[ ] Feature request
[ ] Documentation issue or request
[ ] Support request
[ ] Other... Please describe:

Current behavior

If an http request errors (any 400/500 response), the request remains in the queue and cannot be cleared but emptying the cache.

Expected behavior

And non successful response should not be cached and should be cleared from the queue.

Minimal reproduction of the problem with instructions

  1. Issue a http request that will error. ie:
this.http.get('http://notarealurl.test', withCache())
  1. Check the httpCacheManage queue. The request remains in the queue. This means if the request is retried the original request is replayed.

What is the motivation / use case for changing the behavior?

For instance, when requesting an endpoint that is protected by authorisation, the first user may not be able to access the resource (returning 403). This user logs out and another user logs in. The request is retried, but the original request is issued including the Authorization header from user 1, not the new request with the new Authorization header.

Suggested Fix

I suggest changing the interceptor like this (https://github.com/ngneat/cashew/blob/master/projects/ngneat/cashew/src/lib/httpCacheInterceptor.ts#L37):

tap({
    next: event => {
          if (event instanceof HttpResponse) {
            const cache = this.httpCacheManager._resolveResponse(event);
            this.httpCacheManager._set(key, cache, +ttl);
           }
    },
    // Delete the item from the queue when the request completes
    // This will ensure it is not replayed if the request is cancelled or it errors
    complete: () => {
        queue.delete(key);
    }
}),

Environment


Angular version: 9.1.3
@ngneat/cashew 1.1.3 


Browser:
- [x] Chrome (desktop) version 81
- [ ] Chrome (Android) version XX
- [ ] Chrome (iOS) version XX
- [ ] Firefox version XX
- [ ] Safari (desktop) version XX
- [ ] Safari (iOS) version XX
- [ ] IE version XX
- [ ] Edge version XX
 
For Tooling issues:
- Node version: 12 
- Platform:  Mac

Others:

@NetanelBasal
Copy link
Member

Sure, do you want to create a PR?

@jmannau
Copy link
Contributor Author

jmannau commented Apr 29, 2020 via email

jmannau added a commit to jmannau/cashew that referenced this issue Apr 29, 2020
@jmannau
Copy link
Contributor Author

jmannau commented Apr 29, 2020

I have opened a PR #10 to fix this. I hope it is acceptable. Thanks for your work.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants