Skip to content

fix: timeout on ios uploads on http 1.1 connection - #3787

Open
isekovanic wants to merge 1 commit into
developfrom
fix/http-1-ios-native-upload-failure
Open

fix: timeout on ios uploads on http 1.1 connection#3787
isekovanic wants to merge 1 commit into
developfrom
fix/http-1-ios-native-upload-failure

Conversation

@isekovanic

Copy link
Copy Markdown
Contributor

🎯 Goal

This PR fixes iOS attachment uploads hanging for 60s and then failing with -1001 (timed out) after the server has already accepted the file.

Affects apps using useNativeMultipartUpload specifically.

The CFNetwork trace of a failing upload looks something like this:

resuming, timeouts(60.0, …)
received response, status 201        ← server took the file
…60 s of nothing…
finished with error [-1001]
summary for task failure { response_status=201, request_bytes=2810041,
                           response_duration_ms=0, protocol="http/1.1" }

So the upload succeeds and the client throws it away a minute later. The user sees a failed attachment (and the message with the attachment is not sent consequently).

🛠 Implementation details

The root cause is unfortunately not as simple. The multipart body was pretty much a hand rolled NSInputStream subclass. CFNetwork drives an HTTP/1.1 request body through the CFReadStream interface, which a plain InputStream subclass cannot participate in, so it can never report end-of-stream. CFNetwork stops calling read the moment Content-Length is satisfied, so the subclass never returned 0, never reached .atEnd and so CFNetwork never learned the body had ended. The transaction stayed open until the timeout (which is 60 seconds later).

This isn't something that would fire all the time, however. It happens whenever the connection ends up on HTTP/1.1. That's decided on a much lower level (during the TLS handshake), so it's server and network determined rather than something the app controls, so if an endpoint that doesn't advertise h2, a TLS intercepting proxy or a local debugging proxy such as Charles or Proxyman will all put us there. Over HTTP/2 and HTTP/3 the request body is framed and terminated by Content-Length, so the missing end-of-stream signal never mattered and the bug stayed dormant. Hence, why this has gone unnoticed for so long. Naturally, this is an edge case altogether but I've decided to rewrite chunks of the body stream class. There have always existed some certain parts of it that bothered me and we attempt to address them here as well.

makeStream() now hands URLSession the read end of a CFStreamCreateBoundPair, so a real CFReadStream that reports every event and a new StreamMultipartBodyProducer feeds the write end from the same element list. Closing the write end is what tells CFNetwork the body is complete.

The producer is driven by GCD (CFWriteStreamSetClient + CFWriteStreamSetDispatchQueue) rather than a run loop, so it owns no thread and can't outlive its work.

Because a bound pair has no error channel, body production failures are recorded in a StreamMultipartBodyErrorBox and preferred over the transport error in didCompleteWithError. The box is per attempt so then URLSession can request a fresh body stream on retry and an abandoned attempt's failure must not fail a later one that succeeds.

Aside from a bit of code complexity, this also costs about 128 KiB of extra memory per request (which is nothing). In turn we handle a lot of odd edge cases, like:

  • no longer relying on kind of undocumented behaviour
  • failures actually get reported every time now
  • backpressure is now being handled instead of just falsely quitting the upload
  • retry scoping is now strictly

And naturally, HTTP/1.1 no longer fails uploads.

🎨 UI Changes

🧪 Testing

☑️ Checklist

  • I have signed the Stream CLA (required)
  • PR targets the develop branch
  • Documentation is updated
  • New code is tested in main example apps, including all possible scenarios
    • SampleApp iOS and Android
    • Expo iOS and Android

@Stream-SDK-Bot

Copy link
Copy Markdown
Contributor

SDK Size

title develop branch diff status
js_bundle_size 1997 KB 1997 KB 0 B 🟢

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