swift-sdk icon indicating copy to clipboard operation
swift-sdk copied to clipboard

[SDK-45] Fix iOS deep link resolution failures for greenFi

Open sumeruchat opened this issue 2 months ago • 7 comments

Ticket: https://iterable.atlassian.net/browse/MOB-11490

Issue

GreenFi, 2ULaundry, and Hello Heart reported that iOS deep links were failing intermittently. Users would tap SMS/email links but the app wouldn't open to the correct screen. This was blocking all iOS deep linking for these clients.

What Was Happening

When a user taps an Iterable deep link (like https://links.greenfi.com/a/N2Nbu), the SDK needs to:

  1. Make a request to the shortened link
  2. Get the 303 redirect response with attribution data
  3. Extract the final destination URL and campaign info
  4. Open the app to that URL

The problem was in step 3. Here's what was going wrong:

The SDK intercepts the redirect to capture attribution cookies, then calls completionHandler(nil) to prevent actually following the redirect (we only want one hop). However, when the redirect is cancelled, URLSession sometimes returns an error - either NSURLErrorTimedOut (-1001) or NSURLErrorCancelled (-999).

The original code checked for errors FIRST:

if let error = error {
    // Fail immediately ❌
    fulfill.resolve(with: (nil, nil))
}

But by this point, the redirect delegate had ALREADY successfully captured the destination URL! We were throwing away perfectly good data just because URLSession reported an error from cancelling the redirect.

Why It Was Intermittent

This was a race condition based on network timing:

  • Fast networks/good conditions: Request completes quickly → timeout error → deep link fails ❌
  • Slow networks/cached responses: Request completes before timeout → no error → deep link works ✅

So ironically, clients with better network conditions were MORE likely to experience failures.

The Fix

We reversed the logic to check if we captured the redirect URL FIRST, before checking for errors:

if self.deepLinkLocation != nil {
    // We got the data we need, use it! ✅
    fulfill.resolve(with: (self.deepLinkLocation, attributionInfo))
} else if let error = error {
    // Only fail if we truly didn't get the redirect data
    fulfill.resolve(with: (nil, nil))
}

We also fixed two missing completionHandler(nil) calls in early return paths that could cause URLSession tasks to hang.

Why This Works

The redirect delegate is called SYNCHRONOUSLY when the 303 response arrives, so deepLinkLocation is always set before the completion handler runs. By checking for this data first, we use it regardless of any subsequent timeout/cancellation errors from URLSession.

This matches what we saw in 2ULaundry's successful cases - they were getting the location and opening links correctly, just without attribution data (separate cookie parsing issue).

Impact

  • ✅ Fixes deep linking for all clients using /a/* URL patterns (all Iterable email/SMS links)
  • ✅ Resolves timeout errors (-1001)
  • ✅ Resolves cancellation errors (-999)
  • ✅ Eliminates network timing race conditions
  • ✅ Prevents hanging URLSession tasks

sumeruchat avatar Nov 18 '25 17:11 sumeruchat

Codecov Report

:x: Patch coverage is 41.17647% with 10 lines in your changes missing coverage. Please review. :white_check_mark: Project coverage is 69.34%. Comparing base (49af0f3) to head (70cfb19).

Files with missing lines Patch % Lines
swift-sdk/Internal/DeepLinkManager.swift 46.66% 8 Missing :warning:
swift-sdk/Internal/Network/NetworkSession.swift 0.00% 2 Missing :warning:
Additional details and impacted files
@@            Coverage Diff             @@
##           master     #974      +/-   ##
==========================================
- Coverage   69.43%   69.34%   -0.09%     
==========================================
  Files         109      109              
  Lines        8916     8929      +13     
==========================================
+ Hits         6191     6192       +1     
- Misses       2725     2737      +12     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

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

codecov[bot] avatar Nov 18 '25 17:11 codecov[bot]

@sumeruchat Is this issue happening specifically with shortened urls with SMS only or with any re-written url?

davidtruong avatar Nov 20 '25 15:11 davidtruong

@davidtruong It would happen a /a url in particular that would follow the redirect but if iOS returned an error of any kind it wouldnt use the redirect url

sumeruchat avatar Nov 20 '25 15:11 sumeruchat

@davidtruong It would happen a /a url in particular that would follow the redirect but if iOS returned an error of any kind it wouldnt use the redirect url

Do we know why a NSURLErrorCancelled would get called for the original request?

davidtruong avatar Nov 20 '25 15:11 davidtruong

⚠️ We should probably do a small Bug Bash to ensure these changes are covered in various scenarios before allowing this to go into GA.

joaodordio avatar Nov 20 '25 15:11 joaodordio

@davidtruong So basically I was thinking that passing nil to the completionHandler in willPerformHTTPRedirection might return a cancelled error or its also possible that the request times out due to the server being slow to responr or bad network and it doesnt use the URL it already has received.

sumeruchat avatar Nov 21 '25 18:11 sumeruchat

@davidtruong Another bug we fixed is that if we dont call the completionHandler at all before return in the guard condition it might hang and not complete the request at all.

sumeruchat avatar Nov 21 '25 18:11 sumeruchat