Merge 0b1261c54973777c818c7a51d79673311494140c into aafb40fd94ad5184024616db30daf02212303eff

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

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,
+                rateLimitFailure != null || proofRequiredFailure != null,
-                sendMessageResult.getProofRequiredFailure() == null
+                proofRequiredFailure == null ? null : new ProofRequiredException(proofRequiredFailure),
-                        ? null
+                sendMessageResult.isInvalidPreKeyFailure(),
-                        : new ProofRequiredException(sendMessageResult.getProofRequiredFailure()),
+                retryAfterSeconds);
-                sendMessageResult.isInvalidPreKeyFailure());
+    }
+
+    static long millisToCeilingSeconds(long millis) {
+        // Round up so we never advise a retry before the server's deadline.
+        return (millis + 999L) / 1000L;
     }
 }

lib/src/main/java/org/asamk/signal/manager/api/SendMessageResults.java

@@ -26,4 +26,18 @@ public record SendMessageResults(long timestamp, Map<RecipientIdentifier, List<S
                 .flatMap(res -> 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);
+    }
 }

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());
+    }
+}

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 {

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()) {

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);
+    }
+}