Skip to content

feat: [SDK-5391] send a session end when a sessions API session ends - #1782

Open
fadi-george wants to merge 6 commits into
new-session-eventsfrom
fadi/SDK-5391-ios-session-end
Open

fadi-george wants to merge 6 commits into
new-session-eventsfrom
fadi/SDK-5391-ios-session-end

Conversation

@fadi-george

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

Copy link
Copy Markdown
Collaborator

Description

One Line Summary

When the sessions API flag is on, queue a session end with end_time and the final duration when the next session starts.

Details

Motivation

Part of the iOS SDK Sessions stack (SDK-5391). Ended sessions need a PATCH /apps/{app_id}/sessions/{session_id} with end_time and the final cumulative duration_seconds. Builds on #1768 and #1779. Replaces #1772, which was based on the earlier session queue. Android counterpart: OneSignal/OneSignal-Android-SDK#2781.

Scope

  • iOS has no background job to end a session, so a session ends when the next one starts (resume after more than 30 s, or cold start).
  • OSSessionService.startNewSession now returns OSSessionStart(started:ended:). The replaced record is filled with any IDs assigned since and handed back under the same lock that replaces it, so the session is not lost.
  • OneSignalUserManagerImpl.startNewSession queues the ended session's update before the new session's create.
  • end_time is the later of the last unfocus and the last focus, saved in new optional OSSessionRecord.lastUnfocusTime and lastFocusTime fields (records stored by earlier versions still decode). Like Android, a session killed in the foreground ends at its last focus. The focus that starts a new session is reported before the session starts, so that visit counts toward the new session and does not move the previous one's end.
  • Identity models are not cached across launches, so a session whose user was replaced by a login in an earlier process had its requests dropped. Its identity model is now restored from the session's pinned onesignal_id. A session with no pinned onesignal_id was never created, so it is still dropped.
  • End replaces unsent updates for the session, carries the highest queued duration, and blocks later updates, through the existing logic in OSSessionOperationExecutor. No executor changes.
  • Only sessions whose captured usesSessionsApi is true are ended this way. Flag off: nothing new is sent, and the legacy focus-time processors are unchanged.

Things to note

  • The end is sent at the next app open, which can be long after the session ended. Android sends it from a background job after the timeout. A best-effort end before suspension is tracked in SDK-5596.
  • Foreground time from a visit cut short by a kill is not counted, same as before, and the end carries the last unfocus. The heartbeat and background checkpoint tickets cover that.
  • end_time and start_time are both wall clock, so a clock moved back mid-session could produce end_time before start_time.

Testing

Unit testing

  • OSSessionServiceTests: unfocus records the wall clock time; the first session ends nothing; a new session returns the previous one with its final duration and last unfocus; a session killed in the foreground ends at its start, or at its last focus after returning to the foreground; a resume past the threshold and a cold start do not move the previous session's end; a session from an earlier process ends with the IDs assigned since.
  • SessionRecordUserTests: the previous session's end is queued before the next create; a legacy session queues no end; the end of a session whose user is no longer loaded is queued for its pinned user.

OSSessionServiceTests, SessionRecordUserTests, and OSSessionOperationExecutorTests pass locally. One run failed testNewSessionQueuesNoCreateWhenTheFlagIsOff, then the same 50 tests passed on 2 reruns, so it looks flaky. SwiftLint reports no new violation types.

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

… starts

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.

The end of the current user looks right. It is the record just replaced, queued before the next create, and a flag-off session is not ended.

Two ends are lost. The restored user does not survive a relaunch, and a record saved before lastUnfocusTime is ended at its start even when it already has foreground time.

Comment thread iOS_SDK/OneSignalSDK/OneSignalOSCore/Source/OSSessionService.swift Outdated
Comment thread iOS_SDK/OneSignalSDK/OneSignalUserTests/SessionRecordUserTests.swift Outdated
fadi-george and others added 4 commits October 22, 2026 12:26
Matches Android, which ends it at max(unfocus, focus) time. The focus that starts a new
session is reported before the session starts, so that visit does not move the previous
session's end.

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

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… across a kill

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
… unfocus and duration

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.

The identity restore is right. identityModel(for:) re-registers the model the delta already holds, and the queued-end test now checks the real unfocus and duration.

onFocus still persists the next session's focus onto the current record. A kill before startNewSession ends the previous session at that focus.

return
}
// Saved now, since a kill leaves no unfocus to record when the session was last active.
record?.lastFocusTime = wallNow

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 writes lastFocusTime onto the loaded record and persists it. The previous focus stays only in openVisit, and startNewSession is what puts it back. The tracker calls onFocus and then startNewSession, which is dispatched onto the main queue (OneSignalTracker.m:83, OneSignal.m:379). A kill in that gap leaves the next session's focus on the previous record. The next launch has no openVisit, so endTime takes that focus over lastUnfocusTime.

Keep the boundary focus in openVisit until startNewSession writes it onto the new record. Add a test that focuses on one instance and starts the session on a fresh one.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Fixed in 615c235. onFocus now only saves the focus on the record when it continues that session (record started in this process and back within 30 s of the last unfocus). Any other focus stays in memory until startNewSession puts it on the new record, which also covers a cold start and a skipped session start (e.g. consent pending). Added tests that focus on one instance and start the session on a fresh one, for both a resume past 30 s and a cold start.

/// Seconds since 1970. A session killed in the foreground never unfocused, so it ends at its
/// last focus. The visit the kill cut short is not in the duration either.
public var endTime: TimeInterval {
max(startTime, lastUnfocusTime ?? startTime, lastFocusTime ?? startTime)

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.

A record saved before these timestamps, with activeDuration already greater than 0 and both optionals nil, still ends at its start. When both are nil, end at startTime + activeDuration.

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

Same as #1782 (comment): no saved record predates these fields since OSSessionRecord hasn't shipped. activeDuration also only grows in onUnfocus, which sets lastUnfocusTime in the same step, so duration > 0 with both nil can't happen. Leaving as is.

…t record

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
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