Skip to content

fix android http cancelation - #142

Open
tx3stn wants to merge 4 commits into
Snapchat:mainfrom
tx3stn:fix-android-cancelation
Open

fix android http cancelation#142
tx3stn wants to merge 4 commits into
Snapchat:mainfrom
tx3stn:fix-android-cancelation

Conversation

@tx3stn

@tx3stn tx3stn commented Aug 8, 2026

Copy link
Copy Markdown
Contributor

Description

Android DefaultHTTPRequestManager diverges from the iOS/macOS defaults in two ways:

  1. Cancellation does nothing. HTTPRequestTask.cancel() nulls the completion but never touches the connection, so the transfer runs to completion and the result is thrown away.
    iOS actually cancels the NSURLSessionTask, so it works there.

  2. Requests are serial. The executor is ThreadPoolExecutor(0, 1, …). iOS uses NSURLSession.sharedSession, which is concurrent by default.

So a cancelled request neither stops nor frees the only worker, and everything else queues behind it until it finishes on its own.

Changes

  • cancel() disconnects the connection (a request cancelled while queued never opens a connection).
  • Pool sized to 4, matching NSURLSession.HTTPMaximumConnectionsPerHost (so Android behaviour is consistent with iOS)
  • Added unit tests for these and evrything with bazel test //valdi:test_java --test_output=errors is passing.

Type of Change

  • Bug fix (non-breaking change that fixes an issue)
  • Documentation improvement
  • Performance optimization
  • Test improvement
  • Other (please describe)

Testing

  • Tests pass locally (bazel test //...)
  • Added/updated tests for changes (if applicable)
  • Tested on multiple platforms (iOS/Android/Web/macOS as applicable)
  • Manual testing performed (describe below)

Testing Details

Checklist

  • Code follows project style guidelines
  • Documentation updated (if needed)
  • No breaking changes (or documented in description)
  • Commit messages follow conventional format
  • No secrets, API keys, or internal URLs included

Related Issues

Additional Context

@github-actions

github-actions Bot commented Aug 8, 2026

Copy link
Copy Markdown

⚠️ Bazel & CI Test Results

Test Suite Result
Valdi Smoke Tests ❌ failure
valdi_web Integration Test ✅ success
macOS: C++ & Platform Tests ✅ success
Snapshot Tests ✅ success
API Surface Check ✅ success
Linux: Build Compiler ✅ success
Linux: C++ Tests ✅ success
Linux: Build & Export ✅ success

Some tests failed. Please check the workflow logs for details.

🚀 Bazel remote cache is now enabled - future builds will be faster!

Workflow: Valdi CI

@tx3stn

tx3stn commented Aug 8, 2026

Copy link
Copy Markdown
Contributor Author

failing job appears to be a timeout, not related to my changes, but I don't have permissions to re-run.

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.

1 participant