Visitar URL original
Add user-configurable timeout to wasm http requests (#133) · QuantStack/git2cpp@68ae681 · GitHub
Skip to content

Commit 68ae681

Browse files
ianthomas23ianthomas23
authored
Add user-configurable timeout to wasm http requests (#133)
* Add user-configurable timeout to wasm http requests * Timeout error message * Add timeout tests * Add to online docs * Be more careful with signed/unsigned comparisons
1 parent 659a1f3 commit 68ae681

10 files changed

Lines changed: 158 additions & 8 deletions

File tree

‎CMakeLists.txt‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,7 @@ set(GIT2CPP_SRC
100100
${GIT2CPP_SOURCE_DIR}/utils/progress.hpp
101101
${GIT2CPP_SOURCE_DIR}/utils/terminal_pager.cpp
102102
${GIT2CPP_SOURCE_DIR}/utils/terminal_pager.hpp
103+
${GIT2CPP_SOURCE_DIR}/wasm/constants.hpp
103104
${GIT2CPP_SOURCE_DIR}/wasm/libgit2_internals.cpp
104105
${GIT2CPP_SOURCE_DIR}/wasm/libgit2_internals.hpp
105106
${GIT2CPP_SOURCE_DIR}/wasm/response.cpp
@@ -112,6 +113,8 @@ set(GIT2CPP_SRC
112113
${GIT2CPP_SOURCE_DIR}/wasm/subtransport.hpp
113114
${GIT2CPP_SOURCE_DIR}/wasm/transport.cpp
114115
${GIT2CPP_SOURCE_DIR}/wasm/transport.hpp
116+
${GIT2CPP_SOURCE_DIR}/wasm/utils.cpp
117+
${GIT2CPP_SOURCE_DIR}/wasm/utils.hpp
115118
${GIT2CPP_SOURCE_DIR}/wrapper/annotated_commit_wrapper.cpp
116119
${GIT2CPP_SOURCE_DIR}/wrapper/annotated_commit_wrapper.hpp
117120
${GIT2CPP_SOURCE_DIR}/wrapper/branch_wrapper.cpp

‎docs/conf.py‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,3 +19,4 @@
1919
"show_navbar_depth": 2,
2020
}
2121
html_title = "git2cpp documentation"
22+
myst_enable_extensions = ["deflist"]

‎docs/index.md‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,3 +15,14 @@ please create an issue in the [git2cpp github repository](https://github.com/Qua
1515
:hidden:
1616
created/git2cpp
1717
```
18+
19+
## Environment variables
20+
21+
`GIT_HTTP_TIMEOUT`
22+
: In the WebAssembly build, all http(s) requests are limited by a timeout which has a default of 10
23+
seconds. To use a different timeout set the `GIT_HTTP_TIMEOUT` environment variable. For example,
24+
to set a timeout of 20 seconds use:
25+
26+
```bash
27+
export GIT_HTTP_TIMEOUT=20
28+
```

‎src/wasm/constants.hpp‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
#pragma once
2+
3+
#include <string_view>
4+
5+
// Constants used in wasm transport layer.
6+
// Exposed to non-emscripten builds so that they can be used in help.
7+
8+
// Environment variable for http transport timeout.
9+
// Must be a positive number as a timeout of 0 will block forever.
10+
inline constexpr std::string_view WASM_HTTP_TRANSPORT_TIMEOUT_NAME = "GIT_HTTP_TIMEOUT";
11+
inline constexpr unsigned int WASM_HTTP_TRANSPORT_TIMEOUT_DEFAULT_S = 10;

‎src/wasm/stream.cpp‎

Lines changed: 20 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
# include <emscripten.h>
99

1010
# include "../utils/common.hpp"
11+
# include "constants.hpp"
1112
# include "response.hpp"
1213

1314
// Buffer size used in transport_smart, hardcoded in libgit2.
@@ -51,6 +52,7 @@ EM_JS(
5152
const char* method,
5253
const char* content_type_header,
5354
const char* authorization_header,
55+
unsigned long request_timeout_ms,
5456
size_t buffer_size),
5557
{
5658
const url_js = UTF8ToString(url);
@@ -76,6 +78,7 @@ EM_JS(
7678
{
7779
xhr.setRequestHeader("x-runtime-token", "{#{RUNTIME_TOKEN}#}");
7880
}
81+
xhr.timeout = request_timeout_ms;
7982

8083
// Cache request info on JavaScript side so that it is available in subsequent calls
8184
// without having to pass it back and forth to/from C++.
@@ -104,7 +107,7 @@ EM_JS(
104107
// clang-format off
105108
Module["git2cpp_js_error"] = { name: err.name ?? "", message : err.message ?? "" };
106109
// clang-format on
107-
console.error(err);
110+
(err.name == "TimeoutError" ? console.warn : console.error)(err);
108111
return -1;
109112
}
110113
}
@@ -206,7 +209,7 @@ EM_JS(
206209
// clang-format off
207210
Module["git2cpp_js_error"] = { name: err.name ?? "", message : err.message ?? "" };
208211
// clang-format on
209-
console.error(err);
212+
(err.name == "TimeoutError" ? console.warn : console.error)(err);
210213
return -1;
211214
}
212215
}
@@ -217,7 +220,7 @@ EM_JS(void, js_warning, (const char* msg), {
217220
console.warning(msg_js);
218221
});
219222

220-
EM_JS(size_t, js_write, (int request_index, const char* buffer, size_t buffer_size), {
223+
EM_JS(int, js_write, (int request_index, const char* buffer, size_t buffer_size), {
221224
try
222225
{
223226
const cache = Module["git2cpp_js_cache"];
@@ -243,7 +246,7 @@ EM_JS(size_t, js_write, (int request_index, const char* buffer, size_t buffer_si
243246
// clang-format off
244247
Module["git2cpp_js_error"] = { name: err.name ?? "", message : err.message ?? "" };
245248
// clang-format on
246-
console.error(err);
249+
(err.name == "TimeoutError" ? console.warn : console.error)(err);
247250
return -1;
248251
}
249252
});
@@ -271,6 +274,16 @@ static void convert_js_to_git_error(wasm_http_stream* stream)
271274
stream->m_unconverted_url.c_str()
272275
);
273276
}
277+
else if (std::string_view(error_str).starts_with("TimeoutError:"))
278+
{
279+
git_error_set(
280+
GIT_ERROR_HTTP,
281+
"network request timed out connecting to %s. You can set a longer timeout in seconds using the environment variable %s, the default value is %u seconds.",
282+
stream->m_unconverted_url.c_str(),
283+
WASM_HTTP_TRANSPORT_TIMEOUT_NAME.data(),
284+
WASM_HTTP_TRANSPORT_TIMEOUT_DEFAULT_S
285+
);
286+
}
274287
else
275288
{
276289
git_error_set(GIT_ERROR_HTTP, "%s", error_str);
@@ -285,6 +298,7 @@ static int create_request(wasm_http_stream* stream, std::string_view content_hea
285298
name_for_method(stream->m_service.m_method).c_str(),
286299
content_header.data(),
287300
stream->m_subtransport->m_authorization_header.c_str(),
301+
stream->m_subtransport->m_request_timeout_ms,
288302
EMFORGE_BUFSIZE
289303
);
290304
return stream->m_request_index;
@@ -345,7 +359,7 @@ static int read(wasm_http_stream* stream, wasm_http_response& response, bool is_
345359
&status_text,
346360
&response_headers
347361
);
348-
if (bytes_read < 0)
362+
if (bytes_read == static_cast<size_t>(-1))
349363
{
350364
convert_js_to_git_error(stream);
351365
// Delete const char* allocated in JavaScript.
@@ -537,7 +551,7 @@ int wasm_http_stream_read(git_smart_subtransport_stream* s, char* buffer, size_t
537551
bool send = true;
538552
while (send)
539553
{
540-
if (read(stream, response, false) < 0)
554+
if (read(stream, response, false) == static_cast<size_t>(-1))
541555
{
542556
return -1; // git error already set.
543557
}

‎src/wasm/subtransport.cpp‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,15 +2,16 @@
22

33
# include "subtransport.hpp"
44

5+
# include <iostream>
56
# include <regex>
67
# include <sstream>
78

8-
# include <emscripten.h>
99
# include <git2/sys/credential.h>
1010
# include <git2/sys/remote.h>
1111

1212
# include "libgit2_internals.hpp"
1313
# include "stream.hpp"
14+
# include "utils.hpp"
1415

1516
// C functions.
1617

@@ -93,6 +94,7 @@ int create_wasm_http_subtransport(git_smart_subtransport** out, git_transport* o
9394
subtransport->m_owner = owner;
9495
subtransport->m_base_url = "";
9596
subtransport->m_credential = nullptr;
97+
subtransport->m_request_timeout_ms = get_request_timeout_ms();
9698

9799
*out = &subtransport->m_parent;
98100
return 0;

‎src/wasm/subtransport.hpp‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,8 @@ struct wasm_http_subtransport
1717
// Data stored for reuse on other streams of this transport:
1818
std::string m_base_url;
1919
std::string m_authorization_header;
20-
git_credential* m_credential; // libgit2 creates this, we are responsible for deleting it.
20+
git_credential* m_credential; // libgit2 creates this, we are responsible for deleting it.
21+
unsigned long m_request_timeout_ms; // Timeout for http(s) requests in milliseconds.
2122
};
2223

2324
// git_smart_subtransport_cb

‎src/wasm/utils.cpp‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
#ifdef EMSCRIPTEN
2+
3+
# include "utils.hpp"
4+
5+
# include <termcolor/termcolor.hpp>
6+
7+
# include "constants.hpp"
8+
9+
unsigned long get_request_timeout_ms()
10+
{
11+
double timeout_seconds = WASM_HTTP_TRANSPORT_TIMEOUT_DEFAULT_S;
12+
auto env_var = std::getenv(WASM_HTTP_TRANSPORT_TIMEOUT_NAME.data());
13+
if (env_var != nullptr)
14+
{
15+
try
16+
{
17+
auto value = std::stod(env_var);
18+
if (value < 1e-3) // Must be at least 1 ms.
19+
{
20+
throw std::runtime_error(""); // Caught below.
21+
}
22+
timeout_seconds = value;
23+
}
24+
catch (std::exception& e)
25+
{
26+
// Catch failures from (1) stod and (2) timeout <= 0.
27+
// Print warning and use default value.
28+
std::cout << termcolor::yellow << "Warning: environment variable "
29+
<< WASM_HTTP_TRANSPORT_TIMEOUT_NAME
30+
<< " must be a positive number of seconds, using default value of "
31+
<< WASM_HTTP_TRANSPORT_TIMEOUT_DEFAULT_S << " seconds instead." << termcolor::reset
32+
<< std::endl;
33+
}
34+
}
35+
return 1000 * timeout_seconds;
36+
}
37+
38+
#endif // EMSCRIPTEN

‎src/wasm/utils.hpp‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
#pragma once
2+
3+
#ifdef EMSCRIPTEN
4+
5+
// Get wasm http request timeout in milliseconds from environment variable or default value.
6+
unsigned long get_request_timeout_ms();
7+
8+
#endif // EMSCRIPTEN

‎test/test_clone.py‎

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import pytest
22
import subprocess
3+
from .conftest import GIT2CPP_TEST_WASM
34

45
xtl_url = "https://github.com/xtensor-stack/xtl.git"
56

@@ -144,3 +145,63 @@ def test_clone_gitlab(git2cpp_path, tmp_path, run_in_tmp_path, protocol):
144145
assert p_status.returncode == 0
145146
assert "On branch main" in p_status.stdout
146147
assert "Your branch is up to date with 'origin/main'" in p_status.stdout
148+
149+
150+
@pytest.mark.skipif(not GIT2CPP_TEST_WASM, reason="Only test in WebAssembly")
151+
def test_clone_timeout(git2cpp_path, tmp_path, run_in_tmp_path):
152+
# Set very short timeout.
153+
subprocess.run(["export", "GIT_HTTP_TIMEOUT=0.001"], check=True)
154+
155+
# Check timeout is set.
156+
check_env_cmd = ["env"]
157+
p_check_env = subprocess.run(check_env_cmd, capture_output=True, cwd=tmp_path, text=True)
158+
assert "GIT_HTTP_TIMEOUT=0.001" in p_check_env.stdout
159+
160+
# Clone fails with timeout.
161+
clone_cmd = [git2cpp_path, "clone", xtl_url]
162+
p_clone = subprocess.run(clone_cmd, capture_output=True, cwd=tmp_path, text=True)
163+
assert p_clone.returncode != 0
164+
assert "network request timed out connecting to" in p_clone.stderr
165+
assert (
166+
"set a longer timeout in seconds using the environment variable GIT_HTTP_TIMEOUT"
167+
in p_clone.stderr
168+
)
169+
170+
# Set more reasonable timeout.
171+
subprocess.run(["export", "GIT_HTTP_TIMEOUT=10"], check=True)
172+
173+
# Check timeout is set.
174+
p_check_env = subprocess.run(check_env_cmd, capture_output=True, cwd=tmp_path, text=True)
175+
assert "GIT_HTTP_TIMEOUT=10" in p_check_env.stdout
176+
177+
# Clone succeeds.
178+
clone_cmd = [git2cpp_path, "clone", xtl_url]
179+
p_clone = subprocess.run(clone_cmd, capture_output=True, cwd=tmp_path, text=True)
180+
assert p_clone.returncode == 0
181+
182+
assert (tmp_path / "xtl").exists()
183+
assert (tmp_path / "xtl/include").exists()
184+
185+
186+
@pytest.mark.skipif(not GIT2CPP_TEST_WASM, reason="Only test in WebAssembly")
187+
def test_clone_negative_timeout_ignored(git2cpp_path, tmp_path, run_in_tmp_path):
188+
# Set negative timeout.
189+
subprocess.run(["export", "GIT_HTTP_TIMEOUT=-1"], check=True)
190+
191+
# Check timeout is set.
192+
check_env_cmd = ["env"]
193+
p_check_env = subprocess.run(check_env_cmd, capture_output=True, cwd=tmp_path, text=True)
194+
assert "GIT_HTTP_TIMEOUT=-1" in p_check_env.stdout
195+
196+
# Clone succeeds, ignoring invalid timeout.
197+
clone_cmd = [git2cpp_path, "clone", xtl_url]
198+
p_clone = subprocess.run(clone_cmd, capture_output=True, cwd=tmp_path, text=True)
199+
assert p_clone.returncode == 0
200+
201+
assert (tmp_path / "xtl").exists()
202+
assert (tmp_path / "xtl/include").exists()
203+
204+
assert (
205+
"environment variable GIT_HTTP_TIMEOUT must be a positive number of seconds"
206+
in p_clone.stdout
207+
)

0 commit comments

Comments
 (0)