Skip to content

Conversation

@lukasmasuch
Copy link
Collaborator

@lukasmasuch lukasmasuch commented Nov 26, 2025

Describe your changes

Use a higher websocket timeout on Android and better error message to improve file uploading on Android. Currently, this doesn't work if the user needs more than 15 seconds to choose a file since the websocket connection disconnects.

From my research, a proper fix for this would be quite complicate.

I manually verified on Android that this change works and makes it a lot more convenient to choose files.

GitHub Issue Link (if applicable)

Testing Plan

  • Added unit tests.

Contribution License Agreement

By submitting this pull request you agree that all contributions to this project are made under the Apache 2.0 license.

@snyk-io
Copy link
Contributor

snyk-io bot commented Nov 26, 2025

Snyk checks have passed. No issues have been found so far.

Status Scanner Critical High Medium Low Total (0)
Open Source Security 0 0 0 0 0 issues
Licenses 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

@github-actions
Copy link
Contributor

github-actions bot commented Nov 26, 2025

✅ PR preview is ready!

Name Link
📦 Wheel file https://core-previews.s3-us-west-2.amazonaws.com/pr-13132/streamlit-1.51.0-py3-none-any.whl
📦 @streamlit/component-v2-lib Download from artifacts
🕹️ Preview app pr-13132.streamlit.app (☁️ Deploy here if not accessible)

@lukasmasuch lukasmasuch added security-assessment-completed Security assessment has been completed for PR change:bugfix PR contains bug fix implementation impact:users PR changes affect end users labels Nov 26, 2025
@lukasmasuch lukasmasuch marked this pull request as ready for review November 26, 2025 01:12
Copilot AI review requested due to automatic review settings November 26, 2025 01:12
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes a bug where file uploads fail on Android due to WebSocket disconnection when selecting files. The solution implements a request queue that stores file URL requests made while disconnected and processes them when the connection is re-established.

Key Changes

  • Adds a queue mechanism to handle file URL requests during WebSocket disconnections
  • Processes queued requests automatically when connection is restored
  • Updates tests to verify queuing and processing behavior

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.

File Description
frontend/app/src/App.tsx Implements pendingFileURLRequests queue, modifies requestFileURLs to queue when disconnected, adds processPendingFileURLRequests method to process queue on reconnection
frontend/app/src/App.test.tsx Updates existing test description and adds new test case for verifying queued requests are processed on reconnection
Comments suppressed due to low confidence (1)

frontend/app/src/App.tsx:817

  • The pendingFileURLRequests queue is never cleared when the connection is permanently lost. While this may not be a significant issue in production (as the page would typically be refreshed), it's good practice to clear the queue on permanent disconnection to prevent unbounded memory growth.

Recommended fix: Clear the queue when transitioning to DISCONNECTED_FOREVER state in handleConnectionStateChanged:

} else {
  // If we're starting from the CONNECTED state and going to any other
  // state, we must be disconnecting.
  if (this.state.connectionState === ConnectionState.CONNECTED) {
    this.hostCommunicationMgr.sendMessageToHost({
      type: "WEBSOCKET_DISCONNECTED",
      attemptingToReconnect:
        newState !== ConnectionState.DISCONNECTED_FOREVER,
    })
  }

  // Clear pending file requests on permanent disconnection
  if (newState === ConnectionState.DISCONNECTED_FOREVER) {
    this.pendingFileURLRequests = []
  }

  if (this.sessionInfo.isSet) {
    this.sessionInfo.disconnect()
  }
}
    } else {
      // If we're starting from the CONNECTED state and going to any other
      // state, we must be disconnecting.
      if (this.state.connectionState === ConnectionState.CONNECTED) {
        this.hostCommunicationMgr.sendMessageToHost({
          type: "WEBSOCKET_DISCONNECTED",
          attemptingToReconnect:
            newState !== ConnectionState.DISCONNECTED_FOREVER,
        })
      }

      if (this.sessionInfo.isSet) {
        this.sessionInfo.disconnect()
      }
    }

@lukasmasuch lukasmasuch marked this pull request as draft November 26, 2025 01:25
@lukasmasuch lukasmasuch changed the title Fix file upload failing on Android [WIP] Fix file upload failing on Android Nov 26, 2025
@lukasmasuch lukasmasuch changed the title [WIP] Fix file upload failing on Android [WIP] Enable better file uploading on Android Nov 26, 2025
* See: https://github.com/streamlit/streamlit/issues/11419
*/
export const WEBSOCKET_TIMEOUT_MS = 15 * 1000
export const WEBSOCKET_TIMEOUT_MS = isAndroidDevice() ? 60 * 1000 : 15 * 1000
Copy link
Collaborator Author

@lukasmasuch lukasmasuch Nov 26, 2025

Choose a reason for hiding this comment

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

60 seconds should be a lot less annoying for uploading files compared to 15 seconds timeout.

@lukasmasuch lukasmasuch marked this pull request as ready for review November 26, 2025 01:45
@lukasmasuch lukasmasuch changed the title [WIP] Enable better file uploading on Android Enable better file uploading on Android Nov 26, 2025
Copy link
Collaborator

@sfc-gh-lwilby sfc-gh-lwilby left a comment

Choose a reason for hiding this comment

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

This looks like a good mitigation. Thanks!

@lukasmasuch lukasmasuch merged commit 9442681 into develop Nov 26, 2025
42 checks passed
@lukasmasuch lukasmasuch deleted the fix/file-upload-on-android branch November 26, 2025 17:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

change:bugfix PR contains bug fix implementation impact:users PR changes affect end users security-assessment-completed Security assessment has been completed for PR

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants