From 6e5592eb5ea2b71191b6f6bea7888f58b606ae79 Mon Sep 17 00:00:00 2001 From: Alex Woods Date: Wed, 12 Aug 2026 10:39:26 -0700 Subject: [PATCH] Add cache for nonRecoverableErrors to protect services from repeated calls --- .../auth/LoginCredentialsProviderTest.java | 52 +++ .../sso/auth/SsoCredentialsProviderTest.java | 74 +++++ .../auth/StsCredentialsProviderTestBase.java | 97 +++++- .../awssdk/utils/cache/CachedSupplier.java | 69 +++- .../utils/cache/CachedSupplierTest.java | 314 +++++++++++++++++- 5 files changed, 590 insertions(+), 16 deletions(-) diff --git a/services/signin/src/test/java/software/amazon/awssdk/services/signin/auth/LoginCredentialsProviderTest.java b/services/signin/src/test/java/software/amazon/awssdk/services/signin/auth/LoginCredentialsProviderTest.java index 7ef9e40638a5..643c92bfd5c4 100644 --- a/services/signin/src/test/java/software/amazon/awssdk/services/signin/auth/LoginCredentialsProviderTest.java +++ b/services/signin/src/test/java/software/amazon/awssdk/services/signin/auth/LoginCredentialsProviderTest.java @@ -314,6 +314,58 @@ public void resolveCredentials_whenCredentialsExpired_serviceCallFailsWithInsuff assertTrue(e.getMessage().contains("insufficient permissions")); } + @Test + public void resolveCredentials_tokenExpired_cachedBriefly_immediateRetryDoesNotCallService() { + // Expired credentials force a refresh on every call + AwsSessionCredentials creds = buildCredentials(Instant.now().minusSeconds(60)); + LoginAccessToken token = buildAccessToken(creds); + tokenManager.storeToken(token); + + // Service returns TOKEN_EXPIRED — non-recoverable + stubAccessDeniedException(OAuth2ErrorCode.TOKEN_EXPIRED); + + // First call: hits service, gets non-recoverable error — thrown and cached + assertThrows(AccessDeniedException.class, () -> loginCredentialsProvider.resolveCredentials()); + assertEquals(1, mockHttpClient.getRequests().size()); + + // Second call: immediate retry — should re-raise cached error without calling service + assertThrows(AccessDeniedException.class, () -> loginCredentialsProvider.resolveCredentials()); + assertEquals(1, mockHttpClient.getRequests().size()); // Still 1 — service NOT called again + } + + @Test + public void resolveCredentials_tokenMissing_cachedBriefly_immediateRetryDoesNotReadDiskAgain() { + // No token on disk — will throw InvalidTokenException (non-recoverable) + // This is a client-side error that doesn't call the service at all + + // First call: fails with missing token + assertThrows(SdkClientException.class, () -> loginCredentialsProvider.resolveCredentials()); + assertEquals(0, mockHttpClient.getRequests().size()); // Service never called + + // Second call: immediate retry — should re-raise cached error + assertThrows(SdkClientException.class, () -> loginCredentialsProvider.resolveCredentials()); + assertEquals(0, mockHttpClient.getRequests().size()); // Service still never called + } + + @Test + public void resolveCredentials_userCredentialsChanged_cachedBriefly_immediateRetryDoesNotCallService() { + // Expired credentials force a refresh + AwsSessionCredentials creds = buildCredentials(Instant.now().minusSeconds(60)); + LoginAccessToken token = buildAccessToken(creds); + tokenManager.storeToken(token); + + // Service returns USER_CREDENTIALS_CHANGED — non-recoverable + stubAccessDeniedException(OAuth2ErrorCode.USER_CREDENTIALS_CHANGED); + + // First call: hits service, gets non-recoverable error — thrown and cached + assertThrows(AccessDeniedException.class, () -> loginCredentialsProvider.resolveCredentials()); + assertEquals(1, mockHttpClient.getRequests().size()); + + // Second call: immediate retry — should re-raise cached error without calling service + assertThrows(AccessDeniedException.class, () -> loginCredentialsProvider.resolveCredentials()); + assertEquals(1, mockHttpClient.getRequests().size()); // Still 1 — service NOT called again + } + @Test public void resolveCredentials_tokenCacheMissingAfterSuccessfulCache_throwsAndBypassesStaticStability() throws Exception { // Build a provider without async updates so refresh is synchronous diff --git a/services/sso/src/test/java/software/amazon/awssdk/services/sso/auth/SsoCredentialsProviderTest.java b/services/sso/src/test/java/software/amazon/awssdk/services/sso/auth/SsoCredentialsProviderTest.java index 2c2da892b99d..73e6a4ca6741 100644 --- a/services/sso/src/test/java/software/amazon/awssdk/services/sso/auth/SsoCredentialsProviderTest.java +++ b/services/sso/src/test/java/software/amazon/awssdk/services/sso/auth/SsoCredentialsProviderTest.java @@ -218,6 +218,80 @@ public void expiredTokenException_bypassesStaticStability() { } } + @Test + public void unauthorizedException_cachedBriefly_immediateRetryDoesNotCallSso() { + ssoClient = mock(SsoClient.class); + RoleCredentials credentials = RoleCredentials.builder() + .accessKeyId("a") + .secretAccessKey("b") + .sessionToken("c") + .expiration(Instant.now().minus(Duration.ofSeconds(5)).toEpochMilli()) + .build(); + + Supplier supplier = getRequestSupplier(); + GetRoleCredentialsResponse response = getResponse(credentials); + + UnauthorizedException unauthorizedException = (UnauthorizedException) UnauthorizedException.builder() + .message("Token is expired") + .build(); + + // First call succeeds, second call fails with UnauthorizedException + when(ssoClient.getRoleCredentials(supplier.get())) + .thenReturn(response) + .thenThrow(unauthorizedException); + + try (SsoCredentialsProvider credentialsProvider = SsoCredentialsProvider.builder() + .refreshRequest(supplier) + .ssoClient(ssoClient) + .build()) { + // First call succeeds and caches credentials + credentialsProvider.resolveCredentials(); + + // Second call triggers refresh, hits non-recoverable UnauthorizedException — thrown and cached + assertThatThrownBy(credentialsProvider::resolveCredentials) + .isInstanceOf(UnauthorizedException.class); + + // Third call: immediate retry — should re-raise cached error without calling SSO + assertThatThrownBy(credentialsProvider::resolveCredentials) + .isInstanceOf(UnauthorizedException.class); + + // Verify SSO was called only twice: initial fetch + one failed refresh. + // The third resolveCredentials() re-raised the cached error without contacting SSO. + callClient(verify(ssoClient, times(2)), Mockito.any()); + } + } + + @Test + public void expiredTokenException_cachedBriefly_immediateRetryDoesNotCallSso() { + ssoClient = mock(SsoClient.class); + + ExpiredTokenException expiredTokenException = (ExpiredTokenException) ExpiredTokenException.builder() + .message("The SSO session associated with this profile has expired") + .build(); + + // Request supplier throws ExpiredTokenException (client-side token expiry) + Supplier expiredSupplier = () -> { + throw expiredTokenException; + }; + + try (SsoCredentialsProvider credentialsProvider = SsoCredentialsProvider.builder() + .refreshRequest(expiredSupplier) + .ssoClient(ssoClient) + .build()) { + // First call: initial fetch fails with non-recoverable error + assertThatThrownBy(credentialsProvider::resolveCredentials) + .isInstanceOf(ExpiredTokenException.class); + + // Second call: immediate retry — should re-raise cached error without calling SSO + assertThatThrownBy(credentialsProvider::resolveCredentials) + .isInstanceOf(ExpiredTokenException.class); + + // Since the ExpiredTokenException is thrown by the supplier (before reaching SSO), + // the SSO client should never have been called + callClient(verify(ssoClient, times(0)), Mockito.any()); + } + } + @Test public void noCachedCredentials_anyFailure_throwsImmediately() { ssoClient = mock(SsoClient.class); diff --git a/services/sts/src/test/java/software/amazon/awssdk/services/sts/auth/StsCredentialsProviderTestBase.java b/services/sts/src/test/java/software/amazon/awssdk/services/sts/auth/StsCredentialsProviderTestBase.java index 59957b6b1a9d..eedd598502b5 100644 --- a/services/sts/src/test/java/software/amazon/awssdk/services/sts/auth/StsCredentialsProviderTestBase.java +++ b/services/sts/src/test/java/software/amazon/awssdk/services/sts/auth/StsCredentialsProviderTestBase.java @@ -265,7 +265,102 @@ public void nonRecoverableError_wrappedInSdkClientException_throwsImmediately() } /** - * Verifies that recoverable errors (those with error codes NOT in the non-recoverable set) still + * Non-recoverable errors are cached for a short period (1-5 seconds) to protect the credential source + * from callers that catch the error and retry in a tight loop. An immediate retry after receiving a + * non-recoverable error should re-raise the cached error without contacting STS again. + */ + @Test + public void nonRecoverableError_cachedBriefly_immediateRetryDoesNotCallSts() { + Credentials validCredentials = Credentials.builder() + .accessKeyId("a") + .secretAccessKey("b") + .sessionToken("c") + .expiration(Instant.now().minus(Duration.ofSeconds(5))) + .build(); + RequestT request = getRequest(); + ResponseT response = getResponse(validCredentials); + + AwsServiceException accessDenied = AwsServiceException.builder() + .message("Access denied") + .awsErrorDetails(AwsErrorDetails.builder() + .errorCode("AccessDenied") + .errorMessage("User is not authorized") + .serviceName("STS") + .build()) + .statusCode(403) + .build(); + + // First call succeeds (caches expired credentials), second call fails with non-recoverable error + when(callClient(stsClient, request)) + .thenReturn(response) + .thenThrow(accessDenied); + + StsCredentialsProvider.BaseBuilder credentialsProviderBuilder = + createCredentialsProviderBuilder(request); + + try (StsCredentialsProvider credentialsProvider = credentialsProviderBuilder.stsClient(stsClient).build()) { + // First call succeeds and caches credentials + credentialsProvider.resolveCredentials(); + + // Second call triggers refresh, hits non-recoverable error — thrown and cached + assertThatThrownBy(credentialsProvider::resolveCredentials) + .isInstanceOf(AwsServiceException.class) + .satisfies(e -> assertThat(((AwsServiceException) e).awsErrorDetails().errorCode()) + .isEqualTo("AccessDenied")); + + // Third call: immediate retry — should re-raise cached error without calling STS + assertThatThrownBy(credentialsProvider::resolveCredentials) + .isInstanceOf(AwsServiceException.class) + .satisfies(e -> assertThat(((AwsServiceException) e).awsErrorDetails().errorCode()) + .isEqualTo("AccessDenied")); + + // Verify STS was called only twice: initial fetch + one failed refresh. + // The third resolveCredentials() re-raised the cached error without contacting STS. + callClient(verify(stsClient, times(2)), Mockito.any()); + } + } + + /** + * Non-recoverable errors are cached for a short period on the initial fetch path as well. + * When the very first STS call fails with a non-recoverable error and the caller retries immediately, + * the cached error is re-raised without contacting STS again. + */ + @Test + public void nonRecoverableError_initialFetch_cachedBriefly_immediateRetryDoesNotCallSts() { + RequestT request = getRequest(); + + AwsServiceException accessDenied = AwsServiceException.builder() + .message("Access denied") + .awsErrorDetails(AwsErrorDetails.builder() + .errorCode("AccessDenied") + .errorMessage("User is not authorized") + .serviceName("STS") + .build()) + .statusCode(403) + .build(); + + when(callClient(stsClient, request)) + .thenThrow(accessDenied); + + StsCredentialsProvider.BaseBuilder credentialsProviderBuilder = + createCredentialsProviderBuilder(request); + + try (StsCredentialsProvider credentialsProvider = credentialsProviderBuilder.stsClient(stsClient).build()) { + // First call: initial fetch fails with non-recoverable error + assertThatThrownBy(credentialsProvider::resolveCredentials) + .isInstanceOf(AwsServiceException.class); + + // Second call: immediate retry — should re-raise cached error without calling STS + assertThatThrownBy(credentialsProvider::resolveCredentials) + .isInstanceOf(AwsServiceException.class); + + // Verify STS was called only once — the second call used the cached error + callClient(verify(stsClient, times(1)), Mockito.any()); + } + } + + /** + * Recoverable errors (those with error codes NOT in the non-recoverable set) still * benefit from static stability — the provider returns cached credentials instead of throwing. * This is the complement to the non-recoverable error tests: a service unavailable or throttling * error should not propagate immediately. diff --git a/utils/src/main/java/software/amazon/awssdk/utils/cache/CachedSupplier.java b/utils/src/main/java/software/amazon/awssdk/utils/cache/CachedSupplier.java index 97a32e194eb9..0ccc3380ad2f 100644 --- a/utils/src/main/java/software/amazon/awssdk/utils/cache/CachedSupplier.java +++ b/utils/src/main/java/software/amazon/awssdk/utils/cache/CachedSupplier.java @@ -64,6 +64,16 @@ public class CachedSupplier implements Supplier, SdkAutoCloseable { */ private static final Duration STATIC_STABILITY_BACKOFF_MAX = Duration.ofMinutes(10); + /** + * Minimum cache duration for a non-recoverable error (inclusive). + */ + private static final int NON_RECOVERABLE_ERROR_CACHE_MIN_SECONDS = 1; + + /** + * Maximum cache duration for a non-recoverable error (inclusive). + */ + private static final int NON_RECOVERABLE_ERROR_CACHE_MAX_SECONDS = 5; + /** * Used as a primitive form of rate limiting for the speed of our refreshes. This will make sure that the backing supplier has @@ -128,6 +138,23 @@ public class CachedSupplier implements Supplier, SdkAutoCloseable { */ private volatile Instant nextAllowedRefreshTime; + /** + * The most recent non-recoverable error returned by the credential source. While {@link #cachedNonRecoverableErrorExpiresAt} + * is in the future, subsequent refresh attempts re-raise this error without contacting the source. This protects the + * credential source from callers that catch and retry in a tight loop. + * + *

Set under {@link #refreshLock}. Cleared on successful refresh. + */ + private volatile RuntimeException cachedNonRecoverableError; + + /** + * The expiration time for {@link #cachedNonRecoverableError}. After this instant, the next refresh attempt will contact + * the credential source again instead of re-raising the cached error. + * + *

Set under {@link #refreshLock}. Cleared on successful refresh. + */ + private volatile Instant cachedNonRecoverableErrorExpiresAt; + private CachedSupplier(Builder builder) { Validate.notNull(builder.supplier, "builder.supplier"); Validate.notNull(builder.prefetchJitterEnabled, "builder.prefetchJitterEnabled"); @@ -261,6 +288,31 @@ private boolean refreshRateLimited() { return nextAllowed != null && clock.instant().isBefore(nextAllowed); } + /** + * Returns {@code true} if a non-recoverable error is cached and has not yet expired. While this returns + * {@code true}, refresh attempts re-raise the cached error without contacting the credential source. + * + *

Must be called under {@link #refreshLock}. + */ + private boolean nonRecoverableErrorCached() { + RuntimeException error = this.cachedNonRecoverableError; + Instant expiresAt = this.cachedNonRecoverableErrorExpiresAt; + return error != null && expiresAt != null && clock.instant().isBefore(expiresAt); + } + + /** + * Caches a non-recoverable error for a short jittered duration (1-5 seconds). This prevents a caller that catches + * and retries in a loop from hammering the credential source with requests that are known to fail. + * + *

Must be called under {@link #refreshLock}. + */ + private void cacheNonRecoverableError(RuntimeException error, Instant now) { + this.cachedNonRecoverableError = error; + int cacheSeconds = NON_RECOVERABLE_ERROR_CACHE_MIN_SECONDS + + jitterRandom.nextInt(NON_RECOVERABLE_ERROR_CACHE_MAX_SECONDS - NON_RECOVERABLE_ERROR_CACHE_MIN_SECONDS + 1); + this.cachedNonRecoverableErrorExpiresAt = now.plusSeconds(cacheSeconds); + } + /** * Initiate a pre-fetch of the data using the configured {@link #prefetchStrategy}. */ @@ -280,6 +332,12 @@ private void refreshCache() { try { // Make sure the value was not refreshed while we waited for the lock. if (cacheIsStale() || shouldInitiateCachePrefetch()) { + // Check if a non-recoverable error is still cached. If so, re-raise it without + // contacting the credential source. + if (nonRecoverableErrorCached()) { + throw cachedNonRecoverableError; + } + log.debug(() -> "(" + cachedValueName + ") Refreshing cached value."); // It wasn't, call the supplier to update it. @@ -320,6 +378,8 @@ private RefreshResult handleFetchedSuccess(RefreshResult fetch) { if (now.isBefore(fetch.staleTime())) { this.nextAllowedRefreshTime = null; // Clear backoff gate on success + this.cachedNonRecoverableError = null; // Clear any cached non-recoverable error + this.cachedNonRecoverableErrorExpiresAt = null; return fetch; } @@ -366,6 +426,10 @@ private RefreshResult handleFetchFailure(RuntimeException e) { RefreshResult currentCachedValue = cachedValue; if (currentCachedValue == null) { + // No cached value. Cache the error if it's non-recoverable (protects the source on initial fetch loops). + if (nonRecoverableErrorPredicate != null && nonRecoverableErrorPredicate.test(e)) { + cacheNonRecoverableError(e, clock.instant()); + } throw e; } @@ -375,8 +439,10 @@ private RefreshResult handleFetchFailure(RuntimeException e) { case STRICT: throw e; case ALLOW: - // Non-recoverable errors bypass static stability + // Non-recoverable errors bypass static stability but are cached briefly + // to protect the credential source from tight retry loops. if (nonRecoverableErrorPredicate != null && nonRecoverableErrorPredicate.test(e)) { + cacheNonRecoverableError(e, now); throw e; } @@ -399,6 +465,7 @@ private RefreshResult handleFetchFailure(RuntimeException e) { // Not yet stale — we're in the prefetch window. Handle failure based on mode. if (staleValueBehavior == StaleValueBehavior.ALLOW) { if (nonRecoverableErrorPredicate != null && nonRecoverableErrorPredicate.test(e)) { + cacheNonRecoverableError(e, now); throw e; } diff --git a/utils/src/test/java/software/amazon/awssdk/utils/cache/CachedSupplierTest.java b/utils/src/test/java/software/amazon/awssdk/utils/cache/CachedSupplierTest.java index 5edb1cea8320..ec4c0760f71a 100644 --- a/utils/src/test/java/software/amazon/awssdk/utils/cache/CachedSupplierTest.java +++ b/utils/src/test/java/software/amazon/awssdk/utils/cache/CachedSupplierTest.java @@ -53,6 +53,26 @@ * Validate the functionality of {@link CachedSupplier}. */ public class CachedSupplierTest { + + // --- Constants mirroring CachedSupplier's internal values, for test readability --- + + /** Minimum static stability backoff duration in seconds (5 minutes). */ + private static final long BACKOFF_MIN_SECONDS = 300; + + /** Maximum static stability backoff duration in seconds (10 minutes). */ + private static final long BACKOFF_MAX_SECONDS = 600; + + /** Maximum duration (seconds) that a non-recoverable error stays cached. */ + private static final long NON_RECOVERABLE_ERROR_CACHE_MAX_SECONDS = 5; + + /** A duration safely past the non-recoverable error cache max, guaranteeing the cache has expired. */ + private static final long PAST_NON_RECOVERABLE_ERROR_CACHE = NON_RECOVERABLE_ERROR_CACHE_MAX_SECONDS + 1; + + /** A duration safely past the maximum backoff, guaranteeing the backoff has elapsed. */ + private static final long PAST_MAX_BACKOFF = BACKOFF_MAX_SECONDS + 1; + + /** Long prefetch time (seconds) used in tests where credentials have a 1-hour stale time and 5-minute advisory window. */ + private static final long LONG_PREFETCH_SECONDS = 300; /** * An executor for performing "get" on the cached supplier asynchronously. This, along with the {@link WaitingSupplier} allows * near-manual scheduling of threads so that we can test that the cache is only calling the underlying supplier when we want @@ -454,8 +474,8 @@ public void allowMode_backoffIsInExpectedRange() throws InterruptedException { // Now nextAllowedRefreshTime is set to now(61) + [300,600]s // The cached value should be returned while rate limited - Instant minBackoffEnd = now.plusSeconds(61 + 300); - Instant maxBackoffEnd = now.plusSeconds(61 + 600); + Instant minBackoffEnd = now.plusSeconds(61 + BACKOFF_MIN_SECONDS); + Instant maxBackoffEnd = now.plusSeconds(61 + BACKOFF_MAX_SECONDS); // Advance just before the minimum backoff end - should still be rate limited clock.time = minBackoffEnd.minusSeconds(1); @@ -540,7 +560,7 @@ public void allowMode_prefetchWindowFailure_preservesStaleTime() { // Advance past the maximum possible backoff (61 + 600 = 661s from now) but still before stale time (3600s). // The nextAllowedRefreshTime backoff will have elapsed, so a prefetch refresh will be attempted. - clock.time = now.plusSeconds(700); + clock.time = now.plusSeconds(61 + PAST_MAX_BACKOFF); supplier.set(RefreshResult.builder("refreshed-creds") .staleTime(Instant.MAX) .prefetchTime(Instant.MAX) @@ -966,7 +986,7 @@ public void invalidate_doesNotBypassRefreshBackoff() { assertThat(cache.get()).isEqualTo("old"); // Past backoff — returns fresh - clock.time = now.plusSeconds(700); + clock.time = now.plusSeconds(62 + PAST_MAX_BACKOFF); assertThat(cache.get()).isEqualTo("new"); } } @@ -1037,7 +1057,7 @@ public void allowMode_staleCredentialsFromSource_advisoryWindow_retainsCachedAnd assertThat(cachedSupplier.get()).isEqualTo("original-creds"); // Advance into advisory window (past prefetch but before stale) - clock.time = now.plusSeconds(301); + clock.time = now.plusSeconds(LONG_PREFETCH_SECONDS + 1); // Source returns credentials with staleTime in the past (already expired) supplier.set(RefreshResult.builder("stale-creds") @@ -1049,7 +1069,7 @@ public void allowMode_staleCredentialsFromSource_advisoryWindow_retainsCachedAnd assertThat(cachedSupplier.get()).isEqualTo("original-creds"); // Verify backoff was applied: a subsequent call should still return cached without contacting source - clock.time = now.plusSeconds(302); + clock.time = now.plusSeconds(LONG_PREFETCH_SECONDS + 2); supplier.set(RefreshResult.builder("should-not-reach") .staleTime(Instant.MAX) .prefetchTime(Instant.MAX) @@ -1057,7 +1077,7 @@ public void allowMode_staleCredentialsFromSource_advisoryWindow_retainsCachedAnd assertThat(cachedSupplier.get()).isEqualTo("original-creds"); // Advance past max backoff (600s from the stale response time) - clock.time = now.plusSeconds(301 + 601); + clock.time = now.plusSeconds(LONG_PREFETCH_SECONDS + 1 + PAST_MAX_BACKOFF); assertThat(cachedSupplier.get()).isEqualTo("should-not-reach"); } } @@ -1102,7 +1122,7 @@ public void allowMode_staleCredentialsFromSource_mandatoryWindow_retainsCachedAn assertThat(cachedSupplier.get()).isEqualTo("original-creds"); // Advance past max backoff (600s from the stale response time) - clock.time = now.plusSeconds(61 + 601); + clock.time = now.plusSeconds(61 + PAST_MAX_BACKOFF); assertThat(cachedSupplier.get()).isEqualTo("fresh-creds"); } } @@ -1131,19 +1151,19 @@ public void allowMode_nonRecoverableError_noBackoff_nextCallContactsSource() { assertThat(cachedSupplier.get()).isEqualTo("cached-creds"); // Advance past prefetch time (advisory window) - clock.time = now.plusSeconds(301); + clock.time = now.plusSeconds(LONG_PREFETCH_SECONDS + 1); supplier.set(new CacheInvalidatingRuntimeException("non-recoverable")); // Non-recoverable error is thrown assertThatThrownBy(cachedSupplier::get).isInstanceOf(CacheInvalidatingRuntimeException.class); - // Immediately call again — no backoff should be applied, source should be contacted - clock.time = now.plusSeconds(302); + // Advance past the non-recoverable error cache window (max 5 seconds) — source should be contacted + clock.time = now.plusSeconds(LONG_PREFETCH_SECONDS + 1 + PAST_NON_RECOVERABLE_ERROR_CACHE); supplier.set(RefreshResult.builder("refreshed-creds") .staleTime(now.plusSeconds(7200)) .prefetchTime(now.plusSeconds(5400)) .build()); - // If backoff were applied, this would return "cached-creds"; instead it contacts the source + // No 5-10 minute backoff was applied; after the short error cache expires, source is contacted assertThat(cachedSupplier.get()).isEqualTo("refreshed-creds"); } } @@ -1176,8 +1196,8 @@ public void allowMode_nonRecoverableError_mandatoryWindow_noBackoff_nextCallCont // Non-recoverable error is thrown assertThatThrownBy(cachedSupplier::get).isInstanceOf(CacheInvalidatingRuntimeException.class); - // Immediately call again — no backoff should be applied, source should be contacted - clock.time = now.plusSeconds(62); + // Advance past the non-recoverable error cache window (max 5 seconds) — source should be contacted + clock.time = now.plusSeconds(61 + PAST_NON_RECOVERABLE_ERROR_CACHE); supplier.set(RefreshResult.builder("refreshed-creds") .staleTime(now.plusSeconds(7200)) .prefetchTime(now.plusSeconds(5400)) @@ -1186,6 +1206,272 @@ public void allowMode_nonRecoverableError_mandatoryWindow_noBackoff_nextCallCont } } + // --- non-recoverable error caching tests --- + + @Test + public void allowMode_nonRecoverableErrorCached_withinCacheWindow_reRaisesWithoutCallingSource() { + AdjustableClock clock = new AdjustableClock(); + AtomicInteger supplierCallCount = new AtomicInteger(0); + Instant now = Instant.parse("2024-01-01T00:00:00Z"); + clock.time = now; + + CacheInvalidatingRuntimeException nonRecoverableError = new CacheInvalidatingRuntimeException("token expired"); + + Supplier> countingSupplier = () -> { + supplierCallCount.incrementAndGet(); + throw nonRecoverableError; + }; + + try (CachedSupplier cachedSupplier = CachedSupplier.builder(countingSupplier) + .staleValueBehavior(ALLOW) + .nonRecoverableErrorPredicate( + e -> e instanceof CacheInvalidatingRuntimeException) + .clock(clock) + .prefetchJitterEnabled(false) + .build()) { + // First call fails — initial fetch, no cached value + assertThatThrownBy(cachedSupplier::get).isEqualTo(nonRecoverableError); + assertThat(supplierCallCount.get()).isEqualTo(1); + + // Second call within the cache window (< 5 seconds) — should re-raise without calling source + clock.time = now.plusSeconds(1); + assertThatThrownBy(cachedSupplier::get).isEqualTo(nonRecoverableError); + assertThat(supplierCallCount.get()).isEqualTo(1); // Still 1 — source was NOT contacted + } + } + + @Test + public void allowMode_nonRecoverableErrorCached_afterCacheExpires_contactsSourceAgain() { + AdjustableClock clock = new AdjustableClock(); + AtomicInteger supplierCallCount = new AtomicInteger(0); + Instant now = Instant.parse("2024-01-01T00:00:00Z"); + clock.time = now; + + CacheInvalidatingRuntimeException nonRecoverableError = new CacheInvalidatingRuntimeException("token expired"); + + Supplier> countingSupplier = () -> { + supplierCallCount.incrementAndGet(); + throw nonRecoverableError; + }; + + try (CachedSupplier cachedSupplier = CachedSupplier.builder(countingSupplier) + .staleValueBehavior(ALLOW) + .nonRecoverableErrorPredicate( + e -> e instanceof CacheInvalidatingRuntimeException) + .clock(clock) + .prefetchJitterEnabled(false) + .build()) { + // First call fails + assertThatThrownBy(cachedSupplier::get).isEqualTo(nonRecoverableError); + assertThat(supplierCallCount.get()).isEqualTo(1); + + // Advance past the maximum cache window (5 seconds) — source should be contacted again + clock.time = now.plusSeconds(PAST_NON_RECOVERABLE_ERROR_CACHE); + assertThatThrownBy(cachedSupplier::get).isEqualTo(nonRecoverableError); + assertThat(supplierCallCount.get()).isEqualTo(2); // Source WAS contacted + } + } + + @Test + public void allowMode_nonRecoverableErrorCached_successfulRefreshClearsCache() { + AdjustableClock clock = new AdjustableClock(); + MutableSupplier supplier = new MutableSupplier(); + Instant now = Instant.parse("2024-01-01T00:00:00Z"); + clock.time = now; + + try (CachedSupplier cachedSupplier = CachedSupplier.builder(supplier) + .staleValueBehavior(ALLOW) + .nonRecoverableErrorPredicate( + e -> e instanceof CacheInvalidatingRuntimeException) + .clock(clock) + .prefetchJitterEnabled(false) + .build()) { + // Initial fetch fails with non-recoverable error + supplier.set(new CacheInvalidatingRuntimeException("token expired")); + assertThatThrownBy(cachedSupplier::get).isInstanceOf(CacheInvalidatingRuntimeException.class); + + // Advance past the cache window and fix the underlying issue (source now returns credentials) + clock.time = now.plusSeconds(PAST_NON_RECOVERABLE_ERROR_CACHE); + supplier.set(RefreshResult.builder("fresh-creds") + .staleTime(now.plusSeconds(3600)) + .prefetchTime(now.plusSeconds(300)) + .build()); + assertThat(cachedSupplier.get()).isEqualTo("fresh-creds"); + + // Advance into the prefetch window — trigger another non-recoverable, then verify it clears on success + clock.time = now.plusSeconds(LONG_PREFETCH_SECONDS + 1); + supplier.set(new CacheInvalidatingRuntimeException("token expired again")); + assertThatThrownBy(cachedSupplier::get).isInstanceOf(CacheInvalidatingRuntimeException.class); + + // Advance past cache window, fix again + clock.time = now.plusSeconds(LONG_PREFETCH_SECONDS + 1 + PAST_NON_RECOVERABLE_ERROR_CACHE); + supplier.set(RefreshResult.builder("newer-creds") + .staleTime(now.plusSeconds(7200)) + .prefetchTime(now.plusSeconds(5400)) + .build()); + assertThat(cachedSupplier.get()).isEqualTo("newer-creds"); + } + } + + @Test + public void allowMode_nonRecoverableErrorCached_staleWindow_reRaisesWithoutCallingSource() { + AdjustableClock clock = new AdjustableClock(); + AtomicInteger supplierCallCount = new AtomicInteger(0); + Instant now = Instant.parse("2024-01-01T00:00:00Z"); + clock.time = now; + + MutableSupplier supplier = new MutableSupplier(); + + try (CachedSupplier cachedSupplier = CachedSupplier.builder(supplier) + .staleValueBehavior(ALLOW) + .nonRecoverableErrorPredicate( + e -> e instanceof CacheInvalidatingRuntimeException) + .clock(clock) + .prefetchJitterEnabled(false) + .build()) { + // Initial successful fetch + supplier.set(RefreshResult.builder("cached-creds") + .staleTime(now.plusSeconds(60)) + .prefetchTime(now.plusSeconds(30)) + .build()); + assertThat(cachedSupplier.get()).isEqualTo("cached-creds"); + + // Advance past stale time (mandatory window) + clock.time = now.plusSeconds(61); + CacheInvalidatingRuntimeException error = new CacheInvalidatingRuntimeException("token expired"); + supplier.set(error); + + // First failure — thrown and cached + assertThatThrownBy(cachedSupplier::get).isEqualTo(error); + + // Immediately retry (within cache window) — should re-raise the same error without calling source + clock.time = now.plusSeconds(62); + // Swap supplier to something that would succeed — if called, we'd get "new-creds" not an exception + supplier.set(RefreshResult.builder("new-creds") + .staleTime(Instant.MAX) + .prefetchTime(Instant.MAX) + .build()); + assertThatThrownBy(cachedSupplier::get).isEqualTo(error); + } + } + + @Test + public void allowMode_nonRecoverableErrorCached_prefetchWindow_reRaisesWithoutCallingSource() { + AdjustableClock clock = new AdjustableClock(); + MutableSupplier supplier = new MutableSupplier(); + Instant now = Instant.parse("2024-01-01T00:00:00Z"); + clock.time = now; + + try (CachedSupplier cachedSupplier = CachedSupplier.builder(supplier) + .staleValueBehavior(ALLOW) + .nonRecoverableErrorPredicate( + e -> e instanceof CacheInvalidatingRuntimeException) + .clock(clock) + .prefetchJitterEnabled(false) + .build()) { + // Initial successful fetch + supplier.set(RefreshResult.builder("cached-creds") + .staleTime(now.plusSeconds(3600)) + .prefetchTime(now.plusSeconds(60)) + .build()); + assertThat(cachedSupplier.get()).isEqualTo("cached-creds"); + + // Advance into prefetch window + clock.time = now.plusSeconds(61); + CacheInvalidatingRuntimeException error = new CacheInvalidatingRuntimeException("token expired"); + supplier.set(error); + + // First call — non-recoverable error thrown and cached + assertThatThrownBy(cachedSupplier::get).isEqualTo(error); + + // Immediately retry (within cache window) — should re-raise without calling source + clock.time = now.plusSeconds(62); + supplier.set(RefreshResult.builder("new-creds") + .staleTime(Instant.MAX) + .prefetchTime(Instant.MAX) + .build()); + assertThatThrownBy(cachedSupplier::get).isEqualTo(error); + } + } + + @Test + public void allowMode_nonRecoverableErrorCached_cacheWindowIsJitteredBetween1And5Seconds() { + // Run many iterations to verify the cache window is within [1, 5] seconds + for (int i = 0; i < 100; i++) { + AdjustableClock clock = new AdjustableClock(); + Instant now = Instant.parse("2024-01-01T00:00:00Z"); + clock.time = now; + + CacheInvalidatingRuntimeException error = new CacheInvalidatingRuntimeException("expired"); + AtomicInteger callCount = new AtomicInteger(0); + + try (CachedSupplier cachedSupplier = CachedSupplier.builder(() -> { + callCount.incrementAndGet(); + throw error; + }) + .staleValueBehavior(ALLOW) + .nonRecoverableErrorPredicate( + e -> e instanceof CacheInvalidatingRuntimeException) + .clock(clock) + .prefetchJitterEnabled(false) + .build()) { + // First call caches the error + assertThatThrownBy(cachedSupplier::get).isEqualTo(error); + assertThat(callCount.get()).isEqualTo(1); + + // At 0.9s — should still be cached (cache min is 1s) + clock.time = now.plusMillis(900); + assertThatThrownBy(cachedSupplier::get).isEqualTo(error); + assertThat(callCount.get()).isEqualTo(1); + + // At PAST_NON_RECOVERABLE_ERROR_CACHE — cache must have expired (cache max is 5s) + clock.time = now.plusSeconds(PAST_NON_RECOVERABLE_ERROR_CACHE); + assertThatThrownBy(cachedSupplier::get).isEqualTo(error); + assertThat(callCount.get()).isEqualTo(2); // Source was contacted again + } + } + } + + @Test + public void allowMode_nonRecoverableErrorCached_recoverableErrorDoesNotUseErrorCache() { + AdjustableClock clock = new AdjustableClock(); + AtomicInteger supplierCallCount = new AtomicInteger(0); + Instant now = Instant.parse("2024-01-01T00:00:00Z"); + clock.time = now; + + MutableSupplier supplier = new MutableSupplier(); + + try (CachedSupplier cachedSupplier = CachedSupplier.builder(supplier) + .staleValueBehavior(ALLOW) + .nonRecoverableErrorPredicate( + e -> e instanceof CacheInvalidatingRuntimeException) + .clock(clock) + .prefetchJitterEnabled(false) + .build()) { + // Initial successful fetch + supplier.set(RefreshResult.builder("cached-creds") + .staleTime(now.plusSeconds(60)) + .prefetchTime(now.plusSeconds(30)) + .build()); + assertThat(cachedSupplier.get()).isEqualTo("cached-creds"); + + // Advance past stale time and fail with a RECOVERABLE error + clock.time = now.plusSeconds(61); + supplier.set(new RuntimeException("service unavailable")); + assertThat(cachedSupplier.get()).isEqualTo("cached-creds"); // Static stability applies + + // Verify the error is NOT cached as non-recoverable (no cachedNonRecoverableError set) + // The rate-limiting backoff (nextAllowedRefreshTime) is applied instead. + // Advance 1 second — still within the 5-10min backoff, returns cached + clock.time = now.plusSeconds(62); + supplier.set(RefreshResult.builder("should-not-get") + .staleTime(Instant.MAX) + .prefetchTime(Instant.MAX) + .build()); + assertThat(cachedSupplier.get()).isEqualTo("cached-creds"); // Rate limited, not error-cached + } + } + // --- integrated advisory window recomputation test --- @Test