From 373f9348caa3a6ac1d9c1458055baee746b79641 Mon Sep 17 00:00:00 2001 From: omattsson Date: Sat, 14 Mar 2026 20:03:24 +0100 Subject: [PATCH 1/2] fix: set server WriteTimeout to 0 for WebSocket compatibility (#4) The default WriteTimeout was 10s, which caused WebSocket connections to be forcibly closed by the server after the timeout elapsed mid-stream. Setting it to 0 disables the deadline entirely, allowing long-lived WebSocket connections to remain open. Validation is updated to reject only negative values (< 0) instead of <= 0, so zero is now a valid, explicit configuration. Tests are updated to assert the new default and cover both the zero-passes and negative-fails cases. Refs #4 --- backend/api/main_integration_test.go | 2 +- backend/api/main_test.go | 4 +- backend/internal/config/config.go | 24 ++++++++--- backend/internal/config/config_test.go | 59 +++++++++++++++++++++++++- 4 files changed, 78 insertions(+), 11 deletions(-) diff --git a/backend/api/main_integration_test.go b/backend/api/main_integration_test.go index cb82b92..c3d5b24 100644 --- a/backend/api/main_integration_test.go +++ b/backend/api/main_integration_test.go @@ -34,7 +34,7 @@ func TestDatabaseInitialization(t *testing.T) { Host: "localhost", Port: "8082", ReadTimeout: 10 * time.Second, - WriteTimeout: 10 * time.Second, + WriteTimeout: time.Duration(0), ShutdownTimeout: 30 * time.Second, }, Logging: config.LogConfig{ diff --git a/backend/api/main_test.go b/backend/api/main_test.go index 2fe1a30..1efface 100644 --- a/backend/api/main_test.go +++ b/backend/api/main_test.go @@ -141,7 +141,7 @@ func mockLoadConfig() (*config.Config, error) { Host: "localhost", Port: "8082", ReadTimeout: 10 * time.Second, - WriteTimeout: 10 * time.Second, + WriteTimeout: time.Duration(0), ShutdownTimeout: 30 * time.Second, }, Logging: config.LogConfig{ @@ -288,5 +288,5 @@ func TestServerConfiguration(t *testing.T) { assert.Equal(t, addr, server.Addr) assert.Equal(t, 10*time.Second, server.ReadTimeout) - assert.Equal(t, 10*time.Second, server.WriteTimeout) + assert.Equal(t, time.Duration(0), server.WriteTimeout) } diff --git a/backend/internal/config/config.go b/backend/internal/config/config.go index 9652307..07dec2c 100644 --- a/backend/internal/config/config.go +++ b/backend/internal/config/config.go @@ -18,9 +18,13 @@ const ( defaultMaxIdleConns int32 = 5 defaultConnMaxLifetime = 5 * time.Minute defaultReadTimeout = 10 * time.Second - defaultWriteTimeout = 10 * time.Second - defaultIdleTimeout = 30 * time.Second - defaultShutdownTimeout = 30 * time.Second + // defaultWriteTimeout is 0 (disabled at the server level) because per-write + // deadlines are enforced in the WebSocket write pump (websocket/client.go). + // A non-zero server-level write timeout would prematurely terminate idle + // WebSocket connections. + defaultWriteTimeout = time.Duration(0) + defaultIdleTimeout = 30 * time.Second + defaultShutdownTimeout = 30 * time.Second ) // CORSConfig holds CORS configuration @@ -83,7 +87,13 @@ type AzureTableConfig struct { //nolint:govet // Struct field alignment has been optimized for time.Duration fields type ServerConfig struct { // 8-byte aligned fields first - ReadTimeout time.Duration + ReadTimeout time.Duration + // WriteTimeout is 0 (disabled) at the server level. Per-write deadlines are + // enforced inside the WebSocket write pump (websocket/client.go), so a + // server-level write timeout must be disabled to prevent premature + // termination of idle WebSocket connections. Standard REST handlers complete + // synchronously with sub-millisecond response writes and are not a concern + // in practice; per-handler context deadlines can be applied if required. WriteTimeout time.Duration IdleTimeout time.Duration ShutdownTimeout time.Duration @@ -194,8 +204,10 @@ func (c *ServerConfig) Validate() error { return errors.New("read timeout must be positive") } - if c.WriteTimeout <= 0 { - return errors.New("write timeout must be positive") + // WriteTimeout of 0 is valid — it disables the server-level write timeout + // (per-write deadlines are handled in the WebSocket write pump instead). + if c.WriteTimeout < 0 { + return errors.New("write timeout must be non-negative (0 to disable)") } if c.IdleTimeout <= 0 { diff --git a/backend/internal/config/config_test.go b/backend/internal/config/config_test.go index b6ebf70..973b3d0 100644 --- a/backend/internal/config/config_test.go +++ b/backend/internal/config/config_test.go @@ -122,8 +122,7 @@ func TestLoadConfig(t *testing.T) { assert.Empty(t, config.Server.Host) assert.Equal(t, "8081", config.Server.Port) assert.Equal(t, 10*time.Second, config.Server.ReadTimeout) - assert.Equal(t, 10*time.Second, config.Server.WriteTimeout) - assert.Equal(t, 30*time.Second, config.Server.ShutdownTimeout) + assert.Equal(t, time.Duration(0), config.Server.WriteTimeout) // disabled; per-write deadlines enforced in WebSocket write pump // Check default logging config assert.Equal(t, "info", config.Logging.Level) @@ -281,6 +280,62 @@ func TestConfigValidate(t *testing.T) { require.Error(t, err) assert.Contains(t, err.Error(), "server config") }) + + t.Run("zero WriteTimeout passes validation", func(t *testing.T) { + t.Parallel() + cfg := &config.Config{ + App: config.AppConfig{ + Name: "myapp", + Environment: "production", + }, + Database: config.DatabaseConfig{ + Host: "localhost", + Port: "3306", + User: "user", + DBName: "dbname", + MaxOpenConns: 10, + MaxIdleConns: 5, + ConnMaxLifetime: 1 * time.Minute, + }, + Server: config.ServerConfig{ + Port: "8080", + ReadTimeout: 5 * time.Second, + WriteTimeout: 0, // disabled at server level; per-write deadlines in WebSocket pump + IdleTimeout: 30 * time.Second, + ShutdownTimeout: 10 * time.Second, + }, + } + assert.NoError(t, cfg.Validate()) + }) + + t.Run("negative WriteTimeout fails validation", func(t *testing.T) { + t.Parallel() + cfg := &config.Config{ + App: config.AppConfig{ + Name: "myapp", + Environment: "production", + }, + Database: config.DatabaseConfig{ + Host: "localhost", + Port: "3306", + User: "user", + DBName: "dbname", + MaxOpenConns: 10, + MaxIdleConns: 5, + ConnMaxLifetime: 1 * time.Minute, + }, + Server: config.ServerConfig{ + Port: "8080", + ReadTimeout: 5 * time.Second, + WriteTimeout: -1 * time.Second, + IdleTimeout: 30 * time.Second, + ShutdownTimeout: 10 * time.Second, + }, + } + err := cfg.Validate() + require.Error(t, err) + assert.Contains(t, err.Error(), "write timeout") + }) } func TestAzureTableConfigValidate(t *testing.T) { t.Parallel() From 7972123089de2c725d4c3b111f676332cbb13ae2 Mon Sep 17 00:00:00 2001 From: omattsson Date: Sat, 14 Mar 2026 20:08:50 +0100 Subject: [PATCH 2/2] fix: address Copilot review comments on WriteTimeout PR --- backend/internal/config/config.go | 6 ++++-- backend/internal/config/config_test.go | 4 ++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/backend/internal/config/config.go b/backend/internal/config/config.go index 07dec2c..0122dfb 100644 --- a/backend/internal/config/config.go +++ b/backend/internal/config/config.go @@ -92,8 +92,10 @@ type ServerConfig struct { // enforced inside the WebSocket write pump (websocket/client.go), so a // server-level write timeout must be disabled to prevent premature // termination of idle WebSocket connections. Standard REST handlers complete - // synchronously with sub-millisecond response writes and are not a concern - // in practice; per-handler context deadlines can be applied if required. + // quickly in practice, but setting WriteTimeout to 0 does mean slow or + // stalled clients can hold a connection indefinitely. Operators who want + // protection against slow clients should set SERVER_WRITE_TIMEOUT to a + // positive value (e.g. 30s); per-handler context deadlines can also be applied. WriteTimeout time.Duration IdleTimeout time.Duration ShutdownTimeout time.Duration diff --git a/backend/internal/config/config_test.go b/backend/internal/config/config_test.go index 973b3d0..e117477 100644 --- a/backend/internal/config/config_test.go +++ b/backend/internal/config/config_test.go @@ -122,8 +122,8 @@ func TestLoadConfig(t *testing.T) { assert.Empty(t, config.Server.Host) assert.Equal(t, "8081", config.Server.Port) assert.Equal(t, 10*time.Second, config.Server.ReadTimeout) - assert.Equal(t, time.Duration(0), config.Server.WriteTimeout) // disabled; per-write deadlines enforced in WebSocket write pump - + assert.Equal(t, time.Duration(0), config.Server.WriteTimeout) // disabled; per-write deadlines enforced in WebSocket write pump assert.Equal(t, 30*time.Second, config.Server.IdleTimeout) + assert.Equal(t, 30*time.Second, config.Server.ShutdownTimeout) // Check default logging config assert.Equal(t, "info", config.Logging.Level) assert.Empty(t, config.Logging.File)