Skip to content

feat: enhance RateLimitException to return extra information #426

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

feat: enhance RateLimitException to return extra information #426

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 22 additions & 0 deletions ...xt/src/main/java/com/giffing/bucket4j/spring/boot/starter/context/RateLimitException.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,4 +5,26 @@
* {@link RateLimiting} annotation.
*/
public class RateLimitException extends RuntimeException {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use Lomboks @Getter

private final long retryAfterNanoSeconds;
private final long remainingTokens;
private final String configurationName;

public RateLimitException(Long retryAfterNanoSeconds, Long remainingTokens, String configurationName) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Isn't the cacheKey also of interest?

super("Rate limit exceeded for configuration: " + configurationName + ". Retry after: " + (retryAfterNanoSeconds != null ? retryAfterNanoSeconds : 0) + "ns. Remaining tokens: " + (remainingTokens != null ? remainingTokens : 0));
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not quite sure about it but can the remainingTokens be greater than 0?
Nanoseconds are hard to read. What about milliseconds? Or seconds?

The message line is a little bit to long. my suggestion:

super("Rate limit exceeded for configuration: " + configurationName + ". " +
                "Retry after: " + toMilliseconds(retryAfterNanoSeconds) + "ns. " +
                "Remaining tokens: " + toMilliseconds(remainingTokens));
// the slower variant
super("Rate limit exceeded for configuration %s : . Retry after: %sms . Remaining tokens: %s".formatted(
                configurationName,
                toMilliseconds(retryAfterNanoSeconds),
                toMilliseconds(remainingTokens))
        );
private static long toMilliseconds(Long retryAfterNanoSeconds) {
        return retryAfterNanoSeconds != null ? TimeUnit.NANOSECONDS.toMillis(retryAfterNanoSeconds) : 0;
    }

this.retryAfterNanoSeconds = retryAfterNanoSeconds != null ? retryAfterNanoSeconds : 0;
this.remainingTokens = remainingTokens != null ? remainingTokens : 0;
this.configurationName = configurationName;
}

public long getRetryAfterNanoSeconds() {
return retryAfterNanoSeconds;
}

public long getRemainingTokens() {
return remainingTokens;
}

public String getConfigurationName() {
return configurationName;
}
}
20 changes: 14 additions & 6 deletions ...src/main/java/com/giffing/bucket4j/spring/boot/starter/config/aspect/RateLimitAspect.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@
import com.giffing.bucket4j.spring.boot.starter.context.properties.RateLimit;
import com.giffing.bucket4j.spring.boot.starter.service.RateLimitService;
import com.giffing.bucket4j.spring.boot.starter.utils.RateLimitAopUtils;
import com.giffing.bucket4j.spring.boot.starter.context.RateLimitException;
import jakarta.annotation.PostConstruct;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
Expand Down Expand Up @@ -125,7 +126,11 @@ public Object processMethodsAnnotatedWithRateLimitAnnotation(ProceedingJoinPoint
} else if (fallbackMethod != null) {
return fallbackMethod.invoke(joinPoint.getTarget(), joinPoint.getArgs());
} else {
throw new RateLimitException();
throw new RateLimitException(
consumedResult.retryAfterNanoSeconds,
consumedResult.remainingLimit,
rateLimitAnnotation.name()
);
}

return methodResult;
Expand All @@ -144,25 +149,28 @@ private static void performPostRateLimit(RateLimitService.RateLimitConfigresult<
private static RateLimitConsumedResult performRateLimit(RateLimitService.RateLimitConfigresult<Method, Object> rateLimitConfigResult, Method method, Map<String, Object> params, RateLimit annotationRateLimit) {
boolean allConsumed = true;
Long remainingLimit = null;
Long retryAfterNanoSeconds = null;
for (RateLimitCheck<Method> rl : rateLimitConfigResult.getRateLimitChecks()) {
var wrapper = rl.rateLimit(new ExpressionParams<>(method).addParams(params), annotationRateLimit);
if (wrapper != null && wrapper.getRateLimitResult() != null) {
var rateLimitResult = wrapper.getRateLimitResult();
if (rateLimitResult.isConsumed()) {
remainingLimit = RateLimitService.getRemainingLimit(remainingLimit, rateLimitResult);
} else {
remainingLimit = RateLimitService.getRemainingLimit(remainingLimit, rateLimitResult);
retryAfterNanoSeconds = rateLimitResult.getNanosToWaitForRefill();
if (!rateLimitResult.isConsumed()) {
allConsumed = false;
break;
}
}
}
if (allConsumed) {
log.debug("rate-limit-remaining;limit:{}", remainingLimit);
} else {
log.debug("rate-limit-consumed;limit:{};rate-limit-retry-after;limit:{}", remainingLimit, retryAfterNanoSeconds);
}
return new RateLimitConsumedResult(allConsumed, remainingLimit);
return new RateLimitConsumedResult(allConsumed, remainingLimit, retryAfterNanoSeconds);
}

private record RateLimitConsumedResult(boolean allConsumed, Long remainingLimit) {
private record RateLimitConsumedResult(boolean allConsumed, Long remainingLimit, Long retryAfterNanoSeconds) {
}

/*
Expand Down
15 changes: 10 additions & 5 deletions ...giffing/bucket4j/spring/boot/starter/general/tests/method/method/MethodRateLimitTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,14 +80,19 @@ public void assert_no_rate_limit_with_skip_condition_matches() {

@Test
public void assert_rate_limit_with_cache_key() {
for(int i = 0; i < 5; i++) {
// rate limit by parameter value
for (int i = 0; i < 5; i++) {
testService.withCacheKey("key1");
testService.withCacheKey("key2");
// all tokens consumed
}
assertThrows(RateLimitException.class, () -> testService.withCacheKey("key1"));
assertThrows(RateLimitException.class, () -> testService.withCacheKey("key2"));
RateLimitException ex1 = assertThrows(RateLimitException.class, () -> testService.withCacheKey("key1"));
assertTrue(ex1.getRetryAfterNanoSeconds() >= 0);
assertTrue(ex1.getRemainingTokens() >= 0);
assertNotNull(ex1.getConfigurationName());
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

assertEquals("default", ex1.getConfigurationName());


RateLimitException ex2 = assertThrows(RateLimitException.class, () -> testService.withCacheKey("key2"));
assertTrue(ex2.getRetryAfterNanoSeconds() >= 0);
assertTrue(ex2.getRemainingTokens() >= 0);
assertNotNull(ex2.getConfigurationName());
}

@Test
Expand Down