Skip to content

feat: [SDK-5397] send a heartbeat every 30 minutes of foreground time - #1773

Closed
fadi-george wants to merge 3 commits into
fadi/SDK-5391-ios-session-endfrom
fadi/SDK-5397-ios-heartbeat
Closed

fadi-george wants to merge 3 commits into
fadi/SDK-5391-ios-session-endfrom
fadi/SDK-5397-ios-heartbeat

Conversation

@fadi-george

Copy link
Copy Markdown
Collaborator

Description

One Line Summary

When the sessions API flag is on, enqueue a duration-only session update every 30 minutes of foreground time.

Details

Motivation

Part of the iOS SDK Sessions stack (SDK-5397). Keeps long foreground sessions open on the server and limits lost duration if the app is killed in the foreground. Based on #1772. Android counterpart: OneSignal/OneSignal-Android-SDK#2782.

Scope

  • OSSessionService starts the heartbeat on focus and when a new session starts, only for records whose usesSessionsApi is true, and stops it on unfocus. Flag off: no timer.
  • Counts foreground time only: a heartbeat is due once the session's duration (including the current foreground visit, on the monotonic clock) is 30 minutes past the last heartbeat's. Each scheduled check recomputes the remaining time, so a drifting scheduler can't send early. Durations only increase, so an unchanged duration is never sent.
  • Each heartbeat enqueues an update with the cumulative duration and no end_time. Existing coalescing in OSSessionRequestQueue folds unsent heartbeats and discards them once the end is queued. No queue changes.
  • New optional OSSessionRecord.lastHeartbeatDuration / lastHeartbeatTime. An app killed in the foreground never adds its last visit to activeDuration, so the end now uses the larger of activeDuration and the last heartbeat's duration, and the later of the last unfocus (or start) and the last heartbeat as end_time.
  • A new session restarts the interval. The interval is hardcoded to 30 minutes.
  • Unlike Android, a heartbeat whose user or subscription IDs aren't assigned yet is still enqueued; the queue holds it until they are, like other session requests.

Testing

Unit testing

Added OSSessionHeartbeatTests on a virtual clock: one heartbeat after 30 minutes with the wall time recorded, cumulative duration after 60 minutes, background pauses the interval, a new session restarts it, no timer with the flag off, and a session killed after a heartbeat ends with the heartbeat's duration and time.

UnitTestApp_TestPlan_Reduced passes locally. SwiftLint: OSSessionService.swift now exceeds the 400-line file_length warning (416); no other new violations.

Manual testing

Not tested on device yet; end-to-end verification is planned in the iOS rollout and QA ticket.

Affected code checklist

  • Notifications
    • Display
    • Open
    • Push Processing
    • Confirm Deliveries
  • Outcomes
  • Sessions
  • In-App Messaging
  • REST API requests
  • Public API changes

Checklist

Overview

  • I have filled out all REQUIRED sections above
  • PR does one thing
  • Any Public API changes are explained in the PR details and conform to existing APIs

Testing

  • I have included test coverage for these changes, or explained why they are not needed
  • All automated tests pass, or I explained why that is not possible
  • I have personally tested this on my device, or explained why that is not possible

Final pass

  • Code is as readable as possible.
  • I have reviewed this PR myself, ensuring it meets each checklist item

🤖 Generated with Claude Code

fadi-george and others added 3 commits October 9, 2026 14:50
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… starts

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@fadi-george
fadi-george added this pull request to stack #1771 October 9, 2026 23:34

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Multi-model review

Models: Claude Opus 5.5, GPT 5.6 Sol, Grok 4.7.

Act on

  • A missed heartbeat (visit folded in onUnfocus, late asyncAfter dropped) makes the next onFocus catch-up write lastHeartbeatTime = now. Tracker order is onFocus then async startNewSession, so max(lastUnfocusTime, lastHeartbeatTime) ends the previous session at return time and the background gap is included. (Opus, Grok)

Consider

  • reset() / resetAndClearStoredRecord() do not retire an in-flight checkHeartbeat; it can persist the old record and enqueue on the new app-id queue. (3/3; rare)
  • Consent revoke while foregrounded: tracker skips onUnfocus, so the timer keeps running and can count background time. (Opus)
  • Close the interval / bump generation before pinnableIds() in onUnfocus so a concurrent due check cannot write a later wall time. (Sol)

Noted

  • testHeartbeatRepeatsEveryThirtyMinutesInTheForeground cannot tell two fires from one coalesced late fire. SwiftLint file_length warning at 416 lines.

Dismissed

  • Generation not bumped in the same lock as record = newRecord: the window is only until restartHeartbeat(), and a new record’s duration is ~0 on the usual focus-then-session-start path.
Open in Web View Automation 

Sent by Cursor Automation: PR Reviews

if var previous, previous.usesSessionsApi {
// A session killed in the foreground never added its last visit to the duration or
// recorded an unfocus. It ends with its last heartbeat, or else at its start.
let endTime = max(previous.lastUnfocusTime ?? previous.startTime, previous.lastHeartbeatTime ?? 0)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Catch-up on the next onFocus can set lastHeartbeatTime to the return wall time when activeDuration is already past due (a visit that crossed 30 minutes was folded in onUnfocus and the late asyncAfter was generation-invalidated). max then prefers that return time over lastUnfocusTime, so end_time includes the whole background gap.

Only let lastHeartbeatTime win when lastHeartbeatDuration > activeDuration (killed in the foreground), or write the heartbeat in onUnfocus when the visit crosses the interval.

func onFocus() {
let now = monotonicNow()
stateLock.withLock { focusedAt = now }
restartHeartbeat()

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Tracker calls [OSSessionService onFocus] before async startNewSession. This restartHeartbeat → checkHeartbeat therefore runs against the outgoing record. If that record is already past due, this is what stamps lastHeartbeatTime to now — the value the endTime max above then prefers.

@abdulraqeeb33 abdulraqeeb33 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Requesting changes.

This branch forked before the operation-repo rewrite. Its merge-base with current #1772 is 65000d08. It is missing the operation repo, the Identity Verification exemption, dropping users that will never be created, and foreground/network retry. It still calls OSSessionRequestQueue, which #1772 deleted.

Rebase onto current #1772 and enqueue the heartbeat through enqueueSessionUpdate. The 30-minute foreground timer and the end that uses max(activeDuration, lastHeartbeatDuration) are the right shape. Android #2782 already does that on a clean parent.

}
if let heartbeat = next.heartbeat {
OneSignalLog.onesignalLog(.LL_DEBUG, message: "OSSessionService heartbeat with duration \(heartbeat.activeDuration)")
requestQueue()?.enqueueUpdate(record: heartbeat)

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This still enqueues through OSSessionRequestQueue. Current #1772 deleted that queue and sends updates through enqueueSessionUpdate. Rebase onto that branch and call enqueueSessionUpdate here.

@fadi-george

Copy link
Copy Markdown
Collaborator Author

Closing along with #1772, the session queue PR (#1768) was redone on top of #1775. Will reopen on top of it.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants