Fix Android file uploads failing when OkHttp retries the request - #58817
Open
sepeterson wants to merge 2 commits into
Open
sepeterson wants to merge 2 commits into
sepeterson wants to merge 2 commits into
Conversation
This branch has not been deployed
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.
Summary:
On Android, a
fetch()orXMLHttpRequestfile upload fails withTypeError: Network request failed(native errorIOException: Stream Closed) when OkHttp retries the request, for example after a server or proxy closes a pooled keep-alive connection just as the upload is sent. This matches #32966, where the first upload fails and a second attempt succeeds.NetworkingModulebuilds the body for aurirequest body or aFormDatafile part from a singleInputStream, whichwriteTo()reads to the end and closes. WithretryOnConnectionFailure(OkHttp's default), OkHttp retries on a new connection and callswriteTo()again, which hits the closed stream. The body does reportisOneShot(), but OkHttp only checks the top-level body, and neitherMultipartBodynorProgressRequestBodypasses it up.This adds an internal
UriRequestBodythat opens a new stream for each write, andRequestBodyUtil.create(context, mediaType, uri), which returns one for every URINetworkingModuleaccepts. Forcontent://andfile://URIs the file is still opened up front, so a missing file fails the request before it is sent, and the first write reuses that stream, so a request that isn't retried opens the file once, as before. The DevTools request preview treatsUriRequestBodylike a one-shot body, so it never reads the file.There's no public API change:
UriRequestBodyandRequestBodyUtilareinternal.At iNaturalist, we had some cases where our prod API was closing idle keep-alive connections after a couple of seconds which resulted in elevated photo upload failures on Android only.
Changelog:
[ANDROID] [FIXED] - File uploads no longer fail with "Network request failed" when OkHttp retries the request on a stale connection
Test Plan:
96 tests, 0 failures.
yarn format-check-kotlinpasses. New tests write each URI body twice, as a retry would, and cover reopening on retry,IOExceptionwhen the file can't be reopened, and DevTools never opening the body.End to end: RNTester debug build on a Pixel 9 emulator (API 35). A small Node server (below) answers the first request on each connection and keeps it alive; on the second request, it reads the whole request and then destroys the socket without responding. The JS sends a warm-up
GETso OkHttp pools the connection, then aFormDataPOSTwith a 256 KB file that reuses it. Same app, emulator and file in both runs; only the files in this PR differ.On
main, the app getsupload POST failed after 71 ms: Stream Closed. OkHttp opens a new connection for the retry but closes it without sending anything:With this PR, the retry sends the full body and gets
200 {"connection":8,"bodyBytes":262563,"fileBytes":262144}:Not tested: a physical device, and
http(s)://URIs (downloaded to a temp file before uploading), which have no end-to-end or unit test coverage.Repro: JS (RNTester Playground)
The test file was placed with
adb shell run-as com.facebook.react.uiapp.Repro: server (
node stale-connection-server.js 8099)🤖 Generated with Claude Code
and reviewed/revised by Seth Peterson