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

LocaleInterceptor blocks new request #819

Closed
atarix83 opened this issue Jul 24, 2020 · 4 comments · Fixed by #857
Closed

LocaleInterceptor blocks new request #819

atarix83 opened this issue Jul 24, 2020 · 4 comments · Fixed by #857
Assignees
Labels
bug e/2 Estimate in hours performance / caching Related to performance, caching or embedded objects
Milestone

Comments

@atarix83
Copy link
Contributor

Describe the bug
The new LocaleInterceptor blocks every new rest request when it can't retrieve the logged eperson object because the related entry cache object is expired. This is made in the LocaleService and was due a bug reported in this issue. The severe consequence is that the application is blocked after this error occurs.
A temporary workaround, until cache issuea are fixed, is to disabled that part of the code

@atarix83 atarix83 added bug needs triage New issue needs triage and/or scheduling labels Jul 24, 2020
@atarix83 atarix83 added this to To Do in DSpace 7 Beta 4 via automation Jul 24, 2020
@atarix83 atarix83 added this to the 7.0beta4 milestone Jul 24, 2020
@tdonohue tdonohue removed the needs triage New issue needs triage and/or scheduling label Jul 24, 2020
@tdonohue
Copy link
Member

Thanks for reporting this @atarix83 ! This does sound like an obvious bug we need fixing, and also sounds related to #739. For now I've added to the beta4 Todo list.

@artlowel : Any thoughts on whether we should solve this issue separately? Or would you rather we bundle this as part of your current work on #739?

@tdonohue tdonohue added the performance / caching Related to performance, caching or embedded objects label Jul 24, 2020
@artlowel
Copy link
Member

The way @atarix83 describes it, it should go away if the caching issue is fixed. But in general, blocking the entire app because we can't get the eperson language seems problematic.

It would probably be good to have a fix on the side of the interceptor as well: where if the eperson is undefined, we don't include the eperson language in the request, rather than blocking the request.

@atarix83
Copy link
Contributor Author

@artlowel I agree, but the issue is on getAuthenticatedUserFromStore method that doesn't emit a value due to getAllSucceededRemoteDataPayload operator. I made some test but I can't find a way to allow the observable to emit an eperson on success and undefined on error. I tried to replace getAllSucceededRemoteDataPayload with getFinishedRemoteData but it seems that even if the request is successful the RemoteData is emitted two time, the first time with an undefined payload, the second one with the eperson object. I encountered this bug in the past, but in this specific case it does not allow to have a working solution.
Do you have any idea?

@artlowel
Copy link
Member

@atarix83 after looking at it closer, I don't see a way to deal with the fact that the first payload will always be undefined either. Perhaps just leave out the eperson language until the cache is fixed.

@tdonohue tdonohue added Estimate TBD e/2 Estimate in hours and removed Estimate TBD labels Aug 25, 2020
@tdonohue tdonohue moved this from To Do to In Progress in DSpace 7 Beta 4 Aug 27, 2020
@atarix83 atarix83 mentioned this issue Sep 3, 2020
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug e/2 Estimate in hours performance / caching Related to performance, caching or embedded objects
Projects
No open projects
DSpace 7 Beta 4
  
In Progress
Development

Successfully merging a pull request may close this issue.

3 participants