Skip to content

Add user-configurable timeout to wasm http requests#133

Open
ianthomas23 wants to merge 5 commits into
mainfrom
http_timeout
Open

Add user-configurable timeout to wasm http requests#133
ianthomas23 wants to merge 5 commits into
mainfrom
http_timeout

Conversation

@ianthomas23
Copy link
Copy Markdown
Member

@ianthomas23 ianthomas23 commented Mar 27, 2026

WIP to add a timeout to wasm http requests. Such requests are synchronous blocking requests so we cannot obtain any feedback to present to the user whilst they are underway, and we have to wait until the corresponding http reply is received in full. These requests can take a long time fi they are clones of a large repo, as lots of data needs to be transferred back to us and usually this is a two-step return as it has to be via a CORS proxy. In the event of the request failing, if there isn't a timeout then the operation will block forever and cockle will be unresponsive forever. Note that this is the stage that occurs before the returned data is processed for which we do have progress reported to the user.

Ideally we'd use an asynchronous non-blocking request for this and it is part of our long term plan to implement asynchronous emscripten-forge terminal and kernel packages, but this is not yet available and is non-trivial to implement now.

Here there is a default timeout of 10 seconds (this is TBD really) and it can be overridden by the user via an environment variable:

export GIT_HTTP_TIMEOUT=20

This is a WIP as there is more to do:

  • test downstream
  • decide on sensible default
  • add better documentation in the help
  • in the event of a request timing out print useful information to the user to tell them that they can make the timeout longer
  • add tests

@ianthomas23 ianthomas23 added the enhancement New feature or request label Mar 27, 2026
@codecov
Copy link
Copy Markdown

codecov Bot commented Mar 27, 2026

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 88.52%. Comparing base (b57d3b3) to head (8b7e52c).
⚠️ Report is 1 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #133   +/-   ##
=======================================
  Coverage   88.52%   88.52%           
=======================================
  Files          62       62           
  Lines        2884     2884           
  Branches      358      358           
=======================================
  Hits         2553     2553           
  Misses        331      331           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@ianthomas23
Copy link
Copy Markdown
Member Author

I've opted for a default timeout of 10 seconds. Wasm tests pass (https://github.com/QuantStack/git2cpp/actions/runs/25670073737/job/75352697692), and I've added an environment variable section to the online docs: https://git2cpp--133.org.readthedocs.build/en/133/

We're probably going to need a new page in the online docs to explain how and why the wasm build is different to a real computer, but that is future work.

@ianthomas23 ianthomas23 marked this pull request as ready for review May 11, 2026 12:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant