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

Context is empty if retrieved in RetrySpec since version 3.4.25 #3314

Closed
pascalgosselin opened this issue Dec 8, 2022 · 1 comment · Fixed by #3316
Closed

Context is empty if retrieved in RetrySpec since version 3.4.25 #3314

pascalgosselin opened this issue Dec 8, 2022 · 1 comment · Fixed by #3316
Labels
area/context This issue is related to the Context type/bug A general bug warn/regression A regression from a previous release

Comments

@pascalgosselin
Copy link

The following code was working with reactor-core 3.4.24, but it does not work anymore with version 3.4.25.
ReactiveSecurityContextHolder.getContext() now returns empty.

@Component
public class BusinessClass {

  private static final Logger LOGGER =
      LoggerFactory.getLogger(MethodHandles.lookup().lookupClass());
  private final ComponentClass componentClass;

  public BusinessClass(ComponentClass componentClass) {
    this.componentClass = componentClass;
  }

  public RetrySpec getRetrySpec() {
    return Retry.max(1)
        .doBeforeRetryAsync(
            retrySignal ->
                ReactiveSecurityContextHolder.getContext()
                    .doOnSuccess(
                        securityContext ->
                            LOGGER.info(
                                "The securityContext is null : " + Objects.isNull(securityContext)))
                    .flatMap(securityContext -> componentClass.doBeforeRetry()));
  }
}
@Component
public class ComponentClass {

  public Mono<String> getValue() {
    return Mono.just("String value");
  }

  public Mono<Void> doBeforeRetry() {
    return Mono.empty();
  }
}
@ExtendWith(MockitoExtension.class)
class BusinessClassTest {

  @Mock ComponentClass componentClass;
  @Mock SecurityContext securityContext;

  @InjectMocks BusinessClass businessClass;

  @Test
  @DisplayName("doBusinessLogic should call 'doBeforeRetry' before retry")
  void doBusinessLogic() {

    PublisherProbe<Void> doBeforeRetryProbe = PublisherProbe.empty();
    Context context = ReactiveSecurityContextHolder.withSecurityContext(Mono.just(securityContext));

    given(componentClass.getValue())
        .willReturn(Mono.error(new RuntimeException()))
        .willReturn(Mono.just("someValue"));
    given(componentClass.doBeforeRetry()).willReturn(doBeforeRetryProbe.mono());

    StepVerifier.create(
            Mono.defer(() -> componentClass.getValue())
                .retryWhen(businessClass.getRetrySpec())
                .contextWrite(context))
        .expectNext("someValue")
        .verifyComplete();

    doBeforeRetryProbe.assertWasSubscribed();
  }
}

We first faced the problem when we migrated from spring-boot 2.7.5 to 2.7.6. But it works with spring-boot 2.7.6 by simply downgrading to reactor-core 3.4.24

It could be this change: https://github.com/reactor/reactor-core/pull/3262/files#diff-a5639cea15f6765fecf7b44413831dc92cc0c8f45c1314ae8f4fb2959c60a623, but I'm really not sure.

Thanks in advance

Your Environment

  • Reactor version(s) used: succeed with reactor-core 3.4.24, failed with reactor-core 3.4.25
  • Other relevant libraries versions (eg. netty, ...): spring-boot 2.7.6
  • JVM version (java -version): 11 and 17
@reactorbot reactorbot added the ❓need-triage This issue needs triage, hasn't been looked at by a team member yet label Dec 8, 2022
@chemicL chemicL added type/bug A general bug warn/regression A regression from a previous release area/context This issue is related to the Context and removed ❓need-triage This issue needs triage, hasn't been looked at by a team member yet labels Dec 9, 2022
@chemicL
Copy link
Member

chemicL commented Dec 9, 2022

@pascalgosselin thank you for the report. We will investigate, it is possible it is a regression caused by precisely the change you point to. @OlegDokuka will most probably have a closer look.

chemicL pushed a commit that referenced this issue Mar 7, 2023
closes #3314

This commit restores context for inner Flux inside concatMap, while the concatMap remains without context. It allows Retry methods to have context while onErrorContinue to be disabled for retry concatMap
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/context This issue is related to the Context type/bug A general bug warn/regression A regression from a previous release
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants