Skip to content

fix: notify callers when a Rokt placement cannot be requested - #814

Merged
jamesnrokt merged 1 commit into
mainfrom
fix/rokt-select-placements-missing-callback
Aug 14, 2026
Merged

fix: notify callers when a Rokt placement cannot be requested#814
jamesnrokt merged 1 commit into
mainfrom
fix/rokt-select-placements-missing-callback

Conversation

@jamesnrokt

Copy link
Copy Markdown
Collaborator

Background

  • MPRokt.selectPlacements:attributes:embeddedViews:config:onEvent: resolves the dashboard attribute mapping via getRoktPlacementAttributesMapping before forwarding to the Rokt kit. That method returns nil whenever the Rokt kit configuration is not present in MPKitContainer.originalConfig — most commonly because remote configuration has not been applied yet, and also when the kit is not enabled for the workspace.
  • In that case the SDK logged a warning and returned. The caller's onEvent handler was never invoked, so partners waiting on a placement lifecycle event (loading indicator, spinner, checkout continuation) were left waiting on a callback that would never arrive, with nothing to distinguish "not configured" from "still loading".
  • MPRokt.selectShoppableAds:attributes:config:onEvent: had the same behaviour on the same code path.

What Has Changed

  • Both selectPlacements and selectShoppableAds now deliver RoktEvent.PlacementFailure (RoktPlacementFailure, carrying the requested placement identifier) to the caller's onEvent handler when the Rokt kit configuration is unavailable. The existing warning log is unchanged.
  • The event is delivered on the main queue, since callers drive UI from this handler. Nothing is delivered when onEvent is nil (including the convenience selectPlacements:attributes: overload).
  • RoktPlacementFailure is the event the Rokt SDK already emits when a placement cannot be displayed, so partner event handlers need no new cases.
  • No API surface change: this is behaviour on an existing callback.
  • Added unit tests covering the placement-failure callback for both entry points, a nil-onEvent regression test, and a CHANGELOG entry.

Scope

  • This PR only guarantees that the caller is always notified. It intentionally does not change when the call is dropped. A related gap remains open and is not addressed here: MPKitContainer.forwardSDKCall discards a forwarded call when the Rokt kit is registered and configured but has not finished starting, since the kit only sets started from the asynchronous Rokt.init callback.

Checklist

  • I have performed a self-review of my own code.
  • I have made corresponding changes to the documentation.
  • I have added tests that prove my fix is effective or that my feature works.
  • I have tested this locally.

Additional Notes

  • Partners who currently treat "no callback" as an implicit failure signal will now receive an explicit RoktPlacementFailure. This is additive, but it is the first time this code path emits an event at all, so it is worth calling out in release notes.

Reference Issue (For employees only. Ignore if you are an outside contributor)

@jamesnrokt
jamesnrokt requested a review from a team as a code owner August 14, 2026 01:47
@cursor

cursor Bot commented Aug 14, 2026

Copy link
Copy Markdown

PR Summary

Low Risk
Localized callback behavior in MPRokt with no API changes; partners may see a new failure event where none fired before.

Overview
When Rokt placement attribute mapping is missing (kit not configured or remote config not applied), MPRokt.selectPlacements and MPRokt.selectShoppableAds no longer end with only a warning log. They invoke a new notifyPlacementFailure:onEvent: helper that delivers RoktPlacementFailure with the requested placement identifier on the main queue when the caller supplies an onEvent handler; nil onEvent is unchanged (no event, no crash).

Existing kit forwarding and warning logs are unchanged. Unit tests cover both entry points plus nil-onEvent; CHANGELOG documents the core fix.

Reviewed by Cursor Bugbot for commit 28e0bd4. Bugbot is set up for automated code reviews on this repo. Configure here.

@jamesnrokt
jamesnrokt force-pushed the fix/rokt-select-placements-missing-callback branch from aedede1 to 3ac1f6e Compare August 14, 2026 01:49
@github-actions

github-actions Bot commented Aug 14, 2026

Copy link
Copy Markdown

📦 SDK Size Impact Report

Measures how much the SDK adds to an app's size (with-SDK minus without-SDK).

Metric Target Branch This PR Change
App Bundle Impact 1.79 MB 1.79 MB +N/A
Executable Impact 848 bytes 848 bytes +N/A
XCFramework Size 6.49 MB 6.49 MB +N/A

➡️ SDK size impact change is minimal.

Raw measurements

Target branch (main):

{"baseline_app_size_kb":84,"baseline_executable_size_bytes":75464,"with_sdk_app_size_kb":1920,"with_sdk_executable_size_bytes":76312,"sdk_impact_kb":1836,"sdk_executable_impact_bytes":848,"xcframework_size_kb":6648}

This PR:

{"baseline_app_size_kb":84,"baseline_executable_size_bytes":75464,"with_sdk_app_size_kb":1920,"with_sdk_executable_size_bytes":76312,"sdk_impact_kb":1836,"sdk_executable_impact_bytes":848,"xcframework_size_kb":6648}

@BrandonStalnaker BrandonStalnaker left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

solid improvement

Comment thread mParticle-Apple-SDK/MPRokt.m
selectPlacements and selectShoppableAds returned without invoking the
caller's onEvent handler when the Rokt kit configuration had not been
applied, leaving partners waiting on a callback that never arrived.

Deliver RoktPlacementFailure on the main queue in that case, matching the
event the Rokt SDK emits when a placement cannot be displayed.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
@jamesnrokt
jamesnrokt force-pushed the fix/rokt-select-placements-missing-callback branch from 3ac1f6e to 28e0bd4 Compare August 14, 2026 18:49
@jamesnrokt
jamesnrokt merged commit 9825170 into main Aug 14, 2026
226 of 229 checks passed
@jamesnrokt
jamesnrokt deleted the fix/rokt-select-placements-missing-callback branch August 14, 2026 19:21
@cursor cursor Bot mentioned this pull request Aug 14, 2026
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