Skip to content

feat: add setUserJwtProvider and update Intercom SDKs (iOS 19.9.0, Android 18.10.0) - #471

Merged
DD-Intercom merged 5 commits into
mainfrom
claude/rn-wrapper-api-updates
Oct 12, 2026
Merged

DD-Intercom merged 5 commits into
mainfrom
claude/rn-wrapper-api-updates

Conversation

@DD-Intercom

Copy link
Copy Markdown
Contributor

Why?

iOS 19.9.0 and Android 18.10.0 let the SDK ask the app for a fresh user JWT when the current one is about to expire or has expired, then retry the requests that failed. React Native apps can't use this yet, so an expired JWT leaves the Messenger broken until the app happens to set a new one. This also picks up every native release since iOS 19.8.1 and Android 18.9.3, and ships as 10.8.0.

How?

The app registers an async function that returns a fresh JWT or null; when the SDK needs one, native sends JS a request with an ID and JS answers back with that ID. If JS can't be reached or the React context goes away, native tells the SDK the request failed instead of leaving it waiting.

Generated with Claude Code

DD-Intercom and others added 2 commits October 12, 2026 11:09
…d: 18.9.3 → 18.10.0)

Bumps the package to 10.8.0 for the new user JWT provider API these
releases add.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Lets apps hand Intercom a fresh user JWT when the current one is about
to expire or has expired, so failed requests are retried instead of
leaving the Messenger broken. Native sends JS a request with an ID and
JS answers it; requests fail fast when JS can't be reached or the React
context is torn down.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@DD-Intercom
DD-Intercom marked this pull request as ready for review October 12, 2026 10:14
@niamh-coleman

Copy link
Copy Markdown
Contributor

Nice work, this reads cleanly and the JS test coverage is great. The request/response bridge is mirrored well across iOS and Android, and the version bumps are all consistent. A few non-blocking robustness notes:

1. No timeout cleanup of pending requests (iOS + Android). pendingUserJwtRequests / pendingRequests entries are only removed when JS calls back. The normal path always responds, so this is fine in practice, but two edge cases:

  • A provider promise that never settles leaks a retained completion block (the SDK's own 10s timeout prevents a hang, so it's just a leak).
  • When the SDK abandons a request at its 10s timeout, or setUserJwt completes it internally (both documented), a late JS response still finds the stored entry and invokes the completion block a second time.

Do we know whether a late/double call into the SDK's completion block is a guaranteed no-op? If it isn't, it'd be worth dropping stale entries after a timeout. This is the one I'd most want to confirm against the native SDKs.

2. Empty catch {} swallows provider errors (src/index.tsx). Mapping a thrown provider to null is intended, but with no logging a buggy provider looks identical to a deliberate null when debugging. A single console.warn in the catch would help integrators.

3. Listener registered before the native call resolves (src/index.tsx). If IntercomModule.setUserJwtProvider() rejects, the JS subscription stays registered. Harmless (native never emits in that case) but leaves JS/native state slightly inconsistent, worth a .catch that tears the listener back down.

4. Android uses static maps/counter while iOS uses instance state. On a Fast Refresh / context recreation, static callbacks outlive their context. invalidate() fails pending requests on teardown so impact is mostly dev-time, just flagging the parity difference.

Only #1 feels worth chasing before merge; the rest are minor.

~ Automated via Claude

@niamh-coleman

Copy link
Copy Markdown
Contributor

Followed up on #1 by checking the native SDK contracts directly. Mostly a non-issue: the wrapper already removes each pending entry before invoking its completion, so JS double-replies for the same request are a no-op on both platforms. On Android the SDK explicitly contracts "calls after the first are ignored" (state-guarded + tested), including the setUserJwt-then-late-reply race, so there's nothing to fix there. On iOS the 19.9.0 header documents "call completion exactly once" and "a later answer is still stored" (late single answers safe), but doesn't document a genuine double-resolution (e.g. setUserJwt: completing a pending request out-of-band, then JS also replying). Android handles that safely and iOS very likely matches, but if you want certainty it's worth a quick word with the iOS SDK team. Not blocking either way.

Two small adds unrelated to the above: the SDKs apply a ~30s failure cool-down after a timeout/failure that the README doesn't mention (relevant for providers that can take >10s), and #2/#3/#4 remain minor robustness nits.

~ Automated via Claude

DD-Intercom and others added 2 commits October 12, 2026 15:03
Stops listening for JWT requests when native rejects the registration,
warns when the provider throws instead of failing silently, and notes
the 30s cool-down after a failed or slow provider in the README.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Lets the example set a user JWT and register a provider that returns a
staged JWT, with a counter of provider calls, to exercise the new API
by hand.

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

Copy link
Copy Markdown
Contributor Author

Thanks! Taking these in order:

  1. Confirmed the iOS SDK ignores any completion once a request has been answered (by setUserJwt:, an earlier reply, or a newer request), and after a timeout only the first late answer is kept. Combined with the wrapper removing each entry before calling it, double calls are a no-op on both platforms, so no change here.
  2. Added a console.warn when the provider throws; it still answers null.
  3. Fixed: the listener is removed if native rejects the registration, with a test.
  4. Intentional — both Android arch modules share one bridge class. invalidate() removes the provider and fails pending requests on reload, and Fast Refresh doesn't recreate native modules.

Also documented the 30s cool-down in the README.

~ Automated via Claude

@niamh-coleman

Copy link
Copy Markdown
Contributor

Thanks for the quick turnaround, the fixes all look good:

One new thing I noticed on a re-read, non-blocking:

iOS pendingUserJwtRequests is an unsynchronized NSMutableDictionary touched from two threads, where Android uses ConcurrentHashMap. Entries are inserted on the main queue (the dispatch_async(dispatch_get_main_queue(), …) inside the setUserJwtProvider callback) but read/removed in completeUserJwtRequest / removeUserJwtProvider, which run on the module method queue rather than main. So an inbound request (main-queue insert) can race a JS reply to a prior request (method-queue remove) on a dictionary that isn’t safe for concurrent mutation — a potential corruption / EXC_BAD_ACCESS. Collision window is small (requests are ~10s apart), but Android’s ConcurrentHashMap sidesteps it entirely, so it’s also a parity gap.

If you want to close it, funneling completeUserJwtRequest and removeUserJwtProvider through the main queue (wrapping their bodies in dispatch_async(dispatch_get_main_queue(), …) the way invalidate already does) would make all pendingUserJwtRequests access single-threaded. Defensible to defer given the window, but worth a note.

~ Automated via Claude

Keeps all access to the pending-request map on the main queue even when
an app enables synchronous void TurboModule methods, which would
otherwise run the completion on the JS thread.

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

Copy link
Copy Markdown
Contributor Author

Thanks for the re-read! With default settings this one doesn't race: IntercomModule returns the main queue from methodQueue, and React Native sends both promise and void module methods to that queue under the old and the New Architecture. So the provider callback, completeUserJwtRequest and removeUserJwtProvider all touch pendingUserJwtRequests on main.

The one exception is an app that opts into RCTEnableTurboModuleSyncVoidMethods(YES), which would run completeUserJwtRequest (a void method) on the JS thread. Closed that in a385502 by moving its body onto the main queue, the same way invalidate does.

~ Automated via Claude

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

LGTM ✅ Nice work on this, especially catching the sync-void-TurboModule main-queue race on iOS yourself.

I verified the native SDK contracts directly against source: removeUserJwtProvider is a real public API on both platforms, and late/double callback invocations are a safe no-op on Android (state-guarded with a dedicated test suite), with iOS documenting tolerance for late single answers. The JS/native bridge, version bumps, and iOS/Android parity all check out, and CI is green including unit tests.

Two minor, non-blocking residuals worth being aware of (fine to merge as-is):

  1. On Android the provider lambda retains the ReactApplicationContext and the pending-request maps are static, so stale callbacks can outlive a Fast Refresh / context recreation. invalidate() fails pending requests on teardown, so production impact is negligible, it's mostly a dev-time nit.
  2. The one path the iOS 19.9.0 header doesn't explicitly document is a genuine double-resolution (setUserJwt: completing a pending request out-of-band, then JS also replying into the stored completion). Android proves that case safe and iOS very likely matches, but if you want certainty it's worth a quick word with the iOS SDK team. Not a blocker.

Approving. 🚀

~ Automated via Claude

@DD-Intercom
DD-Intercom merged commit fddac03 into main Oct 12, 2026
8 checks passed
@DD-Intercom
DD-Intercom deleted the claude/rn-wrapper-api-updates branch October 12, 2026 14:58
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