Skip to content

http: port Go net/http timeout handling - #67

Open
Darckfast wants to merge 1 commit into
tinygo-org:mainfrom
Darckfast:port-net-http-timeout
Open

http: port Go net/http timeout handling#67
Darckfast wants to merge 1 commit into
tinygo-org:mainfrom
Darckfast:port-net-http-timeout

Conversation

@Darckfast

@Darckfast Darckfast commented Jul 5, 2026

Copy link
Copy Markdown

This PR ports Go's net/http timeout feature to TinyGo allowing it to handle timeouts when using net/http e.g.

c := http.Client{
  Timeout: 2 * time.Second,
}

^ this will now return a timeout error if the request duration is longer than 2 seconds

This port is pretty much 1:1 to Go own timeout implementation, i removed parts that TInyGo doesnt support (they are commented with // TINYGO)

By bringing the Go net/http client .transport() function, it also fixes the issue where only roundTrip.go was used by default, even when compiling to wasm, where roundTrip_js.go should be used

Fixes #58
Fixes #66

I have wrote a wasm integration test , but i dont know how to go about it because it's inside test/wasm in the main repo, do i just open a PR in the main repo with it? or you guys suggest something else?

This ports Go's timeout feature to TinyGo allowing it to handle timeouts when using `net/http` e.g. `c := http.Client{Timeout: 2 * time.Second}` will now return a timeout error if the request duration is longer than 2 seconds

By bringing the Go net/http client `.transport()` function, it also fixes the issue where only `roundTrip.go` was used, even when compiling to wasm, where `roundTrip_js.go` should be used

Fixes tinygo-org#58 and tinygo-org#66
@Darckfast
Darckfast force-pushed the port-net-http-timeout branch from 146f177 to ba49238 Compare July 12, 2026 01:17
@b0ch3nski

Copy link
Copy Markdown

I have wrote a wasm integration test , but i dont know how to go about it because it's inside test/wasm in the main repo, do i just open a PR in the main repo with it? or you guys suggest something else?

Yes, please do so and maybe change net submodule to your branch so it can be tested together?

@0pcom

0pcom commented Jul 15, 2026

Copy link
Copy Markdown

Tested this against a real workload — the Skycoin daemon and web wallet, which use http.Client{Timeout: …} for peer/node queries — on the native (non-wasm) roundTrip.go path, combined with the host netdev from #59.

Confirmed it fixes the #58 crash: http.Client{Timeout}.Get() to an unreachable host now returns the dial error instead of panicking on the nil didTimeout. 👍

One thing worth flagging for the native path: the Timeout timer fires but doesn't actually abort an in-flight request. Against a server that sleeps 4s with a 1s client timeout, the call returns after the full ~4s with a nil error instead of timing out at ~1s:

T1 refused: true | connection refused        (crash fixed ✓)
T2 slow:   err=<nil> elapsed=4003ms          (want timeout ~1000ms)

setRequestCancel wires up req.Cancel/context and the timer correctly, but http/roundTrip.go (the native approximation of Transport.roundTrip) never observes req.Cancel/context/deadline and nothing sets a read deadline on the conn — so when the timer fires there's nothing to interrupt the read. On the wasm path roundTrip_js.go can cancel via the browser fetch, so this only affects the native path.

Not a blocker for this PR — fixing the crash is already a clear improvement. Just flagging that making Timeout actually cut off a slow response on native will additionally need roundTrip to honor cancellation (close the conn or set a read deadline when req.Cancel fires). Happy to help with that as a follow-up.

@Darckfast

Copy link
Copy Markdown
Author

Tested this against a real workload — the Skycoin daemon and web wallet, which use http.Client{Timeout: …} for peer/node queries — on the native (non-wasm) roundTrip.go path, combined with the host netdev from #59.

Confirmed it fixes the #58 crash: http.Client{Timeout}.Get() to an unreachable host now returns the dial error instead of panicking on the nil didTimeout. 👍

One thing worth flagging for the native path: the Timeout timer fires but doesn't actually abort an in-flight request. Against a server that sleeps 4s with a 1s client timeout, the call returns after the full ~4s with a nil error instead of timing out at ~1s:

T1 refused: true | connection refused        (crash fixed ✓)
T2 slow:   err=<nil> elapsed=4003ms          (want timeout ~1000ms)

setRequestCancel wires up req.Cancel/context and the timer correctly, but http/roundTrip.go (the native approximation of Transport.roundTrip) never observes req.Cancel/context/deadline and nothing sets a read deadline on the conn — so when the timer fires there's nothing to interrupt the read. On the wasm path roundTrip_js.go can cancel via the browser fetch, so this only affects the native path.

Not a blocker for this PR — fixing the crash is already a clear improvement. Just flagging that making Timeout actually cut off a slow response on native will additionally need roundTrip to honor cancellation (close the conn or set a read deadline when req.Cancel fires). Happy to help with that as a follow-up.

interesting, i will take a look at this

@deadprogram

Copy link
Copy Markdown
Member

Hello @Darckfast here is a lightly edited version of an automated code review:

Review: PR #67 — http: port Go net/http timeout handling

PR Summary

This PR ports Go's net/http timeout machinery to TinyGo, enabling http.Client{Timeout: ...} to work. It also fixes the issue where roundTrip.go was used on wasm instead of roundTrip_js.go. Changes span two files (+173, -5): http/client.go and http/transport.go.

Fixes: #58 (crash on Do() with Timeout), #66 (wrong round-tripper on wasm)


Critical Issues

1. Client.Do() bypasses timeout when Transport is set

http/client.go lines 591-597:

func (c *Client) Do(req *Request) (*Response, error) {
	if c.Transport != nil {
		return c.Transport.RoundTrip(req)
	}
	return c.do(req)
}

When a user sets a custom Transport, this short-circuits directly to RoundTrip(), bypassing c.do() and all the new timeout/deadline/cookie machinery. The common pattern:

c := &http.Client{Transport: customTransport, Timeout: 5 * time.Second}

...will not timeout. This should unconditionally call c.do(req) - the c.transport() method (added by this very PR) already handles transport selection inside send().

2. timeoutError doesn't actually implement net.Error

http/transport.go lines 135-141:

// httpTimeoutError represents a timeout.
// It implements net.Error and wraps context.DeadlineExceeded.
type timeoutError struct {
	err string
}

func (e *timeoutError) Error() string { return e.err }

The comment says it implements net.Error, but it's missing the required methods:

func (e *timeoutError) Timeout() bool   { return true }
func (e *timeoutError) Temporary() bool  { return true }
func (e *timeoutError) Unwrap() error    { return context.DeadlineExceeded }

Without Timeout() bool, callers using err.(net.Error).Timeout() or errors.Is(err, context.DeadlineExceeded) won't be able to detect timeout errors, which defeats the purpose of wrapping the error in cancelTimerBody.Read().

3. Timeout error not wrapped in *url.Error

http/client.go line 621:

return nil, fmt.Errorf("%s (Client.Timeout exceeded while awaiting headers)", err.Error())

The Do() documentation states: "Any returned error will be of type [*url.Error]. The url.Error value's Timeout method will report true if the request timed out." But this returns a plain fmt.Errorf string. Code that type-asserts err.(*url.Error) to check .Timeout() will panic. Should be:

return nil, &url.Error{
    Op:  urlErrorOp(req.Method),
    URL: req.URL.String(),
    Err: &timeoutError{err.Error()},
}

Minor Issues

4. ctx in roundTrip() is created but underused

http/client.go lines 420-432:

ctx, cancel := context.WithCancelCause(req.Context())
defer func() {
    if err != nil {
        cancel(err)
    }
}()

select {
case <-ctx.Done():
    req.closeBody()
    return nil, context.Cause(ctx)
default:
}

This is a one-shot pre-dial check. After the select, ctx is never used again — net.Dial("tcp", host) doesn't accept a context. To properly cancel in-flight I/O, you'd need (&net.Dialer{}).DialContext(ctx, ...). This is the known limitation @0pcom flagged (not a blocker).

5. Version comment inconsistency

  • http/roundtrip.go and http/roundtrip_js.go: "Go 1.21.4"
  • http/client.go and http/transport.go: "Go 1.26.2"

The roundtrip*.go files weren't updated to reflect the new version tag.


What works well

  • The setRequestCancel / knownRoundTripperImpl / timeBeforeContextDeadline port is faithful to upstream Go
  • The cancelTimerBody wrapping correctly stops the timer on body close
  • The wasm fix (routing through c.transport()Transport.RoundTrip instead of calling roundTrip() directly) is clean and correct
  • Fixes the nil didTimeout panic from Tinygo crashes on http.Client{}.Do() with Timeout #58

Verdict

The core approach is sound, but issues #1 and #2 need to be fixed before merge since they cause the timeout feature to silently not work in common configurations. Issue #3 is a correctness problem for error handling but less likely to cause crashes.

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.

net/http: client always uses TinyGo roundTrip instead of roundTrip_js in wasm Tinygo crashes on http.Client{}.Do() with Timeout

4 participants