Repository navigation
Fix HTTP server helper reporting success before shutdown finished - #3
Merged
Merged
Conversation
Previously, RunHTTPServerWithContext published the serve result as soon as startFn returned, and http.Server.Shutdown makes Serve return ErrServerClosed at the start of the shutdown rather than at its end. Callers were therefore told the server had exited while requests were still draining, and any shutdown error, including a timeout, only reached the log. After this change a single coordinator owns the channel: on cancellation it awaits Shutdown, collects the serve result and only then publishes the outcome, joining the two errors when both are present. A server that stopped on its own is reported immediately and is not shut down, so a failed start can be retried on it; that also holds when the cancellation lands at the same moment. A shutdown that times out publishes the timeout without waiting on startFn on top of the deadline it already missed, at the cost of not reporting a serving error that surfaces afterwards. The signature and the defaults are unchanged.
umputun
approved these changes
Aug 19, 2026
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.
Previously,
RunHTTPServerWithContextpublished the serve result as soon asstartFnreturned.http.Server.ShutdownmakesServereturnErrServerClosedat the beginning of the shutdown rather than at its end, so a caller receiving from the channel was told the server had exited while requests were still draining, and could terminate the process from under them. A shutdown error, a timeout included, only reached the log.After this change a single coordinator owns the channel. On cancellation it awaits
Shutdown, collects the serve result and only then publishes the outcome, joining the two errors when both are present.What changes for existing callers:
context.DeadlineExceededin particular, now arrives on the channel instead of only being logged, so a caller that treats a non-nil error as fatal will exit non-zero when the drain does not finish in time.The signature, the options and the defaults are unchanged. Two deliberate limits are documented rather than papered over: a server that stops on its own is reported immediately and is not shut down, so a failed bind can still be retried on the same server, and a shutdown that times out publishes the timeout without waiting on
startFnon top of the deadline it already missed, at the cost of not reporting a serving error that surfaces afterwards. Worknet/httpitself does not wait for, hijacked connections, connections dropped byServer.CloseandRegisterOnShutdowncallbacks, stays the caller's to track.Three tests cover the new behaviour: the result stays unpublished while a handler drains, a shutdown that times out delivers
context.DeadlineExceeded, and a server whose bind failed can be started again. Each fails against the previous implementation.