Add custom timeout option for requests #45

caifanuncle · 2025-02-25T11:51:59Z

Related to #44

Currently there is no way to change the default timeout length for requests. This is a must have for dealing with APIs which take a while to generate a response, for example, LLMs.

caifanuncle · 2025-02-25T17:45:12Z

src/gleam/httpc.gleam

  )
 }

 /// Create a new configuration with the default settings.
 ///
 pub fn configure() -> Configuration {
-  Builder(verify_tls: True, follow_redirects: False)
+  Builder(verify_tls: True, follow_redirects: False, timeout: None)


default timeout on Erlang implementation is Infinity (ie no timeout) according to https://www.erlang.org/docs/26/man/httpc#request-5

Do we want to mirror the erlang implementation or have a more "sensible" default, like say 30s/60s? I've only worked with C#/php over the years but haven't seen an infinite timeout before. Dotnet uses 100s for example.

It's ultimately arbitrary, but an infinite default feels like a bit of a trap to me, ha.

lpil

Thank you! I've left some notes inline.

lpil · 2025-02-26T14:12:40Z

test/gleam_httpc_test.gleam

+        |> httpc.dispatch(req)
+    },
+  ])
+}


Split this into two tests please and use the shortest possible timeouts rather than thes long ones. We don't want the tests to take a long time to run.

lpil · 2025-02-26T14:12:55Z

src/gleam/httpc.gleam

@@ -155,13 +161,16 @@ pub opaque type Configuration {
    /// Whether to follow redirects.
    ///
    follow_redirects: Bool,
+    /// Timeout for the request in milliseconds.
+    ///
+    timeout: Option(Int),


Document what the default behaviour is please

Could you document what the timeout is for please? I think it's the timeout for the entire response body to be received, starting from when the request is sent.

Remove the infinity part as there's no such value in Gleam. It would be good to have a default timeout, as @jakecleary says

lpil · 2025-02-26T14:13:15Z

src/gleam/httpc.gleam

@@ -182,6 +191,11 @@ pub fn follow_redirects(config: Configuration, which: Bool) -> Configuration {
  Builder(..config, follow_redirects: which)
 }

+/// Set the timeout for the request in milliseconds.


Document what the default behaviour is if this isn't called please

lpil · 2025-02-26T14:14:50Z

src/gleam/httpc.gleam

 import gleam/result
 import gleam/uri

 pub type HttpError {
  InvalidUtf8Response
  FailedToConnect(ip4: ConnectError, ip6: ConnectError)
+  RequestTimeout


Could we improve this name? I'd like to make it clear that the timeout is that the response body wasn't received in time. Documenting it with a /// comment would be useful too

Sorry, I was asking what ideas you had for names 😁 I wonder what other HTTP clients call this error

did a bit research on http clients from other languages, and i find that the vocabulary they use for this type of error tends to be either a simple Timeout or RequestTimeout.

(source: admittedly chatgpt lol)

Python (requests & httpx) requests.exceptions.Timeout → RequestTimeout httpx.TimeoutException → HttpxTimeoutError JavaScript (Axios, Node fetch) AxiosError: timeout exceeded → AxiosTimeoutError AbortError (used in fetch) → FetchTimeoutError Go (net/http, fasthttp) net/http: request canceled (Client.Timeout exceeded) → RequestCanceledError fasthttp.ErrTimeout → FastHttpTimeoutError Ruby (Net::HTTP, Faraday) Net::OpenTimeout → OpenTimeoutError Net::ReadTimeout → ReadTimeoutError Faraday::TimeoutError → FaradayTimeoutError Rust (reqwest) reqwest::Error::TimedOut → ReqwestTimeoutError Java (HttpClient, OkHttp) java.net.SocketTimeoutException → SocketTimeoutError java.net.ConnectException → ConnectTimeoutError okhttp3.internal.http2.StreamResetException → OkHttpTimeoutError

tbh i still think a simple RequestTimeout is more than descriptive as ppl tend to see the request time as a whole, from the moment the request is triggered to the moment the response body is received in full. wdyt?

I think the LLM is hallucinating or misunderstood the question there. The ones I checked didn't exist and some of them don't seem to make any sense for how the language works?

Please don't share LLM output as research. Following this fake information is a waste of everybody's time.

apologies, just thought it could be a quick way to get some inspiration. will be mindful of this in the future. 🙏

did some 'proper' research this time round and i have the following. not sure which set of vocabulary resonates most w u; i still personally think RequestTimeout is simple and descriptive enough.

WDYT?

NodeJS
Request Timeout

Ruby (Net)
Read Timeout

Python
requests.exceptions.Timeout

Hey 👋 Thought I would chime in...

I think framing it from the perspective of the request is more common (i.e. some variant of RequestTimeout) but I like the idea of mirroring the existing InvalidUtf8Response error and having something like: NoResponseReceived.

I quite like how explicit that is, but it might feel a bit less familiar, coming from other languages. e.g. coming from C#/dotnet, it uses a TimeoutException.

"ResponseTimedOut"? To match "FailedToConnect"?

oderwat · 2025-03-20T12:42:20Z

We would need this for our (yet to finish) glowlama (ollama) package (the existing one is not useable for generations).

Edit: Since the new git repository support let me use the fork easily, this is now used in our package, and it works very nicely!

kwando · 2025-05-27T10:56:36Z

What needs to be done in order to push this over the finish line?

caifanuncle marked this pull request as draft February 25, 2025 11:53

caifanuncle force-pushed the zonghan/custom_timeout branch 4 times, most recently from 0c3c0e0 to a2831b0 Compare February 25, 2025 15:03

caifanuncle marked this pull request as ready for review February 25, 2025 15:05

caifanuncle changed the title ~~Add custom timeout option for outgoing request~~ Add custom timeout option for request Feb 25, 2025

caifanuncle changed the title ~~Add custom timeout option for request~~ Add custom timeout option for requests Feb 25, 2025

caifanuncle force-pushed the zonghan/custom_timeout branch 2 times, most recently from 3ba06ad to 5155726 Compare February 25, 2025 17:41

caifanuncle commented Feb 25, 2025

View reviewed changes

lpil reviewed Feb 26, 2025

View reviewed changes

caifanuncle requested a review from lpil February 26, 2025 15:10

add timeout option

756213b

caifanuncle force-pushed the zonghan/custom_timeout branch from 64db768 to 756213b Compare February 26, 2025 15:25

rename timeout error

0365909

Add custom timeout option for requests #45

Are you sure you want to change the base?

Add custom timeout option for requests #45

Uh oh!

Conversation

caifanuncle commented Feb 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

caifanuncle Feb 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

lpil left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

caifanuncle Feb 28, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

oderwat commented Mar 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kwando commented May 27, 2025

Uh oh!

Uh oh!

caifanuncle commented Feb 25, 2025 •

edited

Loading

caifanuncle Feb 25, 2025 •

edited

Loading

caifanuncle Feb 28, 2025 •

edited

Loading

oderwat commented Mar 20, 2025 •

edited

Loading