diff --git a/CHANGELOG.md b/CHANGELOG.md index af02c08d..44478806 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +### Changed + +- Send message results now surface server-advised retry time for plain rate-limit (HTTP 413) failures, not only for proof-required challenges. The `retryAfterSeconds` field in JSON-RPC `SendMessageResult` is populated whenever the server sends a `Retry-After` header. The canonical way to distinguish proof-required failures remains `token != null`. Text output includes "retry after N seconds" when known. + ## [0.14.2] - 2026-04-04 ### Added diff --git a/lib/src/main/java/org/asamk/signal/manager/api/SendMessageResult.java b/lib/src/main/java/org/asamk/signal/manager/api/SendMessageResult.java index 4158b60e..82b83aa9 100644 --- a/lib/src/main/java/org/asamk/signal/manager/api/SendMessageResult.java +++ b/lib/src/main/java/org/asamk/signal/manager/api/SendMessageResult.java @@ -11,11 +11,38 @@ public record SendMessageResult( boolean isIdentityFailure, boolean isRateLimitFailure, ProofRequiredException proofRequiredFailure, - boolean isInvalidPreKeyFailure + boolean isInvalidPreKeyFailure, + Long rateLimitRetryAfterSeconds ) { + /** + * Source-compatible constructor for callers built against the pre-retry-after record shape. + * Delegates to the canonical constructor with a null retry-after, which is the correct value + * for any result not produced by {@link #from}. + */ + public SendMessageResult( + RecipientAddress address, + boolean isSuccess, + boolean isNetworkFailure, + boolean isUnregisteredFailure, + boolean isIdentityFailure, + boolean isRateLimitFailure, + ProofRequiredException proofRequiredFailure, + boolean isInvalidPreKeyFailure + ) { + this(address, + isSuccess, + isNetworkFailure, + isUnregisteredFailure, + isIdentityFailure, + isRateLimitFailure, + proofRequiredFailure, + isInvalidPreKeyFailure, + null); + } + 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( @@ -23,16 +50,32 @@ public record SendMessageResult( RecipientResolver recipientResolver, RecipientAddressResolver addressResolver ) { + final var rateLimitFailure = sendMessageResult.getRateLimitFailure(); + final var proofRequiredFailure = sendMessageResult.getProofRequiredFailure(); + final Long retryAfterSeconds; + if (proofRequiredFailure != null) { + retryAfterSeconds = proofRequiredFailure.getRetryAfterSeconds(); + } else if (rateLimitFailure != null) { + retryAfterSeconds = rateLimitFailure.getRetryAfterMilliseconds() + .map(SendMessageResult::millisToCeilingSeconds) + .orElse(null); + } else { + retryAfterSeconds = null; + } return new SendMessageResult(addressResolver.resolveRecipientAddress(recipientResolver.resolveRecipient( sendMessageResult.getAddress())).toApiRecipientAddress(), sendMessageResult.isSuccess(), sendMessageResult.isNetworkFailure(), sendMessageResult.isUnregisteredFailure(), sendMessageResult.getIdentityFailure() != null, - sendMessageResult.getRateLimitFailure() != null || sendMessageResult.getProofRequiredFailure() != null, - sendMessageResult.getProofRequiredFailure() == null - ? null - : new ProofRequiredException(sendMessageResult.getProofRequiredFailure()), - sendMessageResult.isInvalidPreKeyFailure()); + rateLimitFailure != null || proofRequiredFailure != null, + proofRequiredFailure == null ? null : new ProofRequiredException(proofRequiredFailure), + sendMessageResult.isInvalidPreKeyFailure(), + retryAfterSeconds); + } + + static long millisToCeilingSeconds(long millis) { + // Round up so we never advise a retry before the server's deadline. + return (millis + 999L) / 1000L; } } diff --git a/lib/src/main/java/org/asamk/signal/manager/api/SendMessageResults.java b/lib/src/main/java/org/asamk/signal/manager/api/SendMessageResults.java index de250163..83e29928 100644 --- a/lib/src/main/java/org/asamk/signal/manager/api/SendMessageResults.java +++ b/lib/src/main/java/org/asamk/signal/manager/api/SendMessageResults.java @@ -26,4 +26,18 @@ public record SendMessageResults(long timestamp, Map res.stream().map(SendMessageResult::isRateLimitFailure)) .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); + } } diff --git a/lib/src/test/java/org/asamk/signal/manager/api/SendMessageResultTest.java b/lib/src/test/java/org/asamk/signal/manager/api/SendMessageResultTest.java new file mode 100644 index 00000000..b4a4bf4e --- /dev/null +++ b/lib/src/test/java/org/asamk/signal/manager/api/SendMessageResultTest.java @@ -0,0 +1,38 @@ +package org.asamk.signal.manager.api; + +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNull; + +class SendMessageResultTest { + + /** + * Ceiling division — we must never advise a retry before the server's deadline, + * so sub-second values round up rather than truncate toward zero. + */ + @Test + void millisToCeilingSecondsRoundsUp() { + assertEquals(0L, SendMessageResult.millisToCeilingSeconds(0L)); + assertEquals(1L, SendMessageResult.millisToCeilingSeconds(1L)); + assertEquals(1L, SendMessageResult.millisToCeilingSeconds(500L)); + assertEquals(1L, SendMessageResult.millisToCeilingSeconds(999L)); + assertEquals(1L, SendMessageResult.millisToCeilingSeconds(1000L)); + assertEquals(2L, SendMessageResult.millisToCeilingSeconds(1001L)); + assertEquals(2L, SendMessageResult.millisToCeilingSeconds(1500L)); + assertEquals(60L, SendMessageResult.millisToCeilingSeconds(60_000L)); + } + + /** + * Source-compat: callers built against the pre-retry-after record shape use the 8-arg + * constructor. It must continue to compile and produce a record with a null retry-after. + */ + @Test + @SuppressWarnings("deprecation") + void legacyEightArgConstructorPreservesSourceCompat() { + var result = new SendMessageResult(new RecipientAddress(null, null, "+15551234567", null), + true, false, false, false, false, null, false); + + assertNull(result.rateLimitRetryAfterSeconds()); + } +} diff --git a/src/main/java/org/asamk/signal/json/JsonSendMessageResult.java b/src/main/java/org/asamk/signal/json/JsonSendMessageResult.java index 1be6cb9e..098d1916 100644 --- a/src/main/java/org/asamk/signal/json/JsonSendMessageResult.java +++ b/src/main/java/org/asamk/signal/json/JsonSendMessageResult.java @@ -32,7 +32,7 @@ public record JsonSendMessageResult( ? Type.INVALID_PRE_KEY_FAILURE : Type.IDENTITY_FAILURE, result.proofRequiredFailure() != null ? result.proofRequiredFailure().getToken() : null, - result.proofRequiredFailure() != null ? result.proofRequiredFailure().getRetryAfterSeconds() : null); + result.rateLimitRetryAfterSeconds()); } public enum Type { diff --git a/src/main/java/org/asamk/signal/util/SendMessageResultUtils.java b/src/main/java/org/asamk/signal/util/SendMessageResultUtils.java index a23e3c54..c438dfc2 100644 --- a/src/main/java/org/asamk/signal/util/SendMessageResultUtils.java +++ b/src/main/java/org/asamk/signal/util/SendMessageResultUtils.java @@ -59,8 +59,10 @@ public class SendMessageResultUtils { if (sendMessageResults.hasOnlyUntrustedIdentity()) { throw new UntrustedKeyErrorException("Failed to send message due to untrusted identities"); } 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", - new RateLimitException(0)); + new RateLimitException(nextAttempt)); } else { throw new UserErrorException("Failed to send message"); } @@ -110,7 +112,10 @@ public class SendMessageResultUtils { } else if (result.isNetworkFailure()) { return String.format("Network failure for \"%s\"", identifier); } 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()) { return String.format("Unregistered user \"%s\"", identifier); } else if (result.isIdentityFailure()) { diff --git a/src/test/java/org/asamk/signal/json/JsonSendMessageResultTest.java b/src/test/java/org/asamk/signal/json/JsonSendMessageResultTest.java new file mode 100644 index 00000000..1811c6f3 --- /dev/null +++ b/src/test/java/org/asamk/signal/json/JsonSendMessageResultTest.java @@ -0,0 +1,113 @@ +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 java.util.UUID; + +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 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()); + } + + @Test + void aggregateReturnsLongestRetryAfter() { + var small = rateLimited("+15551234567", 60L); + var big = rateLimited("+15559876543", 3600L); + var unknown = rateLimited("+15550000000", null); + + var aggregate = new SendMessageResults(1L, + Map.of(new RecipientIdentifier.Uuid(UUID.randomUUID()), List.of(small, big, unknown))); + + assertEquals(3600L, aggregate.maxRateLimitRetryAfterSeconds()); + } + + @Test + void aggregateReturnsNullWhenNoRetryAfter() { + var aggregate = new SendMessageResults(1L, + Map.of(new RecipientIdentifier.Uuid(UUID.randomUUID()), + List.of(rateLimited("+15551234567", null)))); + + assertNull(aggregate.maxRateLimitRetryAfterSeconds()); + } + + /** + * Regression for a bug where the aggregate helper could overlook the longest + * wait if only some recipients reported a value. Ensures the max is picked + * across any mix — which is what downstream captcha/rate-limit clients rely on. + */ + @Test + void aggregatePicksMaxEvenWhenSomeValuesAreNull() { + var withValue = rateLimited("+15551111111", 7200L); + var withoutValue = rateLimited("+15552222222", null); + var alsoWithValue = rateLimited("+15553333333", 120L); + + var aggregate = new SendMessageResults(1L, + Map.of(new RecipientIdentifier.Uuid(UUID.randomUUID()), + List.of(withoutValue, withValue, alsoWithValue))); + + assertEquals(7200L, aggregate.maxRateLimitRetryAfterSeconds()); + } + + private static SendMessageResult rateLimited(String number, Long retryAfterSeconds) { + return new SendMessageResult(new RecipientAddress(null, null, number, null), + false, false, false, false, + true, + null, + false, + retryAfterSeconds); + } +}