Skip to content

feat(android): opt-in signing-URL launch strategy for presentCaptiveSigning - #4

Merged
IronTony merged 4 commits into
mainfrom
feat/android-signing-url-launch-strategy
Aug 29, 2026
Merged

IronTony merged 4 commits into
mainfrom
feat/android-signing-url-launch-strategy

Conversation

@IronTony

@IronTony IronTony commented Aug 22, 2026

Copy link
Copy Markdown
Owner

Draft. Depends on #3 and rebases once that merges.

What this adds

An opt-in launchStrategy: 'signingUrl' on presentCaptiveSigning for Android.

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 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, presentCaptiveSigningWithUrl remains 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:

  • canLaunchOn refuses to launch against a finishing or destroyed Activity, and checks the pending completion by identity, not nullness. reset() and endSigningSession() clear that slot, and a fresh presentCaptiveSigning 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, resolving it with the wrong outcome.
  • Session credentials moved into a single DocuSignSession reference rather than three independent @Volatile fields, 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 Activity reference for the round trip. Timeouts are set to 15s (rather than the 30s this was first written with) to bound that, and canLaunchOn prevents 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:

  • fold presentCaptiveSigningWithUrl into the shared launcher and extract captiveSigningListener(), so the two entrypoints stop carrying duplicate listeners
  • null currentEnvelopeId in presentCaptiveSigning's catch, matching the shape feat: add Android support for captive signing with URL #3 got right
  • drop the duplicate emitSigningError in both flows, so recipient_signing_failed stops flattening into signing_failed
  • forward CodedException.code instead of hard-coding signing_failed
  • fill in the README Throws bullet once the error code settles

Testing

npm test: 17 passing, useDocuSignSigning.ts at 100% lines / 87% branches. Four new cases cover the passthrough, including that omitting launchStrategy produces a payload with no such key at all rather than one set to undefined, which is the backward-compat guarantee.

…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
IronTony force-pushed the feat/android-signing-url-launch-strategy branch from 76bbd43 to 1d2f138 Compare August 27, 2026 07:24
@IronTony
IronTony marked this pull request as ready for review August 27, 2026 07:25
@IronTony
IronTony marked this pull request as draft August 27, 2026 07:27
@IronTony
IronTony force-pushed the feat/android-signing-url-launch-strategy branch from 1d2f138 to 76bbd43 Compare August 27, 2026 07:27
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
IronTony force-pushed the feat/android-signing-url-launch-strategy branch from 76bbd43 to 7e6e9e4 Compare August 27, 2026 16:46
@IronTony IronTony self-assigned this Aug 27, 2026
@IronTony IronTony added the enhancement New feature or request label Aug 27, 2026
@IronTony
IronTony marked this pull request as ready for review August 29, 2026 12:15
@IronTony
IronTony merged commit bc73204 into main Aug 29, 2026
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant