Skip to content

Fix concurrent Unity callback routing - #1008

Draft
vegaro wants to merge 2 commits into
cesar/ios-callback-request-idsfrom
cesar/fix-unity-callbacks
Draft

Fix concurrent Unity callback routing#1008
vegaro wants to merge 2 commits into
cesar/ios-callback-request-idsfrom
cesar/fix-unity-callbacks

Conversation

@vegaro

@vegaro vegaro commented Jul 24, 2026

Copy link
Copy Markdown
Member

Motivation

Purchases stores one mutable callback per operation. Two overlapping calls can overwrite each other, so the first response invokes the wrong callback and the other response is dropped.

Description

Correlate every one-shot callback with the request IDs added by the earlier PRs in this stack.

  • Register callbacks by request ID before calling the native wrapper.
  • Remove and invoke only the callback matching each response.
  • Keep duplicate and unknown responses from invoking callbacks twice.
  • Update the existing callback fixtures to use the same request-ID response envelope as the native bridges.
  • Keep the existing malformed storefront logging behavior.

No public API changes in this final layer.

Testing

  • Full EditMode suite: 145/145 passed
  • Unity package export and import passed
  • ./scripts/check-meta-files.sh passed
  • ./scripts/check-android-keep-annotations.sh passed

Physical-device concurrency smoke tests were not run.

Stack

  1. Add callback request registry #1066
  2. Pass Unity callback request IDs through Android #1067
  3. Pass Unity callback request IDs through iOS #1068
  4. This PR

@vegaro

vegaro commented Aug 6, 2026

Copy link
Copy Markdown
Member Author

Heads-up from #1019: _getCurrentOfferingForPlacement carries an ordering bug that this branch preserves, and it gets worse under request-ID correlation.

RevenueCat/Scripts/Purchases.cs:1682 checks for a missing offering before ResponseHasError. Both native wrappers send only an error key on failure — there is no offering key in that payload — so every failure returns early via callback(null, null) and the error is dropped. A network failure is indistinguishable from "no offering is configured for this placement".

Suggested change, in this branch's shape:

private void _getCurrentOfferingForPlacement(string offeringJson)
{
    var response = JSON.Parse(offeringJson);
    if (!TryTakeCallback(response, out GetCurrentOfferingForPlacementFunc callback) || callback == null)
    {
        return;
    }

    // The error check has to come first: both native wrappers send only an "error" key on
    // failure, so testing for a missing "offering" first reports every failure as
    // "no offering configured for this placement".
    if (ResponseHasError(response))
    {
        callback(null, new Error(response["error"]));
        return;
    }
    if (response["offering"] == null)
    {
        callback(null, null);
        return;
    }
    callback(new Offering(response["offering"]), null);
}

#1019 makes this exact change against main and has a regression test for it (GetCurrentOfferingForPlacementDeliversNativeError), so whichever lands second will need the other's version. Not pushing it here since this branch is checked out in another workspace and currently shows conflicts with main.

@vegaro
vegaro force-pushed the cesar/fix-unity-callbacks branch 2 times, most recently from 68246df to 4fc0292 Compare September 1, 2026 17:14
@vegaro
vegaro changed the base branch from main to cesar/ios-callback-request-ids September 1, 2026 17:16
@vegaro
vegaro force-pushed the cesar/fix-unity-callbacks branch from 4fc0292 to 42913c8 Compare September 2, 2026 11:05
@vegaro
vegaro force-pushed the cesar/fix-unity-callbacks branch from 42913c8 to 1ed37b2 Compare September 2, 2026 12:28
@vegaro
vegaro force-pushed the cesar/fix-unity-callbacks branch from 1ed37b2 to 3a7f6d8 Compare September 2, 2026 14:35
@vegaro
vegaro force-pushed the cesar/fix-unity-callbacks branch from 3a7f6d8 to 0ca00b9 Compare September 2, 2026 16:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

pr:fix A bug fix

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant