mirror of
https://github.com/AsamK/signal-cli.git
synced 2026-05-18 13:14:14 +00:00
Surface retry-after seconds for plain rate-limit failures
libsignal-service's RateLimitException exposes retryAfterMilliseconds for HTTP 413 responses, but signal-cli only forwarded retry-after for ProofRequired (428) failures. Clients had no signal for when it was safe to retry plain rate-limited sends, so every failed retry potentially extended the server-side window. SendMessageResult now carries an optional rateLimitRetryAfterSeconds, populated from the upstream Optional<Long>. JsonSendMessageResult exposes it for RATE_LIMIT_FAILURE type. Text output includes the window when known. Aggregate RateLimitErrorException now carries the real nextAttemptTimestamp (was hardcoded to 0). Closes #1996. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This commit is contained in:
parent
a03d17a9e4
commit
196b70b63d
@ -11,11 +11,12 @@ public record SendMessageResult(
|
|||||||
boolean isIdentityFailure,
|
boolean isIdentityFailure,
|
||||||
boolean isRateLimitFailure,
|
boolean isRateLimitFailure,
|
||||||
ProofRequiredException proofRequiredFailure,
|
ProofRequiredException proofRequiredFailure,
|
||||||
boolean isInvalidPreKeyFailure
|
boolean isInvalidPreKeyFailure,
|
||||||
|
Long rateLimitRetryAfterSeconds
|
||||||
) {
|
) {
|
||||||
|
|
||||||
public static SendMessageResult unregisteredFailure(RecipientAddress address) {
|
public static SendMessageResult unregisteredFailure(RecipientAddress address) {
|
||||||
return new SendMessageResult(address, false, false, true, false, false, null, false);
|
return new SendMessageResult(address, false, false, true, false, false, null, false, null);
|
||||||
}
|
}
|
||||||
|
|
||||||
public static SendMessageResult from(
|
public static SendMessageResult from(
|
||||||
@ -23,16 +24,19 @@ public record SendMessageResult(
|
|||||||
RecipientResolver recipientResolver,
|
RecipientResolver recipientResolver,
|
||||||
RecipientAddressResolver addressResolver
|
RecipientAddressResolver addressResolver
|
||||||
) {
|
) {
|
||||||
|
final var rateLimitFailure = sendMessageResult.getRateLimitFailure();
|
||||||
|
final var proofRequiredFailure = sendMessageResult.getProofRequiredFailure();
|
||||||
return new SendMessageResult(addressResolver.resolveRecipientAddress(recipientResolver.resolveRecipient(
|
return new SendMessageResult(addressResolver.resolveRecipientAddress(recipientResolver.resolveRecipient(
|
||||||
sendMessageResult.getAddress())).toApiRecipientAddress(),
|
sendMessageResult.getAddress())).toApiRecipientAddress(),
|
||||||
sendMessageResult.isSuccess(),
|
sendMessageResult.isSuccess(),
|
||||||
sendMessageResult.isNetworkFailure(),
|
sendMessageResult.isNetworkFailure(),
|
||||||
sendMessageResult.isUnregisteredFailure(),
|
sendMessageResult.isUnregisteredFailure(),
|
||||||
sendMessageResult.getIdentityFailure() != null,
|
sendMessageResult.getIdentityFailure() != null,
|
||||||
sendMessageResult.getRateLimitFailure() != null || sendMessageResult.getProofRequiredFailure() != null,
|
rateLimitFailure != null || proofRequiredFailure != null,
|
||||||
sendMessageResult.getProofRequiredFailure() == null
|
proofRequiredFailure == null ? null : new ProofRequiredException(proofRequiredFailure),
|
||||||
|
sendMessageResult.isInvalidPreKeyFailure(),
|
||||||
|
rateLimitFailure == null
|
||||||
? null
|
? null
|
||||||
: new ProofRequiredException(sendMessageResult.getProofRequiredFailure()),
|
: rateLimitFailure.getRetryAfterMilliseconds().map(ms -> ms / 1000L).orElse(null));
|
||||||
sendMessageResult.isInvalidPreKeyFailure());
|
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@ -26,4 +26,18 @@ public record SendMessageResults(long timestamp, Map<RecipientIdentifier, List<S
|
|||||||
.flatMap(res -> res.stream().map(SendMessageResult::isRateLimitFailure))
|
.flatMap(res -> res.stream().map(SendMessageResult::isRateLimitFailure))
|
||||||
.allMatch(r -> r) && results.values().stream().mapToInt(List::size).sum() > 0;
|
.allMatch(r -> r) && results.values().stream().mapToInt(List::size).sum() > 0;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Longest rate-limit retry-after window across all rate-limited recipients, in seconds.
|
||||||
|
* Null when no recipient reported one (server omitted Retry-After, or no rate-limit failures).
|
||||||
|
*/
|
||||||
|
public Long maxRateLimitRetryAfterSeconds() {
|
||||||
|
return results.values()
|
||||||
|
.stream()
|
||||||
|
.flatMap(List::stream)
|
||||||
|
.map(SendMessageResult::rateLimitRetryAfterSeconds)
|
||||||
|
.filter(r -> r != null)
|
||||||
|
.max(Long::compareTo)
|
||||||
|
.orElse(null);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@ -32,7 +32,9 @@ public record JsonSendMessageResult(
|
|||||||
? Type.INVALID_PRE_KEY_FAILURE
|
? Type.INVALID_PRE_KEY_FAILURE
|
||||||
: Type.IDENTITY_FAILURE,
|
: Type.IDENTITY_FAILURE,
|
||||||
result.proofRequiredFailure() != null ? result.proofRequiredFailure().getToken() : null,
|
result.proofRequiredFailure() != null ? result.proofRequiredFailure().getToken() : null,
|
||||||
result.proofRequiredFailure() != null ? result.proofRequiredFailure().getRetryAfterSeconds() : null);
|
result.proofRequiredFailure() != null
|
||||||
|
? (Long) result.proofRequiredFailure().getRetryAfterSeconds()
|
||||||
|
: result.rateLimitRetryAfterSeconds());
|
||||||
}
|
}
|
||||||
|
|
||||||
public enum Type {
|
public enum Type {
|
||||||
|
|||||||
@ -59,8 +59,10 @@ public class SendMessageResultUtils {
|
|||||||
if (sendMessageResults.hasOnlyUntrustedIdentity()) {
|
if (sendMessageResults.hasOnlyUntrustedIdentity()) {
|
||||||
throw new UntrustedKeyErrorException("Failed to send message due to untrusted identities");
|
throw new UntrustedKeyErrorException("Failed to send message due to untrusted identities");
|
||||||
} else if (sendMessageResults.hasOnlyRateLimitFailure()) {
|
} else if (sendMessageResults.hasOnlyRateLimitFailure()) {
|
||||||
|
final var retryAfter = sendMessageResults.maxRateLimitRetryAfterSeconds();
|
||||||
|
final var nextAttempt = retryAfter == null ? 0L : System.currentTimeMillis() + retryAfter * 1000L;
|
||||||
throw new RateLimitErrorException("Failed to send message due to rate limiting",
|
throw new RateLimitErrorException("Failed to send message due to rate limiting",
|
||||||
new RateLimitException(0));
|
new RateLimitException(nextAttempt));
|
||||||
} else {
|
} else {
|
||||||
throw new UserErrorException("Failed to send message");
|
throw new UserErrorException("Failed to send message");
|
||||||
}
|
}
|
||||||
@ -110,7 +112,10 @@ public class SendMessageResultUtils {
|
|||||||
} else if (result.isNetworkFailure()) {
|
} else if (result.isNetworkFailure()) {
|
||||||
return String.format("Network failure for \"%s\"", identifier);
|
return String.format("Network failure for \"%s\"", identifier);
|
||||||
} else if (result.isRateLimitFailure()) {
|
} else if (result.isRateLimitFailure()) {
|
||||||
return String.format("Rate limit failure for \"%s\"", identifier);
|
final var retryAfter = result.rateLimitRetryAfterSeconds();
|
||||||
|
return retryAfter != null
|
||||||
|
? String.format("Rate limit failure for \"%s\", retry after %d seconds", identifier, retryAfter)
|
||||||
|
: String.format("Rate limit failure for \"%s\"", identifier);
|
||||||
} else if (result.isUnregisteredFailure()) {
|
} else if (result.isUnregisteredFailure()) {
|
||||||
return String.format("Unregistered user \"%s\"", identifier);
|
return String.format("Unregistered user \"%s\"", identifier);
|
||||||
} else if (result.isIdentityFailure()) {
|
} else if (result.isIdentityFailure()) {
|
||||||
|
|||||||
@ -0,0 +1,105 @@
|
|||||||
|
package org.asamk.signal.json;
|
||||||
|
|
||||||
|
import org.asamk.signal.manager.api.RecipientAddress;
|
||||||
|
import org.asamk.signal.manager.api.RecipientIdentifier;
|
||||||
|
import org.asamk.signal.manager.api.SendMessageResult;
|
||||||
|
import org.asamk.signal.manager.api.SendMessageResults;
|
||||||
|
import org.junit.jupiter.api.Test;
|
||||||
|
|
||||||
|
import java.util.List;
|
||||||
|
import java.util.Map;
|
||||||
|
|
||||||
|
import static org.junit.jupiter.api.Assertions.assertEquals;
|
||||||
|
import static org.junit.jupiter.api.Assertions.assertNull;
|
||||||
|
|
||||||
|
class JsonSendMessageResultTest {
|
||||||
|
|
||||||
|
private static final RecipientAddress ADDRESS = new RecipientAddress(null, null, "+15551234567", null);
|
||||||
|
|
||||||
|
@Test
|
||||||
|
void rateLimitFailureSurfacesRetryAfterSeconds() {
|
||||||
|
var result = new SendMessageResult(ADDRESS,
|
||||||
|
false, false, false, false,
|
||||||
|
true,
|
||||||
|
null,
|
||||||
|
false,
|
||||||
|
3600L);
|
||||||
|
|
||||||
|
var json = JsonSendMessageResult.from(result);
|
||||||
|
|
||||||
|
assertEquals(JsonSendMessageResult.Type.RATE_LIMIT_FAILURE, json.type());
|
||||||
|
assertEquals(3600L, json.retryAfterSeconds());
|
||||||
|
assertNull(json.token());
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
void rateLimitFailureWithoutRetryAfterLeavesFieldNull() {
|
||||||
|
var result = new SendMessageResult(ADDRESS,
|
||||||
|
false, false, false, false,
|
||||||
|
true,
|
||||||
|
null,
|
||||||
|
false,
|
||||||
|
null);
|
||||||
|
|
||||||
|
var json = JsonSendMessageResult.from(result);
|
||||||
|
|
||||||
|
assertEquals(JsonSendMessageResult.Type.RATE_LIMIT_FAILURE, json.type());
|
||||||
|
assertNull(json.retryAfterSeconds());
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
void sendMessageResultsReturnsMaxRetryAfter() {
|
||||||
|
var small = new SendMessageResult(ADDRESS,
|
||||||
|
false, false, false, false,
|
||||||
|
true,
|
||||||
|
null,
|
||||||
|
false,
|
||||||
|
60L);
|
||||||
|
var big = new SendMessageResult(new RecipientAddress(null, null, "+15559876543", null),
|
||||||
|
false, false, false, false,
|
||||||
|
true,
|
||||||
|
null,
|
||||||
|
false,
|
||||||
|
3600L);
|
||||||
|
var unknown = new SendMessageResult(new RecipientAddress(null, null, "+15550000000", null),
|
||||||
|
false, false, false, false,
|
||||||
|
true,
|
||||||
|
null,
|
||||||
|
false,
|
||||||
|
null);
|
||||||
|
|
||||||
|
var aggregate = new SendMessageResults(1L,
|
||||||
|
Map.of(new RecipientIdentifier.Uuid(java.util.UUID.randomUUID()), List.of(small, big, unknown)));
|
||||||
|
|
||||||
|
assertEquals(3600L, aggregate.maxRateLimitRetryAfterSeconds());
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
void sendMessageResultsReturnsNullWhenNoRetryAfter() {
|
||||||
|
var noRetry = new SendMessageResult(ADDRESS,
|
||||||
|
false, false, false, false,
|
||||||
|
true,
|
||||||
|
null,
|
||||||
|
false,
|
||||||
|
null);
|
||||||
|
var aggregate = new SendMessageResults(1L,
|
||||||
|
Map.of(new RecipientIdentifier.Uuid(java.util.UUID.randomUUID()), List.of(noRetry)));
|
||||||
|
|
||||||
|
assertNull(aggregate.maxRateLimitRetryAfterSeconds());
|
||||||
|
}
|
||||||
|
|
||||||
|
@Test
|
||||||
|
void successLeavesRetryAfterNull() {
|
||||||
|
var result = new SendMessageResult(ADDRESS,
|
||||||
|
true, false, false, false,
|
||||||
|
false,
|
||||||
|
null,
|
||||||
|
false,
|
||||||
|
null);
|
||||||
|
|
||||||
|
var json = JsonSendMessageResult.from(result);
|
||||||
|
|
||||||
|
assertEquals(JsonSendMessageResult.Type.SUCCESS, json.type());
|
||||||
|
assertNull(json.retryAfterSeconds());
|
||||||
|
}
|
||||||
|
}
|
||||||
Loading…
x
Reference in New Issue
Block a user