Improve Message.createdLocallyAt creation logic using estimated server time

[VelikovPetar]

Goal

When a device clock is skewed relative to the server, createdLocallyAt timestamps assigned to locally-sent messages can be incorrect. This causes cross-user ordering issues — messages from a user with a fast clock appear in the future relative to messages from other users.

Implementation

Introduced ServerClockOffset, an NTP-style estimator that tracks the offset between the local device clock and the server clock using WebSocket round-trips:

• onConnectionStarted() — records local time before opening the WebSocket.
• onConnected(serverTime) — computes initial offset from the ConnectedEvent timestamp using the NTP midpoint formula: offset = (sentAt + receivedAt) / 2 - serverTime.
• onHealthCheckSent() — records local time before each health check echo.
• onHealthCheck(serverTime) — refines the offset from each HealthEvent, accepting only the sample with the lowest RTT (min-RTT selection minimises network-asymmetry error).
• estimatedServerTime() — returns Date(localTime - offset), used in ChatClient.ensureCreatedLocallyAt() instead of the raw local clock.

ServerClockOffset is constructed once per ChatClient instance, wired through ChatModule into ChatSocket (for calibration) and ChatClient (for consumption). All mutable state uses @Volatile for thread safety.

Defensive max-offset cap

A maxOffsetMs parameter (default 1 hour) guards against unreliable server timestamps — for example, a test environment mock server that returns a static, stale health.check timestamp. Without this cap, a large artificial offset would cause estimatedServerTime() to return a date far in the past, breaking exceedsSyncThreshold comparisons and suppressing the message delivery status icon.

When the computed offset exceeds maxOffsetMs, the sample is silently rejected:

• onConnected: resets offsetMs = 0 before attempting to apply the new offset, so a rejection always falls back to local time (safe pre-PR behaviour).
• onHealthCheck: leaves offsetMs unchanged on rejection, preserving the last known good value from onConnected or a previous health check.

Why 1 hour? Both System.currentTimeMillis() and server timestamps are UTC epoch milliseconds — device timezone has no effect on the offset. Real-world NTP drift is at most seconds; even severely misconfigured clocks rarely exceed minutes. 1 hour is a very conservative upper bound that correctly rejects stale test fixtures while accepting any realistic skew.

UI Changes

Before	After
clock-before.mov	clock-after.mov

Testing

• New unit test class ServerClockOffsetTest covers:
  • Initial state (zero offset, returns local time)
  • onConnected with and without a paired onConnectionStarted
  • onHealthCheck min-RTT selection and stale-sample rejection
  • maxRttMs boundary conditions
  • maxOffsetMs boundary conditions — implausibly large offsets rejected on both onConnected and onHealthCheck; onHealthCheck preserves prior valid offset on rejection
  • Thread-safety via concurrent coroutines
• Existing tests updated to supply ServerClockOffset via FakeChatSocket and MockClientBuilder.

[github-actions]

PR checklist ✅

All required conditions are satisfied:

• Title length is OK (or ignored by label).
• At least one pr: label exists.
• Sections ### Goal, ### Implementation, and ### Testing are filled.

🎉 Great job! This PR is ready for review.

[github-actions]

SDK Size Comparison 📏

SDK	Before	After	Difference	Status
stream-chat-android-client	5.26 MB	5.26 MB	0.00 MB	🟢
stream-chat-android-offline	5.48 MB	5.49 MB	0.00 MB	🟢
stream-chat-android-ui-components	10.63 MB	10.63 MB	0.00 MB	🟢
stream-chat-android-compose	12.85 MB	12.85 MB	0.00 MB	🟢

[coderabbitai]

Walkthrough

A new ServerClockOffset utility class is introduced to estimate server time via NTP-style synchronization using WebSocket health checks. The dependency is integrated through ChatClient, ChatModule, and ChatSocket to synchronize local timestamps with server time during message creation and connection lifecycle events.

Changes

Cohort / File(s)	Summary
Clock Offset Implementation stream-chat-android-client/src/main/java/io/getstream/chat/android/client/utils/internal/ServerClockOffset.kt	New internal utility class managing server/local clock synchronization with NTP-style offset calculation, calibration on connection, health check refinement, and estimated server time derivation via volatile state and RTT tracking.
Client Integration stream-chat-android-client/src/main/java/io/getstream/chat/android/client/ChatClient.kt	Added serverClockOffset dependency to ChatClient constructor; replaced local now() with serverClockOffset.estimatedServerTime() when computing Message.createdLocallyAt to reduce cross-client clock skew.
Dependency Injection stream-chat-android-client/src/main/java/io/getstream/chat/android/client/di/ChatModule.kt	Wired ServerClockOffset through ChatModule constructor and propagated to ChatSocket instantiation, threading the clock offset from module initialization to socket layer.
Socket Lifecycle Integration stream-chat-android-client/src/main/java/io/getstream/chat/android/client/socket/ChatSocket.kt	Integrated serverClockOffset callbacks at connection lifecycle milestones: onConnectionStarted() at WebSocket init, onHealthCheckSent() before health callback, onConnected() at connection established, and onHealthCheck() on health event receipt.
Test Fixtures & Builders stream-chat-android-client/src/test/java/io/getstream/chat/android/client/.../BaseChatClientTest.kt, ChatClientConnectionTests.kt, ChatClientTest.kt, DependencyResolverTest.kt, MockClientBuilder.kt, FakeChatSocket.kt, ChatClientDebuggerTest.kt	Updated all test file ChatClient and FakeChatSocket constructors to instantiate and wire ServerClockOffset as a required parameter in test fixtures and mock builders.
Test Coverage stream-chat-android-client/src/test/java/io/getstream/chat/android/client/utils/internal/ServerClockOffsetTest.kt	Comprehensive test suite covering baseline behavior, onConnected calibration under forward/backward clock skew, health check convergence via NTP midpoint, RTT selection and maxRttMs filtering, reconnect resets, edge cases, and validation of estimatedServerTime consistency across scenarios.

Sequence Diagram(s)

sequenceDiagram
    participant Client as Client
    participant Socket as ChatSocket
    participant Offset as ServerClockOffset
    participant State as State Service

    Client->>Socket: initiateConnection()
    activate Socket
    
    Socket->>Offset: onConnectionStarted()
    activate Offset
    Offset->>Offset: recordConnectionStart(localTime)
    deactivate Offset
    
    Socket->>Socket: createWebSocket()
    
    Note over Socket: Connection Established
    Socket->>Offset: onConnected(serverTime)
    activate Offset
    Offset->>Offset: calibrate(serverTime, localTime)
    Offset->>Offset: calculateOffset via NTP midpoint
    deactivate Offset
    
    Socket->>State: notifyConnected()
    
    rect rgba(100, 200, 150, 0.5)
        Note over Socket: Health Check Loop
        Socket->>Offset: onHealthCheckSent()
        activate Offset
        Offset->>Offset: recordHealthCheckSent(localTime)
        deactivate Offset
        
        Socket->>Socket: sendHealthMessage()
        
        Socket->>Offset: onHealthCheck(serverTime)
        activate Offset
        Offset->>Offset: refineOffset via RTT & NTP
        deactivate Offset
    end
    
    deactivate Socket
    
    Client->>Client: createMessage()
    Client->>Offset: estimatedServerTime()
    activate Offset
    Offset->>Offset: return localTime + offset
    deactivate Offset
    Client->>Client: setMessageCreatedLocallyAt()

Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Poem

🐰 Tick-tock, the bunny hops through time,
Syncing clocks in perfect rhyme,
No more skew, no more delay,
Server time saves the day! ⏰
Hop on westbound, hop on east,
All messages ordered, watch them feast!

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 34.21% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (2 passed)

Check name	Status	Explanation
Description check	✅ Passed	The PR description comprehensively covers Goal, Implementation, UI Changes with before/after videos, and extensive Testing details. All required template sections are present and well-documented.
Title check	✅ Passed	The pull request title clearly and specifically summarizes the main change: improving message timestamp creation by using estimated server time instead of local time to handle device clock skew.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch bug/AND-1069_calculate_created_locally_at_using_server_clock_offset_calculation

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against the current code and only fix it if needed.

Inline comments:
In
`@stream-chat-android-client/src/main/java/io/getstream/chat/android/client/socket/ChatSocket.kt`:
- Around line 66-70: The health-check timestamp is recorded before confirming
the echo was actually sent; update the checkCallback so that when
chatSocketStateService.currentState is a State.Connected and you obtain the
event, you call sendEvent(it) first and only invoke
serverClockOffset.onHealthCheckSent() if sendEvent(it) returns true (i.e., the
echo was successfully sent). Ensure you reference checkCallback,
State.Connected, sendEvent(it), and serverClockOffset.onHealthCheckSent() when
making the change.

In
`@stream-chat-android-client/src/main/java/io/getstream/chat/android/client/utils/internal/ServerClockOffset.kt`:
- Around line 44-55: The compound read/modify/write on volatile fields
(offsetMs, bestRttMs, healthCheckSentAtMs, connectionStartedAtMs) in
onConnected() and onHealthCheck() is racy; wrap those multi-field state
transitions in a mutual-exclusion block using a dedicated lock (e.g., private
val stateLock = Any()) and replace the check-then-set sequences with
synchronized(stateLock) { ... } blocks around the code that updates bestRttMs
and offsetMs so stale comparisons cannot overwrite better values; keep
single-field setters (onConnectionStarted(), onHealthCheckSent()) unchanged and
update the KDoc to state that compound operations are synchronized via stateLock
rather than relying solely on `@Volatile`.

---

ℹ️ Review info

Configuration used: Repository UI

Review profile: CHILL

Plan: Pro

Disabled knowledge base sources:

• Linear integration is disabled

You can enable these sources in your CodeRabbit configuration.

📥 Commits

Reviewing files that changed from the base of the PR and between 29b2d88 and f455b35.

📒 Files selected for processing (12)

• stream-chat-android-client/src/main/java/io/getstream/chat/android/client/ChatClient.kt
• stream-chat-android-client/src/main/java/io/getstream/chat/android/client/di/ChatModule.kt
• stream-chat-android-client/src/main/java/io/getstream/chat/android/client/socket/ChatSocket.kt
• stream-chat-android-client/src/main/java/io/getstream/chat/android/client/utils/internal/ServerClockOffset.kt
• stream-chat-android-client/src/test/java/io/getstream/chat/android/client/ChatClientConnectionTests.kt
• stream-chat-android-client/src/test/java/io/getstream/chat/android/client/ChatClientTest.kt
• stream-chat-android-client/src/test/java/io/getstream/chat/android/client/DependencyResolverTest.kt
• stream-chat-android-client/src/test/java/io/getstream/chat/android/client/MockClientBuilder.kt
• stream-chat-android-client/src/test/java/io/getstream/chat/android/client/chatclient/BaseChatClientTest.kt
• stream-chat-android-client/src/test/java/io/getstream/chat/android/client/debugger/ChatClientDebuggerTest.kt
• stream-chat-android-client/src/test/java/io/getstream/chat/android/client/socket/FakeChatSocket.kt
• stream-chat-android-client/src/test/java/io/getstream/chat/android/client/utils/internal/ServerClockOffsetTest.kt

[sonarqubecloud]

Quality Gate failed

Failed conditions
65.5% Coverage on New Code (required ≥ 80%)

See analysis details on SonarQube Cloud

[andremion]

Good job!
I think the approach is clear and it helps for createdLocallyAt and sync checks.

I have some small questions, not blockers:

Reactions: Messages use estimatedServerTime() but reactions still use now() for createdLocallyAt. Should we align reactions to estimatedServerTime() also?

Integrators: ServerClockOffset has parameters like maxOffsetMs / maxRttMs. Should we let integrators adjust these (e.g. on ChatClient builder), or we keep it internal only? Exposing could help special environments, but also more risk if someone sets bad values.

What do you think?

[VelikovPetar]

Good job! I think the approach is clear and it helps for createdLocallyAt and sync checks.

I have some small questions, not blockers:

Reactions: Messages use estimatedServerTime() but reactions still use now() for createdLocallyAt. Should we align reactions to estimatedServerTime() also?

Integrators: ServerClockOffset has parameters like maxOffsetMs / maxRttMs. Should we let integrators adjust these (e.g. on ChatClient builder), or we keep it internal only? Exposing could help special environments, but also more risk if someone sets bad values.

What do you think?

Concerning the customisation, I would personally not allow it (at least not initially). I would say that this is internal logic, and we shouldn't allow customisation, unless explicitly requested. Plus, it would mean another (tricky) public API to maintain.

Reactions

I would say that messages is the most important thing to cover here - as messages appearing out-of-order is a big issue, but I don't think it is that important for reactions. I am bit in favour of keeping it simple (keeping the logic as-is), but if you think we should align the reactions as well, I would be happy to do it.

[stream-public-bot]

🚀 Available in v6.35.1