Skip to content

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

Merged
fadi-george merged 3 commits into
new-session-eventsfrom
fadi/SDK-5397-ios-heartbeat
Oct 29, 2026
Merged

fadi-george merged 3 commits into
new-session-eventsfrom
fadi/SDK-5397-ios-heartbeat

Conversation

@fadi-george

@fadi-george fadi-george commented Oct 27, 2026 •

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. Replaces #1773, which was closed when its base merged. Android counterpart: OneSignal/OneSignal-Android-SDK#2782.

Scope

  • OSSessionService runs the heartbeat while a sessions API session is in the foreground. It starts when a session starts (after startNewSession's enqueue closure queues the create, since an update before its create is discarded) or when a focus continues the current session, and stops on unfocus. A focus that will start a new session leaves the heartbeat to that session. Flag off: no timer.
  • Counts foreground time only: a heartbeat is due once the session's duration (including the open visit, on the monotonic clock) is 30 minutes past the last heartbeat's. Each scheduled check recomputes the remaining time, and a generation counter makes stale checks do nothing. Durations only increase, so an unchanged duration is never sent.
  • The service hands the due record to the User module through a new OSSessionUserProvider.sessionHeartbeat(_:), which enqueues an update delta without end_time. The executor's existing coalescing folds unsent heartbeats and discards them once the end is queued. No executor changes.
  • New optional OSSessionRecord.lastHeartbeatDuration / lastHeartbeatTime. An app killed in the foreground never adds its last visit to activeDuration, so updates now send reportedDuration (at least the last heartbeat's duration), and endTime also considers the last heartbeat time. That time is the visit's focus time plus its monotonic duration, so a wall-clock jump can't move a killed session's end.
  • The interval is hardcoded to 30 minutes. It matches Android's HEARTBEAT_INTERVAL_MS, and the two should change together.

Testing

Unit testing

Added OSSessionHeartbeatTests on a virtual clock: one heartbeat after 30 minutes with its time recorded, two after 60, a short background pauses the interval, a new session restarts it, a focus that starts a new session doesn't heartbeat the old one, a new session's heartbeat waits for its create, a pending check after unfocus does nothing, a late check reports the full duration, no timer with the flag off, and a kill after a heartbeat (including after a clock jump) ends with the heartbeat's duration and time. SessionRecordUserTests checks that the heartbeat and end deltas carry at least the last heartbeat's duration.

OneSignalOSCore tests pass locally. In OneSignalUserTests, ExecutorRetryBackoffTests.testACreateUserRejectedAfterANewSessionStartedIsResentRightAway fails before and after this round of changes, and UserConcurrencyTests.testSubscriptionExecutorConcurrency is intermittent. SwiftLint: no new violations beyond OSSessionService.swift growing past the file_length warning it was already near.

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

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@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 (Opus 5.5, GPT 5.6 Sol, Grok 4.7)

Intent: enqueue a duration-only session update every 30 minutes of foreground time so sessions-API sessions stay open and a foreground kill loses little duration.

Opus and Grok independently flagged overlapping issues. GPT 5.6 Sol reported no findings. No criticals; the generation counter and executor coalescing look sound on the normal tracker resume path.

Act on

  1. Heartbeat vs create ordering. startNewSession swaps the record, unlocks, then restartHeartbeat bumps heartbeatGeneration and may run checkHeartbeat synchronously. A due check can call sessionHeartbeat before enqueueSessionCreate. admitUpdate then drops that update (“no create or server session ID”) after lastHeartbeatDuration has already advanced, so the next send waits a full interval. Bump generation in the same lock as the record swap, and do not send a heartbeat until the create is queued.
  2. No focusedAt → no heartbeat. Heartbeats require focusedAt != nil, which only onFocus() sets. Consent-gated OneSignalTracker.onFocus returns before OSSessionService.onFocus, and init while already active may never get another become-active. That first long foreground visit then gets no heartbeats — the case this feature is meant to protect. Seed focus when a session starts and the app is already active (same idea as StartFeatureFlagsRefresh).

Consider

  • The interval is foreground time, not wall-clock. Short interruptions stretch time between updates; confirm that still satisfies the backend keep-alive (including repo poll + retry).
  • endTime takes max(..., lastHeartbeatTime). A forward clock jump stored on a heartbeat can outrank a later unfocus and send a future end_time.
  • A missed unfocus (e.g. consent revoke) leaves the chain running and can persist background time into reportedDuration / endTime.
  • resetAndClearStoredRecord does not invalidate an in-flight instance; a global-queue persist() can rewrite the old record after an app-id change.

Noted

  • Protocol says sessionHeartbeat is off-main; the first checkHeartbeat is synchronous on the caller (often main).
  • Stale asyncAfter blocks still call pinnableIds() before the generation guard.
  • Tests cover the virtual-clock happy path; they miss overdue-on-focus, heartbeat-before-create, and concurrent unfocus.
Open in Web View Automation 

Sent by Cursor Automation: PR Reviews

Comment thread iOS_SDK/OneSignalSDK/OneSignalOSCore/Source/OSSessionService.swift
Comment thread iOS_SDK/OneSignalSDK/OneSignalOSCore/Source/OSSessionService.swift Outdated
Comment thread iOS_SDK/OneSignalSDK/OneSignalOSCore/Source/OSSessionService.swift
Start a new session's heartbeat only after its create is queued, stop it
on reset, and drop the heartbeat time from the end once the app leaves
the foreground after it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

@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.

Follow-up pass on the heartbeat, reviewed with a three model fan-out (Opus, Grok, GPT), with each finding checked against the code before writing it up.

Since the last round: the create-ordering race is genuinely fixed, and clearing lastHeartbeatTime on unfocus closes the wall-clock path raised on endTime. The generation counter holds up. I traced every bump against the stale-callback windows, and the one interleave that can reorder sends is absorbed by admitUpdate's max-fold.

Two things worth fixing before this lands, and both are consequences of where the last round's fixes landed rather than new regressions:

  1. Clearing lastHeartbeatTime on unfocus fixed the unfocus path, but the kill path it deliberately carved out still carries an unclamped wall-clock sample.
  2. Moving the arming into the User module left startHeartbeat() as a single production line that no test covers.

Everything else is a nit or a residual risk, noted inline.

Comment thread iOS_SDK/OneSignalSDK/OneSignalOSCore/Source/OSSessionService.swift
Comment thread iOS_SDK/OneSignalSDK/OneSignalUser/Source/OneSignalUserManagerImpl+Session.swift Outdated
Comment thread iOS_SDK/OneSignalSDK/OneSignalOSCore/Source/OSSessionService.swift
Comment thread iOS_SDK/OneSignalSDK/OneSignalOSCore/Source/OSSessionService.swift
Comment thread iOS_SDK/OneSignalSDK/OneSignalOSCore/Source/OSSessionService.swift Outdated
Comment thread iOS_SDK/OneSignalSDK/OneSignalOSCore/Source/OSSessionService.swift
Comment thread iOS_SDK/OneSignalSDK/OneSignalOSCore/Source/OSSessionService.swift
- Time a heartbeat from the visit's focus on the monotonic clock, so a
  wall-clock jump can't move a killed session's end.
- startNewSession takes the enqueue as a closure and starts the
  heartbeat after it, so there's no second call to forget.
- Log when a heartbeat starts, with the session ID.
- Cover a stale check after unfocus, a late check, and a kill after a
  clock jump.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@fadi-george
fadi-george merged commit 45d857b into new-session-events Oct 29, 2026
4 checks passed
@fadi-george
fadi-george deleted the fadi/SDK-5397-ios-heartbeat branch October 29, 2026 00:48
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.

3 participants