feat(android): opt-in signing-URL launch strategy for presentCaptiveSigning - #4
Merged
Merged
Conversation
…ning The SDK's fetch-based launchCaptiveSigning downloads the envelope with include=documents before opening the ceremony. That download runs on a read timeout derived from the envelope size which floors at 15s when nothing is cached, so a large envelope on a slow connection exhausts it and the ceremony never opens. The SDK also exposes a signingURL overload that skips the download. Minting a recipient view needs only the recipient details already passed to presentCaptiveSigning plus the session credentials, so opting into launchStrategy 'signingUrl' takes the timing-out call out of the code path. Opt-in, defaulting to 'fetch', because the mint spends the consumer's bearer token on an endpoint that token is not guaranteed to be scoped for. A mint failure falls back to the fetch path, so the strategy can only add a way to succeed. Verified on device across 12 consecutive ceremonies. Two hazards the async hop introduces, both guarded: Minting puts a network round trip between capturing the Activity and using it, which the fetch path never did. canLaunchOn refuses to launch against a finishing Activity, and checks the pending completion by identity rather than nullness. reset() and endSigningSession() clear that slot and a fresh call can claim it while a mint is still in flight; a nullness check would pass in that window and launch this envelope wired to the new session's promise. Session credentials move into a single DocuSignSession reference so a login racing an in-flight mint cannot tear the triple and build a request with one session's token and another's account id.
Four cases on the hook: signingUrl and an explicit fetch both reach the native call unchanged, the url flow never carries the field, and omitting it produces a payload with no launchStrategy key at all rather than one set to undefined. That last one is the backward-compatibility guarantee under test. The hook spreads the field in conditionally so an existing consumer who never opts in sends the exact payload previous releases sent, and the test asserts the key is absent rather than merely undefined. README gains an Android launch strategies section covering the timeout this addresses, the token scope the mint needs, and a pointer to presentCaptiveSigningWithUrl as the better shape when the backend can mint.
IronTony
force-pushed
the
feat/android-signing-url-launch-strategy
branch
from
August 27, 2026 07:24
76bbd43 to
1d2f138
Compare
IronTony
marked this pull request as ready for review
August 27, 2026 07:25
IronTony
marked this pull request as draft
August 27, 2026 07:27
IronTony
force-pushed
the
feat/android-signing-url-launch-strategy
branch
from
August 27, 2026 07:27
1d2f138 to
76bbd43
Compare
Everything here touches code that predates #3. It sat on that PR's review as requested changes, which was wrong of me: asking a contributor to refactor the maintainer's own code as a condition of landing their feature is scope creep. Pulled off #3 and landed here instead. Both entrypoints carried their own copy of a 33-line DSCaptiveSigningListener, so any callback fix had to be made twice and the copies could drift. captiveSigningListener() is now the single source, and presentCaptiveSigningWithUrl folds onto the same launchWithSigningUrl helper the signing-URL strategy uses. #3's https/host guard becomes isHttpsUrl and now covers every URL this object hands to the SDK or sends credentials to. It stays ahead of the compareAndSet in presentCaptiveSigningWithUrl: a rejected URL must not claim the pending slot, or the next valid call is refused as already in progress. That guard also now covers the recipient-view request itself. host arrives from JS unvalidated and that request carries the session bearer token, so an http:// host would have sent it in cleartext. The signing-URL strategy is the first code path to treat host as an endpoint rather than passing it to the SDK, so the exposure comes in with this feature. The check runs before the connection is opened and before the Authorization header is set, and throwing routes to the fetch fallback, so a misconfigured host degrades instead of failing outright. launchViaEnvelopeFetch's catch now clears currentEnvelopeId. Only the URL path did, and #3 had the correct shape. restApiRoot is lifted out of the request builder. Its ordering is load-bearing, since an already-versioned host would otherwise fall through and get a second /restapi/v2.1 appended, and that reads better as a named function than buried in a connection setup. Both branches are now case-insensitive; an uppercase RESTAPI segment previously fell through and built a doubled path. Not unit tested: android/build.gradle declares implementation project(':expo-modules-core'), which only resolves inside a host app, so there is no standalone gradle build and CI runs jest only. Error codes are the behaviour change. CodedException infers a code from the class name when none is given, so forwarding it verbatim would have surfaced NotInitializedException as ERR_NOT_INITIALIZED rather than the not_initialized the README error table has always documented. The exceptions now carry explicit codes and the module forwards them, so callers can tell a missing initialize() from a missing login from a real signing failure instead of receiving signing_failed for all three. The module also stops emitting onSigningError itself. handleSigningError already emits, so every failure delivered two events, and the module's copy flattened recipient_signing_failed into signing_failed. The emit in loginWithAccessToken stays: that path never reaches handleSigningError, so it is the only event.
The Throws entries for both present methods claimed signing_failed covered a missing initialize() and a missing login. That was true when the module hard-coded that code; it is not now that the real code is forwarded, so both entries name the specific codes instead. CHANGELOG gains the launch strategy, the URL validation and the currentEnvelopeId fix, plus a Breaking changes section. The error codes and the single onSigningError event are both observable behaviour changes: anyone matching on signing_failed to detect an uninitialised SDK, or counting error events, will see different results after upgrading. That belongs above the fold rather than inferred from a feature bullet.
IronTony
force-pushed
the
feat/android-signing-url-launch-strategy
branch
from
August 27, 2026 16:46
76bbd43 to
7e6e9e4
Compare
IronTony
marked this pull request as ready for review
August 29, 2026 12:15
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Draft. Depends on #3 and rebases once that merges.
What this adds
An opt-in
launchStrategy: 'signingUrl'onpresentCaptiveSigningfor Android.The SDK's fetch-based
launchCaptiveSigningdownloads the envelope withinclude=documentsbefore opening the ceremony. That download runs on a read timeout derived from the envelope size, which floors at 15s when nothing is cached, so a large envelope on a slow connection exhausts it and the ceremony never opens.The SDK also exposes a
signingURLoverload that skips the download entirely.'signingUrl'mints a recipient view with the session access token and points the SDK straight at it, so the call that times out never runs. Verified on device across 12 consecutive ceremonies.Defaults to
'fetch', which is byte-identical to current behaviour, so upgrading changes nothing for existing consumers. Opt-in rather than default because the mint spends the consumer's bearer token on an endpoint that token is not guaranteed to be scoped for. A mint failure falls back to'fetch', so the strategy can only add a way to succeed.If your backend already mints recipient views,
presentCaptiveSigningWithUrlremains the better shape since it keeps the token off the device entirely. This exists for teams who cannot change their backend.Concurrency notes
Minting puts a network round trip between capturing the Activity and using it, which the synchronous fetch path never did. Two guards:
canLaunchOnrefuses to launch against a finishing or destroyed Activity, and checks the pending completion by identity, not nullness.reset()andendSigningSession()clear that slot, and a freshpresentCaptiveSigningcan claim it while a mint is still in flight. A nullness check would pass in that window and launch this envelope wired to the new session's promise, resolving it with the wrong outcome.DocuSignSessionreference rather than three independent@Volatilefields, so a login racing an in-flight mint cannot tear the triple and build a request with one session's token and another's account id.Known trade-off: the mint thread holds the
Activityreference for the round trip. Timeouts are set to 15s (rather than the 30s this was first written with) to bound that, andcanLaunchOnprevents any use of a dead Activity, but the reference is retained for the duration regardless.After #3 merges
This PR also picks up the maintainer items I pulled off that review. All of them touch pre-existing code rather than anything nikonhub introduced, so they were never that PR's to carry:
presentCaptiveSigningWithUrlinto the shared launcher and extractcaptiveSigningListener(), so the two entrypoints stop carrying duplicate listenerscurrentEnvelopeIdinpresentCaptiveSigning's catch, matching the shape feat: add Android support for captive signing with URL #3 got rightemitSigningErrorin both flows, sorecipient_signing_failedstops flattening intosigning_failedCodedException.codeinstead of hard-codingsigning_failedThrowsbullet once the error code settlesTesting
npm test: 17 passing,useDocuSignSigning.tsat 100% lines / 87% branches. Four new cases cover the passthrough, including that omittinglaunchStrategyproduces a payload with no such key at all rather than one set toundefined, which is the backward-compat guarantee.