DuplicatedContext is replaced by EventLoopContext
runtarinn opened this issue · 4 comments
Version
4.3.0 - 4.3.3
Context
The DuplicateContext that is bound to a HTTP request is replaced by EventLoopContext when calling another service using OAuth2WebClient with client_credentials grant and access_token is expired.
So everything works fine until the token expires, then the request that will trigger re-authentication (request new access_token using client_credentials) will loose it's DuplicatedContext and there for everything that has been put into the ContextData is lost.
The problem we are hitting is exactly here:
https://github.com/vert-x3/vertx-auth/blob/c097a5cad0b4b3eeca88e944e86b5153a124fcf2/vertx-auth-oauth2/src/main/java/io/vertx/ext/auth/oauth2/impl/OAuth2AuthProviderImpl.java#L494
If no refresh_token is found the future is failed using an old context that OAuth2AuthProviderImpl keeps as member. Should this not simply use Future.failed(...) ?
Do you have a reproducer?
https://github.com/runtarinn/duplicate-context-reproducer
Steps to reproduce
./gradlew test
Hope this is enough details provided
/Rúnar
Any change this can be looked into soon? Anything more I can provide to speed up the process? I don't feel comfortable submitting a PR since it's hard to understand what the problem was before the commit that broke this. The commit message ("Less missed contexts") that broke this functionality sounds like there was another problem with the contexts.
@runtarinn you're right. There were already context related issues initially, I believe the mentioned commit, was blindly, forcing a "context" (not always the right one) on all asynchronous operations.
I think the right approach is to review the commit and all operations that do not require an asynchronous call like you mention, should not use the ctx.XYZFuture(... call but use the Future.failed/sucess(... call. This way we don't force a change in the context.
@pmlopes I created a PR that will at least address this issue (OAuth2). I hope this does not break anything.
I think this issue has been fixed. If not please re-open. and we can work on a new PR.