Riptide 4 does not respect X-RateLimit-Reset header when used through riptide-spring-boot-autoconfigure

[sklv-max]

Description

Upon receiving a 429 response along with X-RateLimit headers a client should obey those headers.
For instance, if X-RateLimit-Remaining header has 0 as its value, then a client should wait X-RateLimit-Reset seconds before making another retry.

However, when riptide clients are configured through riptide-spring-boot-autoconfigure in riptide 4.1.0, those headers get ignored.
This was not the case in Riptide 3.x.x

Expected Behavior

X-RateLimit headers are respected

Actual Behavior

X-RateLimit headers are ignored

Possible Fix

I am not sure but one possible reason is this change.

Riptide autoconfiguration creates a composite delay function, consisting of two functions: RetryAfterDelayFunction and RateLimitResetDelayFunction. When Retry-After is missing and the X-RateLimit headers are present it should take result of the RateLimitResetDelayFunction (see here). But since RetryAfterDelayFunction does not return null anymore, its result is taken (Duration.ofMinutes(-1)). Because it is a negative interval, retry happens immediately.

However, this is just a guess.

Steps to Reproduce

The following test works in Riptide 3.4.0 (with JDK 8) but does not work in the current main branch (JDK 17)

package org.zalando.riptide.autoconfigure.retry;

import static org.hamcrest.MatcherAssert.assertThat;
import static org.hamcrest.Matchers.greaterThanOrEqualTo;
import static org.springframework.http.HttpStatus.Series.SUCCESSFUL;
import static org.springframework.test.web.client.ExpectedCount.times;
import static org.springframework.test.web.client.match.MockRestRequestMatchers.requestTo;
import static org.springframework.test.web.client.response.MockRestResponseCreators.withRawStatus;
import static org.springframework.test.web.client.response.MockRestResponseCreators.withSuccess;
import static org.zalando.riptide.Bindings.anySeries;
import static org.zalando.riptide.Bindings.on;
import static org.zalando.riptide.Navigators.series;
import static org.zalando.riptide.Navigators.status;
import static org.zalando.riptide.PassRoute.pass;
import static org.zalando.riptide.failsafe.RetryRoute.retry;

import com.google.common.base.Stopwatch;
import java.time.Duration;
import org.junit.jupiter.api.Test;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.beans.factory.annotation.Qualifier;
import org.springframework.boot.autoconfigure.ImportAutoConfiguration;
import org.springframework.boot.autoconfigure.jackson.JacksonAutoConfiguration;
import org.springframework.context.annotation.Configuration;
import org.springframework.http.HttpHeaders;
import org.springframework.http.HttpStatus;
import org.springframework.test.context.ActiveProfiles;
import org.springframework.test.web.client.MockRestServiceServer;
import org.zalando.logbook.autoconfigure.LogbookAutoConfiguration;
import org.zalando.riptide.Http;
import org.zalando.riptide.autoconfigure.MetricsTestAutoConfiguration;
import org.zalando.riptide.autoconfigure.OpenTracingTestAutoConfiguration;
import org.zalando.riptide.autoconfigure.RiptideClientTest;

@RiptideClientTest
@ActiveProfiles("default")
public class XRateLimitResetRetryTest {
  @Configuration
  @ImportAutoConfiguration({
      JacksonAutoConfiguration.class,
      LogbookAutoConfiguration.class,
      OpenTracingTestAutoConfiguration.class,
      MetricsTestAutoConfiguration.class,
  })
  static class ContextConfiguration {
  }

  @Autowired
  @Qualifier("retry-test")
  private Http retryClient;

  @Autowired
  private MockRestServiceServer server;

  @Test
  void shouldObeyXRateLimitHeader() {
    server.expect(times(1), requestTo("http://retry-test"))
        .andRespond(withRawStatus(429).headers(new HttpHeaders() {{
          add("X-RateLimit-Limit", "1");
          add("X-RateLimit-Remaining", "0");
          add("X-RateLimit-Reset", "2");
        }}));
    server.expect(times(1), requestTo("http://retry-test")).andRespond(withSuccess());

    final Stopwatch stopwatch = Stopwatch.createStarted();
    retryClient.get()
        .dispatch(series(),
            on(SUCCESSFUL).call(pass()),
            anySeries().dispatch(status(),
                on(HttpStatus.TOO_MANY_REQUESTS).call(retry())))
        .join();
    Duration elapsed = stopwatch.stop().elapsed();

    assertThat(elapsed, greaterThanOrEqualTo(Duration.ofSeconds(2)));
    server.verify();
  }
}

Output for the current main:

java.lang.AssertionError: 
Expected: a value equal to or greater than <PT2S>
     but: <PT0.084450105S> was less than <PT2S>

	at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:20)
	at org.hamcrest.MatcherAssert.assertThat(MatcherAssert.java:6)
	at org.zalando.riptide.autoconfigure.retry.XRateLimitResetRetryTest.shouldObeyXRateLimitHeader(XRateLimitResetRetryTest.java:74)
	at java.base/java.lang.reflect.Method.invoke(Method.java:568)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)
	at java.base/java.util.ArrayList.forEach(ArrayList.java:1511)

Context

One of our tests started to fail when we migrated to Riptide 4.1.0 (4.0.0 seems to have the same problem).