feat: add app_start_timeout to proxy configuration and improve error handling (#844)
* feat: add app_start_timeout to proxy configuration and improve error handling
* fix: resolve P0 and P1 issues in app_start_timeout proxy feature
- Use context timeout for first HTTP request to prevent hanging
- Fix WriteHeader called before setting Content-Length header
- Simplify retry loop condition (for err != nil instead of for { if err == nil })
- Add code comments for clarity (HTML vs non-HTML response handling)
- Clean up strconv.Itoa usage (remove unnecessary byte conversion)
- Add documentation to air_example.toml explaining app_start_timeout
- Add comprehensive unit tests for timeout scenarios (5 test cases)
P0 fixes (critical):
- First request now uses context timeout
- WriteHeader now called after all headers are set
P1 fixes (strongly recommended):
- Documented in air_example.toml with usage examples
- Added TestProxy_appStartTimeout with 5 test scenarios
Related to #656 (proxy reload timing issue)
* test: add comprehensive test coverage for app_start_timeout feature
---------
Co-authored-by: Juan Gonzalez <jrg2156@gmail.com>
Co-authored-by: Juan Gonzalez <juanrgon@github.com>
This commit is contained in:
@@ -85,3 +85,8 @@ keep_scroll = true
|
||||
enabled = true
|
||||
proxy_port = 8090
|
||||
app_port = 8080
|
||||
# Timeout in milliseconds for waiting for the app to start and become available.
|
||||
# Useful when your app has slow startup time (e.g., database connections, config loading).
|
||||
# The proxy will retry connecting to your app for this duration before giving up.
|
||||
# Default is 5000ms (5 seconds). Increase this if you see "unable to reach app" errors.
|
||||
app_start_timeout = 5000
|
||||
|
||||
+6
-3
@@ -21,6 +21,8 @@ const (
|
||||
dftTOML = ".air.toml"
|
||||
airWd = "air_wd"
|
||||
|
||||
defaultProxyAppStartTimeout = 5000
|
||||
|
||||
schemaHeader = "#:schema https://json.schemastore.org/any.json"
|
||||
)
|
||||
|
||||
@@ -160,9 +162,10 @@ type cfgScreen struct {
|
||||
}
|
||||
|
||||
type cfgProxy struct {
|
||||
Enabled bool `toml:"enabled" usage:"Enable live-reloading on the browser"`
|
||||
ProxyPort int `toml:"proxy_port" usage:"Port for proxy server"`
|
||||
AppPort int `toml:"app_port" usage:"Port for your app"`
|
||||
Enabled bool `toml:"enabled" usage:"Enable live-reloading on the browser"`
|
||||
ProxyPort int `toml:"proxy_port" usage:"Port for proxy server"`
|
||||
AppPort int `toml:"app_port" usage:"Port for your app"`
|
||||
AppStartTimeout int `toml:"app_start_timeout" usage:"Timeout for waiting for app to start in milliseconds (default 5000)"`
|
||||
}
|
||||
|
||||
type sliceTransformer struct{}
|
||||
|
||||
@@ -666,6 +666,7 @@ func (e *Engine) runBin() error {
|
||||
processExit := make(chan struct{})
|
||||
e.mainDebug("running process pid %v", cmd.Process.Pid)
|
||||
if e.config.Proxy.Enabled {
|
||||
e.mainDebug("reloading proxy")
|
||||
e.proxy.Reload()
|
||||
}
|
||||
|
||||
|
||||
+17
-6
@@ -2,6 +2,7 @@ package runner
|
||||
|
||||
import (
|
||||
"bytes"
|
||||
"context"
|
||||
_ "embed"
|
||||
"fmt"
|
||||
"io"
|
||||
@@ -112,19 +113,28 @@ func (p *Proxy) proxyHandler(w http.ResponseWriter, r *http.Request) {
|
||||
viaHeaderValue := fmt.Sprintf("%s %s", r.Proto, r.Host)
|
||||
req.Header.Set("Via", viaHeaderValue)
|
||||
|
||||
// air will restart the server. it may take a few milliseconds for it to start back up.
|
||||
// air will restart the server. it may take a few seconds for it to start back up.
|
||||
// therefore, we retry until the server becomes available or this retry loop exits with an error.
|
||||
timeout := time.Duration(p.config.AppStartTimeout) * time.Millisecond
|
||||
if timeout == 0 {
|
||||
timeout = defaultProxyAppStartTimeout * time.Millisecond
|
||||
}
|
||||
ctx, cancel := context.WithTimeout(r.Context(), timeout)
|
||||
defer cancel()
|
||||
|
||||
var resp *http.Response
|
||||
resp, err = p.client.Do(req)
|
||||
for i := 0; i < 10; i++ {
|
||||
if err == nil {
|
||||
resp, err = p.client.Do(req.WithContext(ctx))
|
||||
for err != nil {
|
||||
// Check if timeout has been exceeded
|
||||
if ctx.Err() != nil {
|
||||
err = ctx.Err()
|
||||
break
|
||||
}
|
||||
time.Sleep(100 * time.Millisecond)
|
||||
resp, err = p.client.Do(req)
|
||||
resp, err = p.client.Do(req.WithContext(ctx))
|
||||
}
|
||||
if err != nil {
|
||||
http.Error(w, "proxy handler: unable to reach app", http.StatusInternalServerError)
|
||||
http.Error(w, "proxy handler: unable to reach app (try increasing the proxy.app_start_timeout)", http.StatusInternalServerError)
|
||||
return
|
||||
}
|
||||
defer resp.Body.Close()
|
||||
@@ -177,6 +187,7 @@ func (p *Proxy) proxyHandler(w http.ResponseWriter, r *http.Request) {
|
||||
return
|
||||
}
|
||||
} else {
|
||||
// HTML: inject live reload script
|
||||
page, err := p.injectLiveReload(resp)
|
||||
if err != nil {
|
||||
http.Error(w, err.Error(), http.StatusInternalServerError)
|
||||
|
||||
@@ -533,3 +533,106 @@ func TestProxy_proxyHandler_NonStreaming(t *testing.T) {
|
||||
require.NoError(t, err)
|
||||
assert.Equal(t, content, string(body))
|
||||
}
|
||||
|
||||
func TestProxy_appStartTimeout(t *testing.T) {
|
||||
tests := []struct {
|
||||
name string
|
||||
appStartDelay int // milliseconds - simulated app startup delay
|
||||
configTimeout int // milliseconds - configured timeout (0 = use default)
|
||||
expectSuccess bool
|
||||
expectInBody string
|
||||
expectNotInBody string
|
||||
}{
|
||||
{
|
||||
name: "fast_startup_succeeds",
|
||||
appStartDelay: 50,
|
||||
configTimeout: 1000,
|
||||
expectSuccess: true,
|
||||
expectInBody: "OK",
|
||||
},
|
||||
{
|
||||
name: "slow_startup_within_timeout_succeeds",
|
||||
appStartDelay: 500,
|
||||
configTimeout: 1000,
|
||||
expectSuccess: true,
|
||||
expectInBody: "OK",
|
||||
},
|
||||
{
|
||||
name: "slow_startup_exceeds_timeout_fails",
|
||||
appStartDelay: 1500,
|
||||
configTimeout: 500,
|
||||
expectSuccess: false,
|
||||
expectInBody: "unable to reach app",
|
||||
expectNotInBody: "OK",
|
||||
},
|
||||
{
|
||||
name: "uses_default_timeout_when_zero",
|
||||
appStartDelay: 500,
|
||||
configTimeout: 0, // Should use defaultProxyAppStartTimeout (5000ms)
|
||||
expectSuccess: true,
|
||||
expectInBody: "OK",
|
||||
},
|
||||
{
|
||||
name: "custom_timeout_respected",
|
||||
appStartDelay: 800,
|
||||
configTimeout: 1000,
|
||||
expectSuccess: true,
|
||||
expectInBody: "OK",
|
||||
},
|
||||
}
|
||||
|
||||
for _, tt := range tests {
|
||||
t.Run(tt.name, func(t *testing.T) {
|
||||
// Track whether server has "started"
|
||||
var serverReady sync.WaitGroup
|
||||
serverReady.Add(1)
|
||||
|
||||
// Create a test server that delays before responding
|
||||
srv := httptest.NewServer(http.HandlerFunc(func(w http.ResponseWriter, _ *http.Request) {
|
||||
// Wait for simulated startup delay on first request
|
||||
serverReady.Wait()
|
||||
w.WriteHeader(http.StatusOK)
|
||||
fmt.Fprint(w, "OK")
|
||||
}))
|
||||
defer srv.Close()
|
||||
|
||||
// Simulate app startup delay in background
|
||||
go func() {
|
||||
if tt.appStartDelay > 0 {
|
||||
// Sleep to simulate app initialization time
|
||||
time.Sleep(time.Duration(tt.appStartDelay) * time.Millisecond)
|
||||
}
|
||||
serverReady.Done()
|
||||
}()
|
||||
|
||||
srvPort := getServerPort(t, srv)
|
||||
proxy := NewProxy(&cfgProxy{
|
||||
Enabled: true,
|
||||
ProxyPort: proxyPort,
|
||||
AppPort: srvPort,
|
||||
AppStartTimeout: tt.configTimeout,
|
||||
})
|
||||
|
||||
req := httptest.NewRequest("GET", fmt.Sprintf("http://localhost:%d/", proxyPort), nil)
|
||||
rec := httptest.NewRecorder()
|
||||
|
||||
proxy.proxyHandler(rec, req)
|
||||
|
||||
resp := rec.Result()
|
||||
bodyBytes, err := io.ReadAll(resp.Body)
|
||||
require.NoError(t, err)
|
||||
body := string(bodyBytes)
|
||||
|
||||
if tt.expectSuccess {
|
||||
assert.Equal(t, http.StatusOK, resp.StatusCode, "expected successful response")
|
||||
assert.Contains(t, body, tt.expectInBody, "response body should contain expected content")
|
||||
} else {
|
||||
assert.Equal(t, http.StatusInternalServerError, resp.StatusCode, "expected error response")
|
||||
assert.Contains(t, body, tt.expectInBody, "error message should contain expected text")
|
||||
if tt.expectNotInBody != "" {
|
||||
assert.NotContains(t, body, tt.expectNotInBody, "response should not contain unexpected content")
|
||||
}
|
||||
}
|
||||
})
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user