Skip to content

SDK-382 BCIT JWT Auth Retry: add real Iterable backend mode#1057

Open
sumeruchat wants to merge 2 commits intomasterfrom
feature/SDK-382-bcit-real-backend-mode
Open

SDK-382 BCIT JWT Auth Retry: add real Iterable backend mode#1057
sumeruchat wants to merge 2 commits intomasterfrom
feature/SDK-382-bcit-real-backend-mode

Conversation

@sumeruchat
Copy link
Copy Markdown
Contributor

What

The BCIT tester's JWT Auth Retry screen used to fabricate every response locally, so the SDK's new JWT retry logic was never validated against real Iterable traffic. Mirror the Android network-tester so Normal/401 proxy through to api.iterable.com while 500/ConnErr stay locally synthesized.

Changes

  • MockAPIServerURLProtocol.startLoading dispatches on response mode: Normal and 401 proxy through URLSession.shared (handled-marker avoids URLProtocol recursion); 500/ConnErr keep the existing local synthesis.
  • For mode 401 the forwarded request's Authorization is rewritten to Bearer <locally-forged expired JWT> using the same email source (IterableAPI.email ?? AppDelegate.currentTestEmail) as MockAuthDelegate, so the backend returns real InvalidJwtPayload rather than a user-mismatch error.
  • stopLoading cancels the forwarded task; completion swallows NSURLErrorCancelled to respect the URLProtocol contract.
  • MockAPIServer exposes jwtSecret, drops unused success/jwt401 local helpers, and narrows mockResponse(for:) to 500/ConnErr.
  • Small UI hint next to the response radios: "→ real backend" vs "→ local mock".
  • README gains a JWT Auth Retry response-mode table and test-config.json notes.

Impact

  • Breaking changes: None — SDK and prod app code untouched; changes are scoped to the BCIT tester app.
  • Dependencies: None.
  • Performance: Proxied round-trips introduce real network latency (that is the point).
  • Data: Normal and 401 now send real traffic to whichever Iterable project test-config.json points at. Current projectId is 28411 — keep using a dedicated test project so prod dashboards stay clean.

Testing

How to test:

  1. tests/business-critical-integration/scripts/build-and-run.sh
  2. Open JWT Auth Retry → enter a test email → Login.
  3. Mode = Normal, tap Fire All → network log should show real 200s from api.iterable.com (not the old canned {"successCount":1}).
  4. Mode = 401, tap Fire All → expect real 401 with {"code":"InvalidJwtPayload", ...} from Iterable; confirm SDK pause + refresh + queue flush when flipped back to Normal.
  5. Mode = 500 / Conn Err → unchanged local behavior.

Edge cases:

  • Log out (email cleared): proxy in Normal still runs, but the SDK won't attach an auth token — you'll see real 401s from the backend; that's expected.
  • Rapid mode flipping during an in-flight request: stopLoading should cancel the forwarded task silently (no noisy error).

Known unverified items: I have not yet observed a round-trip myself, so POST body forwarding through URLRequest.httpBody over URLProtocol is untested end-to-end (known iOS quirk when the SDK uses a body stream). Worth watching the first Normal-mode Fire All for empty-body 400s.

The JWT Auth Retry screen used to fabricate every response locally in
MockAPIServerURLProtocol, so the SDK's retry logic was never exercised
against real Iterable traffic — no real latency, no real JWT validation,
no real 401 InvalidJwtPayload bodies. Mirror the Android network-tester
so Normal and 401 modes proxy through to api.iterable.com, while 500
and Conn Err stay locally synthesized (you can't force prod to 500).

- .normal / .jwt401 now re-issue the request on URLSession.shared with
  an internal handled-marker so URLProtocol doesn't recurse on itself;
  .jwt401 swaps Authorization with a locally-forged expired JWT for the
  SDK's current email, matching MockAuthDelegate's source of truth.
- Swallow NSURLErrorCancelled in the forwarded completion to respect
  the URLProtocol contract after stopLoading().
- MockAPIServer keeps jwtSecret for the swap, drops unused success /
  jwt401 local helpers, and narrows mockResponse(for:) to 500/ConnErr.
- OfflineRetryTestViewController shows a "-> real backend" / "-> local
  mock" hint next to the response radios.
- README documents the per-mode destinations and the test-config.json
  requirements (dedicated Iterable test project recommended).

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@codecov
Copy link
Copy Markdown

codecov bot commented Apr 15, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 70.72%. Comparing base (778d0a9) to head (8a11161).

Additional details and impacted files
@@           Coverage Diff           @@
##           master    #1057   +/-   ##
=======================================
  Coverage   70.72%   70.72%           
=======================================
  Files         112      112           
  Lines        9201     9201           
=======================================
  Hits         6507     6507           
  Misses       2694     2694           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@franco-zalamena-iterable
Copy link
Copy Markdown

This is great, but maybe we can simplify it.

The core goal is validating JWT retry against real 401s, and the .jwt401 proxy path does exactly that. But proxying .normal mode adds complexity we don't really need. On android we do proxy on normal mode but almost because it is more complex to not do it on normal mode than it is to just always proxy it. On iOS that does not seem to be the case, we are having a lot of logic just to proxy to normal mode where we don't really want to change anything.

My suggestion is to just to call the real api without interception on normal mode and keep proxy for 401 and mock for 500 and connection error

Copy link
Copy Markdown

@franco-zalamena-iterable franco-zalamena-iterable left a comment

Choose a reason for hiding this comment

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

We can possibly simplify this pr, check above comment

Per Franco's review: on Android the NanoHTTPD proxy is necessary because
SDK traffic is force-routed through localhost, so Normal mode has to
proxy. On iOS we control interception per-request, so the simpler
equivalent of "Normal = real backend" is to have URLProtocol opt out of
the request and let URLSession talk to api.iterable.com directly. That
drops the Normal-mode proxy path entirely (no URLSession.shared re-issue,
no httpBody-forwarding concern).

- canInit returns false when apiResponseMode is .normal; the SDK's
  existing URLSession handles the request with zero tester involvement.
- startLoading now only handles .jwt401 (proxy with expired JWT swap)
  plus .server500 / .connectionError (local synthesis). .normal kept as
  a defensive no-op in case the mode flips between canInit and here.
- UI hint clarifies the new split: "real backend (direct)" vs "real
  backend (expired JWT)" vs "local mock".
- README table updated to match.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@sumeruchat
Copy link
Copy Markdown
Contributor Author

Good call @franco-zalamena-iterable — you're right, the iOS equivalent of Android's always-proxy doesn't buy us anything since URLProtocol can just opt out per request.

Pushed 8a11161e:

  • canInit returns false when mode is .normal → the SDK's URLSession hits api.iterable.com directly, tester completely out of the way.
  • startLoading trimmed to only .jwt401 (proxy + expired JWT) and .server500 / .connectionError (local synth).
  • Also kills the POST httpBody-through-URLProtocol concern from the original PR description, since we only forward for the 401 path now.
  • UI hint updated: "real backend (direct)" / "real backend (expired JWT)" / "local mock". README table updated to match.

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