fix: adjust server WriteTimeout for WebSocket compatibility#11
Merged
fix: adjust server WriteTimeout for WebSocket compatibility#11
Conversation
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
There was a problem hiding this comment.
Pull request overview
Adjusts the backend HTTP server’s default WriteTimeout to support long-lived WebSocket connections by disabling the server-level write deadline while retaining per-write WebSocket deadlines.
Changes:
- Set
defaultWriteTimeoutto0(disabled) and allowWriteTimeout == 0in server config validation. - Update config tests to assert the new default and add validation cases for
WriteTimeoutbeing0and negative. - Update
maintests/integration tests to expectWriteTimeoutof0.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| backend/internal/config/config.go | Changes default WriteTimeout to 0, updates validation, and expands documentation comments. |
| backend/internal/config/config_test.go | Updates default assertions and adds validation test cases for WriteTimeout. |
| backend/api/main_test.go | Updates mocked config and server assertions to match new WriteTimeout default. |
| backend/api/main_integration_test.go | Updates integration test config to use WriteTimeout: 0. |
You can also share your feedback on Copilot code review. Take the survey.
Comment on lines
122
to
126
| 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 | ||
|
|
backend/internal/config/config.go
Outdated
Comment on lines
+91
to
+96
| // 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. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
The HTTP server's
WriteTimeoutwas hardcoded to10sby default. This caused WebSocket connections to be forcibly terminated by the server after 10 seconds of inactivity or mid-stream, making long-lived WebSocket connections impossible without manual configuration overrides.Setting the default to
0disables the write deadline entirely, which is the standard practice for servers that need to support WebSocket or streaming responses.Changes
backend/internal/config/config.go— changeddefaultWriteTimeoutfrom10sto0; updated validation to reject only negative values (< 0instead of<= 0); corrected doc comment onServerConfig.WriteTimeoutbackend/internal/config/config_test.go— updated default assertion; added two new test cases: zero value passes validation, negative value fails validation-backend/internal/config/config_test.go— updatCo-backend/internal/config/config_test.go— updated defaultbackend/api/main_integration_test.go— updatedTestDatabaseInitializationto usetime.Duration(0)Testing
Closes #4